From d9e17a62ce0a35af35a92061dbdbff070c655a50 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Sat, 31 Mar 2018 12:03:03 -0400 Subject: [PATCH] Refine SyncInvocableHandlerMethod error handling Ensure the error is wrapped as ServerErrorException --- .../web/server/ServerErrorException.java | 59 +++++++++++++++---- .../web/reactive/BindingContext.java | 6 +- .../method/SyncInvocableHandlerMethod.java | 9 ++- .../PathVariableMethodArgumentResolver.java | 2 +- 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/server/ServerErrorException.java b/spring-web/src/main/java/org/springframework/web/server/ServerErrorException.java index f25cad1886..523b44d751 100644 --- a/spring-web/src/main/java/org/springframework/web/server/ServerErrorException.java +++ b/spring-web/src/main/java/org/springframework/web/server/ServerErrorException.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,11 +19,12 @@ package org.springframework.web.server; import org.springframework.core.MethodParameter; import org.springframework.http.HttpStatus; import org.springframework.lang.Nullable; +import org.springframework.web.method.HandlerMethod; /** - * Exception for errors that fit response status 500 (bad request) for use in - * Spring Web applications. The exception provides additional fields (e.g. - * an optional {@link MethodParameter} if related to the error). + * Exception for an {@link HttpStatus#INTERNAL_SERVER_ERROR} that exposes extra + * information about a controller method that failed, or a controller method + * argument that could not be resolved. * * @author Rossen Stoyanchev * @since 5.0 @@ -31,6 +32,9 @@ import org.springframework.lang.Nullable; @SuppressWarnings("serial") public class ServerErrorException extends ResponseStatusException { + @Nullable + private final HandlerMethod handlerMethod; + @Nullable private final MethodParameter parameter; @@ -39,27 +43,60 @@ public class ServerErrorException extends ResponseStatusException { * Constructor with an explanation only. */ public ServerErrorException(String reason) { - this(reason, null, null); + super(HttpStatus.INTERNAL_SERVER_ERROR, reason, null); + this.handlerMethod = null; + this.parameter = null; } /** - * Constructor for a 500 error linked to a specific {@code MethodParameter}. + * Constructor with a reason and root cause. + * @since 5.0.5 */ - public ServerErrorException(String reason, MethodParameter parameter) { - this(reason, parameter, null); + public ServerErrorException(String reason, Throwable cause) { + super(HttpStatus.INTERNAL_SERVER_ERROR, reason, cause); + this.handlerMethod = null; + this.parameter = null; + } + + /** + * Constructor for a 500 error with a {@link MethodParameter}. + */ + public ServerErrorException(String reason, MethodParameter parameter, @Nullable Throwable cause) { + super(HttpStatus.INTERNAL_SERVER_ERROR, reason, cause); + this.handlerMethod = null; + this.parameter = parameter; } /** * Constructor for a 500 error with a root cause. */ - public ServerErrorException(String reason, @Nullable MethodParameter parameter, @Nullable Throwable cause) { + public ServerErrorException(String reason, HandlerMethod handlerMethod, @Nullable Throwable cause) { super(HttpStatus.INTERNAL_SERVER_ERROR, reason, cause); - this.parameter = parameter; + this.handlerMethod = handlerMethod; + this.parameter = null; + } + + /** + * Constructor for a 500 error linked to a specific {@code MethodParameter}. + * @deprecated in favor of {@link #ServerErrorException(String, MethodParameter, Throwable)} + */ + @Deprecated + public ServerErrorException(String reason, MethodParameter parameter) { + this(reason, parameter, null); } /** - * Return the {@code MethodParameter} associated with this error, if any. + * Return the controller method associated with the error, if any. + * @since 5.0.5 + */ + @Nullable + public HandlerMethod getHandlerMethod() { + return this.handlerMethod; + } + + /** + * Return the controller method argument associated with this error, if any. */ @Nullable public MethodParameter getMethodParameter() { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java b/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java index 4e0b7cf98c..8099a538d7 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/BindingContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,7 @@ import org.springframework.ui.Model; import org.springframework.validation.support.BindingAwareConcurrentModel; import org.springframework.web.bind.support.WebBindingInitializer; import org.springframework.web.bind.support.WebExchangeDataBinder; +import org.springframework.web.server.ServerErrorException; import org.springframework.web.server.ServerWebExchange; /** @@ -75,6 +76,7 @@ public class BindingContext { * @param target the object to create a data binder for * @param name the name of the target object * @return the created data binder + * @throws ServerErrorException if {@code @InitBinder} method invocation fails */ public WebExchangeDataBinder createDataBinder(ServerWebExchange exchange, @Nullable Object target, String name) { WebExchangeDataBinder dataBinder = new WebExchangeDataBinder(target, name); @@ -86,6 +88,7 @@ public class BindingContext { /** * Initialize the data binder instance for the given exchange. + * @throws ServerErrorException if {@code @InitBinder} method invocation fails */ protected WebExchangeDataBinder initDataBinder(WebExchangeDataBinder binder, ServerWebExchange exchange) { return binder; @@ -97,6 +100,7 @@ public class BindingContext { * @param exchange the current exchange * @param name the name of the target object * @return the created data binder + * @throws ServerErrorException if {@code @InitBinder} method invocation fails */ public WebExchangeDataBinder createDataBinder(ServerWebExchange exchange, String name) { return createDataBinder(exchange, null, name); diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/SyncInvocableHandlerMethod.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/SyncInvocableHandlerMethod.java index 216ecf7a0c..c60f24b88b 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/SyncInvocableHandlerMethod.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/SyncInvocableHandlerMethod.java @@ -29,6 +29,7 @@ import org.springframework.lang.Nullable; import org.springframework.web.method.HandlerMethod; import org.springframework.web.reactive.BindingContext; import org.springframework.web.reactive.HandlerResult; +import org.springframework.web.server.ServerErrorException; import org.springframework.web.server.ServerWebExchange; /** @@ -95,6 +96,7 @@ public class SyncInvocableHandlerMethod extends HandlerMethod { * @param bindingContext the binding context to use * @param providedArgs optional list of argument values to match by type * @return Mono with a {@link HandlerResult}. + * @throws ServerErrorException if method argument resolution or method invocation fails */ @Nullable public HandlerResult invokeForHandlerResult(ServerWebExchange exchange, @@ -104,9 +106,10 @@ public class SyncInvocableHandlerMethod extends HandlerMethod { this.delegate.invoke(exchange, bindingContext, providedArgs).subscribeWith(processor); if (processor.isTerminated()) { - Throwable error = processor.getError(); - if (error != null) { - throw (RuntimeException) error; + Throwable ex = processor.getError(); + if (ex != null) { + throw (ex instanceof ServerErrorException ? (ServerErrorException) ex : + new ServerErrorException("Failed to invoke: " + getShortLogMessage(), this, ex)); } return processor.peek(); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/PathVariableMethodArgumentResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/PathVariableMethodArgumentResolver.java index 0a7274fd07..28d5a4ce9d 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/PathVariableMethodArgumentResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/PathVariableMethodArgumentResolver.java @@ -92,7 +92,7 @@ public class PathVariableMethodArgumentResolver extends AbstractNamedValueSyncAr @Override protected void handleMissingValue(String name, MethodParameter parameter) { - throw new ServerErrorException(name, parameter); + throw new ServerErrorException(name, parameter, null); } @Override