Skip to content

Commit

Permalink
Improve mapping any Exception to ErrorResponse
Browse files Browse the repository at this point in the history 
Add protected, convenience method in ResponseEntityExceptionHandler
to create a ProblemDetail for any exception, along with a
MessageSource lookup for the "detail" field.

Closes gh-29384
  • Loading branch information
@rstoyanchev
rstoyanchev committed Nov 1, 2022
1 parent 210019c commit 506fbe5
Show file tree
Hide file tree
Showing 5 changed files with 171 additions and 52 deletions.
49 changes: 49 additions & 0 deletions spring-web/src/main/java/org/springframework/web/ErrorResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,24 @@ default Object[] getDetailMessageArguments(MessageSource messageSource, Locale l
return getDetailMessageArguments();
}

/**
* Resolve the {@link #getDetailMessageCode() detailMessageCode} through the
* given {@link MessageSource}, and if found, update the "detail" field.
* @param messageSource the {@code MessageSource} to use for the lookup
* @param locale the {@code Locale} to use for the lookup
*/
default ProblemDetail updateAndGetBody(@Nullable MessageSource messageSource, Locale locale) {
if (messageSource != null) {
Object[] arguments = getDetailMessageArguments(messageSource, locale);
String detail = messageSource.getMessage(getDetailMessageCode(), arguments, null, locale);
if (detail != null) {
getBody().setDetail(detail);
}
}
return getBody();
}


/**
* Build a message code for the given exception type, which consists of
* {@code "problemDetail."} followed by the full {@link Class#getName() class name}.
Expand All @@ -105,4 +123,35 @@ static String getDefaultDetailMessageCode(Class<?> exceptionType, @Nullable Stri
return "problemDetail." + exceptionType.getName() + (suffix != null ? "." + suffix : "");
}

/**
* Map the given Exception to an {@link ErrorResponse}.
* @param ex the Exception, mostly to derive message codes, if not provided
* @param status the response status to use
* @param headers optional headers to add to the response
* @param defaultDetail default value for the "detail" field
* @param detailMessageCode the code to use to look up the "detail" field
* through a {@code MessageSource}, falling back on
* {@link #getDefaultDetailMessageCode(Class, String)}
* @param detailMessageArguments the arguments to go with the detailMessageCode
* @return the created {@code ErrorResponse} instance
*/
static ErrorResponse createFor(
Exception ex, HttpStatusCode status, @Nullable HttpHeaders headers,
String defaultDetail, @Nullable String detailMessageCode, @Nullable Object[] detailMessageArguments) {

if (detailMessageCode == null) {
detailMessageCode = ErrorResponse.getDefaultDetailMessageCode(ex.getClass(), null);
}

ErrorResponseException errorResponse = new ErrorResponseException(
status, ProblemDetail.forStatusAndDetail(status, defaultDetail), null,
detailMessageCode, detailMessageArguments);

if (headers != null) {
errorResponse.getHeaders().putAll(headers);
}

return errorResponse;
}

}
49 changes: 33 additions & 16 deletions ...springframework/web/reactive/result/method/annotation/ResponseEntityExceptionHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,35 @@ protected Mono<ResponseEntity<Object>> handleErrorResponseException(
return handleExceptionInternal(ex, null, headers, status, exchange);
}

/**
* Convenience method to create a {@link ProblemDetail} for any exception
* that doesn't implement {@link ErrorResponse}, also performing a
* {@link MessageSource} lookup for the "detail" field.
* @param ex the exception being handled
* @param status the status to associate with the exception
* @param defaultDetail default value for the "detail" field
* @param detailMessageCode the code to use to look up the "detail" field
* through a {@code MessageSource}, falling back on
* {@link ErrorResponse#getDefaultDetailMessageCode(Class, String)}
* @param detailMessageArguments the arguments to go with the detailMessageCode
* @return the created {@code ProblemDetail} instance
*/
protected ProblemDetail createProblemDetail(
Exception ex, HttpStatusCode status, @Nullable HttpHeaders headers,
String defaultDetail, @Nullable String detailMessageCode, @Nullable Object[] detailMessageArguments,
ServerWebExchange exchange) {

ErrorResponse response = ErrorResponse.createFor(
ex, status, headers, defaultDetail, detailMessageCode, detailMessageArguments);

return response.updateAndGetBody(this.messageSource, getLocale(exchange));
}

private static Locale getLocale(ServerWebExchange exchange) {
Locale locale = exchange.getLocaleContext().getLocale();
return (locale != null ? locale : Locale.getDefault());
}

/**
* Internal handler method that all others in this class delegate to, for
* common handling, and for the creation of a {@link ResponseEntity}.
Expand All @@ -311,33 +340,20 @@ protected Mono<ResponseEntity<Object>> handleErrorResponseException(
* @return a {@code Mono} with the {@code ResponseEntity} for the response
*/
protected Mono<ResponseEntity<Object>> handleExceptionInternal(
Exception ex, @Nullable Object body, HttpHeaders headers, HttpStatusCode status,
Exception ex, @Nullable Object body, @Nullable HttpHeaders headers, HttpStatusCode status,
ServerWebExchange exchange) {

if (exchange.getResponse().isCommitted()) {
return Mono.error(ex);
}

if (body == null && ex instanceof ErrorResponse errorResponse) {
body = resolveDetailViaMessageSource(errorResponse, exchange.getLocaleContext().getLocale());
body = errorResponse.updateAndGetBody(this.messageSource, getLocale(exchange));
}

return createResponseEntity(body, headers, status, exchange);
}

private ProblemDetail resolveDetailViaMessageSource(ErrorResponse response, @Nullable Locale locale) {
ProblemDetail body = response.getBody();
if (this.messageSource != null) {
locale = (locale != null ? locale : Locale.getDefault());
Object[] arguments = response.getDetailMessageArguments(this.messageSource, locale);
String detail = this.messageSource.getMessage(response.getDetailMessageCode(), arguments, null, locale);
if (detail != null) {
body.setDetail(detail);
}
}
return body;
}

/**
* Create the {@link ResponseEntity} to use from the given body, headers,
* and statusCode. Subclasses can override this method to inspect and possibly
Expand All @@ -351,7 +367,8 @@ private ProblemDetail resolveDetailViaMessageSource(ErrorResponse response, @Nul
* @since 6.0
*/
protected Mono<ResponseEntity<Object>> createResponseEntity(
@Nullable Object body, HttpHeaders headers, HttpStatusCode status, ServerWebExchange exchange) {
@Nullable Object body, @Nullable HttpHeaders headers, HttpStatusCode status,
ServerWebExchange exchange) {

return Mono.just(new ResponseEntity<>(body, headers, status));
}
Expand Down
31 changes: 30 additions & 1 deletion ...gframework/web/reactive/result/method/annotation/ResponseEntityExceptionHandlerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
*/
public class ResponseEntityExceptionHandlerTests {

private final ResponseEntityExceptionHandler exceptionHandler = new GlobalExceptionHandler();
private final GlobalExceptionHandler exceptionHandler = new GlobalExceptionHandler();

private final MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/").build());

Expand Down Expand Up @@ -149,6 +149,29 @@ void errorResponseProblemDetailViaMessageSource() {
"Content-Type application/json not supported. Supported: [application/atom+xml, application/xml]");
}

@Test
void customExceptionToProblemDetailViaMessageSource() {

Locale locale = Locale.UK;
LocaleContextHolder.setLocale(locale);

StaticMessageSource messageSource = new StaticMessageSource();
messageSource.addMessage(
"problemDetail." + IllegalStateException.class.getName(), locale,
"Invalid state: {0}");

this.exceptionHandler.setMessageSource(messageSource);

MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")
.acceptLanguageAsLocales(locale).build());

ResponseEntity<?> responseEntity =
this.exceptionHandler.handleException(new IllegalStateException(), exchange).block();

ProblemDetail body = (ProblemDetail) responseEntity.getBody();
assertThat(body.getDetail()).isEqualTo("Invalid state: A");
}


@SuppressWarnings("unchecked")
private ResponseEntity<ProblemDetail> testException(ErrorResponseException exception) {
Expand Down Expand Up @@ -247,6 +270,12 @@ protected Mono<ResponseEntity<Object>> handleErrorResponseException(

return handleAndSetTypeToExceptionName(ex, headers, status, exchange);
}

public Mono<ResponseEntity<Object>> handleException(IllegalStateException ex, ServerWebExchange exchange) {
HttpStatus status = HttpStatus.INTERNAL_SERVER_ERROR;
ProblemDetail body = createProblemDetail(ex, status, null, ex.getMessage(), null, new Object[] {"A"}, exchange);
return handleExceptionInternal(ex, body, null, status, exchange);
}
}

}
69 changes: 34 additions & 35 deletions ...org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

package org.springframework.web.servlet.mvc.method.annotation;

import java.util.Locale;

import jakarta.servlet.http.HttpServletResponse;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
Expand Down Expand Up @@ -395,9 +393,8 @@ protected ResponseEntity<Object> handleConversionNotSupported(
ConversionNotSupportedException ex, HttpHeaders headers, HttpStatusCode status, WebRequest request) {

Object[] args = {ex.getPropertyName(), ex.getValue()};

ProblemDetail body = resolveDetailViaMessageSource(
status, args, "Failed to convert '" + args[0] + "' with value: '" + args[1] + "'");
String defaultDetail = "Failed to convert '" + args[0] + "' with value: '" + args[1] + "'";
ProblemDetail body = createProblemDetail(ex, status, headers, defaultDetail, null, args, request);

return handleExceptionInternal(ex, body, headers, status, request);
}
Expand All @@ -420,9 +417,9 @@ protected ResponseEntity<Object> handleTypeMismatch(
TypeMismatchException ex, HttpHeaders headers, HttpStatusCode status, WebRequest request) {

Object[] args = {ex.getPropertyName(), ex.getValue()};

ProblemDetail body = resolveDetailViaMessageSource(
status, args, "Failed to convert '" + args[0] + "' with value: '" + args[1] + "'");
String defaultDetail = "Failed to convert '" + args[0] + "' with value: '" + args[1] + "'";
String messageCode = ErrorResponse.getDefaultDetailMessageCode(TypeMismatchException.class, null);
ProblemDetail body = createProblemDetail(ex, status, headers, defaultDetail, messageCode, args, request);

return handleExceptionInternal(ex, body, headers, status, request);
}
Expand All @@ -444,7 +441,7 @@ protected ResponseEntity<Object> handleTypeMismatch(
protected ResponseEntity<Object> handleHttpMessageNotReadable(
HttpMessageNotReadableException ex, HttpHeaders headers, HttpStatusCode status, WebRequest request) {

ProblemDetail body = resolveDetailViaMessageSource(status, null, "Failed to read request");
ProblemDetail body = createProblemDetail(ex, status, headers, "Failed to read request", null, null, request);
return handleExceptionInternal(ex, body, headers, status, request);
}

Expand All @@ -465,7 +462,7 @@ protected ResponseEntity<Object> handleHttpMessageNotReadable(
protected ResponseEntity<Object> handleHttpMessageNotWritable(
HttpMessageNotWritableException ex, HttpHeaders headers, HttpStatusCode status, WebRequest request) {

ProblemDetail body = resolveDetailViaMessageSource(status, null, "Failed to write request");
ProblemDetail body = createProblemDetail(ex, status, headers, "Failed to write request", null, null, request);
return handleExceptionInternal(ex, body, headers, status, request);
}

Expand All @@ -492,6 +489,32 @@ protected ResponseEntity<Object> handleBindException(
return handleExceptionInternal(ex, body, headers, status, request);
}

/**
* Convenience method to create a {@link ProblemDetail} for any exception
* that doesn't implement {@link ErrorResponse}, also performing a
* {@link MessageSource} lookup for the "detail" field.
* @param ex the exception being handled
* @param status the status to associate with the exception
* @param defaultDetail default value for the "detail" field
* @param detailMessageCode the code to use to look up the "detail" field
* through a {@code MessageSource}, falling back on
* {@link ErrorResponse#getDefaultDetailMessageCode(Class, String)}
* @param detailMessageArguments the arguments to go with the detailMessageCode
* @param request the current request
* @return the created {@code ProblemDetail} instance
* @since 6.0
*/
protected ProblemDetail createProblemDetail(
Exception ex, HttpStatusCode status, @Nullable HttpHeaders headers,
String defaultDetail, @Nullable String detailMessageCode, @Nullable Object[] detailMessageArguments,
WebRequest request) {

ErrorResponse errorResponse = ErrorResponse.createFor(
ex, status, headers, defaultDetail, detailMessageCode, detailMessageArguments);

return errorResponse.updateAndGetBody(this.messageSource, LocaleContextHolder.getLocale());
}

/**
* Internal handler method that all others in this class delegate to, for
* common handling, and for the creation of a {@link ResponseEntity}.
Expand Down Expand Up @@ -530,36 +553,12 @@ protected ResponseEntity<Object> handleExceptionInternal(
}

if (body == null && ex instanceof ErrorResponse errorResponse) {
body = resolveDetailViaMessageSource(errorResponse);
body = errorResponse.updateAndGetBody(this.messageSource, LocaleContextHolder.getLocale());
}

return createResponseEntity(body, headers, statusCode, request);
}

// For non-Web exceptions
private ProblemDetail resolveDetailViaMessageSource(
HttpStatusCode status, @Nullable Object[] arguments, String defaultDetail) {

ProblemDetail body = ProblemDetail.forStatusAndDetail(status, defaultDetail);
ErrorResponseException errorResponseEx = new ErrorResponseException(status, body, null, null, arguments);
body = resolveDetailViaMessageSource(errorResponseEx);
return body;
}

// For ErrorResponse exceptions
private ProblemDetail resolveDetailViaMessageSource(ErrorResponse response) {
ProblemDetail body = response.getBody();
if (this.messageSource != null) {
Locale locale = LocaleContextHolder.getLocale();
Object[] arguments = response.getDetailMessageArguments(this.messageSource, locale);
String detail = this.messageSource.getMessage(response.getDetailMessageCode(), arguments, null, locale);
if (detail != null) {
body.setDetail(detail);
}
}
return body;
}

/**
* Create the {@link ResponseEntity} to use from the given body, headers,
* and statusCode. Subclasses can override this method to inspect and possibly
Expand Down
25 changes: 25 additions & 0 deletions ...pringframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandlerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.springframework.web.servlet.mvc.method.annotation;

import java.beans.PropertyChangeEvent;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -192,6 +193,30 @@ public void typeMismatch() {
testException(new TypeMismatchException("foo", String.class));
}

@Test
public void typeMismatchWithProblemDetailViaMessageSource() {
Locale locale = Locale.UK;
LocaleContextHolder.setLocale(locale);

try {
StaticMessageSource messageSource = new StaticMessageSource();
messageSource.addMessage(
"problemDetail." + TypeMismatchException.class.getName(), locale,
"Failed to set {0} to value: {1}");

this.exceptionHandler.setMessageSource(messageSource);

ResponseEntity<?> entity = testException(
new TypeMismatchException(new PropertyChangeEvent(this, "name", "John", "James"), String.class));

ProblemDetail body = (ProblemDetail) entity.getBody();
assertThat(body.getDetail()).isEqualTo("Failed to set name to value: James");
}
finally {
LocaleContextHolder.resetLocaleContext();
}
}

@Test
@SuppressWarnings("deprecation")
public void httpMessageNotReadable() {
Expand Down

0 comments on commit 506fbe5

Please sign in to comment.