ResponseStatusException reason is optional (with lazily constructed message)

Issue: SPR-15524

Author: jhoeller

spring-web/src/main/java/org/springframework/web/server/ResponseStatusException.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -16,6 +16,7 @@
 
 package org.springframework.web.server;
 
+import org.springframework.core.NestedExceptionUtils;
 import org.springframework.core.NestedRuntimeException;
 import org.springframework.http.HttpStatus;
 import org.springframework.util.Assert;
@@ -24,6 +25,7 @@
  * Base class for exceptions associated with specific HTTP response status codes.
  *
  * @author Rossen Stoyanchev
+ * @author Juergen Hoeller
  * @since 5.0
  */
 @SuppressWarnings("serial")
@@ -35,37 +37,56 @@ public class ResponseStatusException extends NestedRuntimeException {
 
 
 	/**
-	 * Constructor with a response code and a reason to add to the exception
+	 * Constructor with a response status.
+	 * @param status the HTTP status (required)
+	 */
+	public ResponseStatusException(HttpStatus status) {
+		this(status, null, null);
+	}
+
+	/**
+	 * Constructor with a response status and a reason to add to the exception
 	 * message as explanation.
+	 * @param status the HTTP status (required)
+	 * @param reason the associated reason (optional)
 	 */
 	public ResponseStatusException(HttpStatus status, String reason) {
 		this(status, reason, null);
 	}
 
 	/**
-	 * Constructor with a nested exception.
+	 * Constructor with a response status and a reason to add to the exception
+	 * message as explanation, as well as a nested exception.
+	 * @param status the HTTP status (required)
+	 * @param reason the associated reason (optional)
+	 * @param cause a nested exception (optional)
 	 */
 	public ResponseStatusException(HttpStatus status, String reason, Throwable cause) {
-		super("Request failure [status: " + status + ", reason: \"" + reason + "\"]", cause);
-		Assert.notNull(status, "'status' is required");
-		Assert.notNull(reason, "'reason' is required");
+		super(null, cause);
+		Assert.notNull(status, "HttpStatus is required");
 		this.status = status;
 		this.reason = reason;
 	}
 
 
 	/**
-	 * The HTTP status that fits the exception.
+	 * The HTTP status that fits the exception (never {@code null}).
 	 */
 	public HttpStatus getStatus() {
 		return this.status;
 	}
 
 	/**
-	 * The reason explaining the exception.
+	 * The reason explaining the exception (potentially {@code null} or empty).
 	 */
 	public String getReason() {
 		return this.reason;
 	}
 
+	@Override
+	public String getMessage() {
+		String msg = "Response status " + this.status + (this.reason != null ? " with reason \"" + reason + "\"" : "");
+		return NestedExceptionUtils.buildMessage(msg, getCause());
+	}
+
 }

spring-webflux/src/test/java/org/springframework/web/reactive/DispatcherHandlerErrorTests.java

@@ -51,10 +51,8 @@
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.startsWith;
 import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertThat;
-import static org.springframework.http.MediaType.APPLICATION_JSON;
+import static org.junit.Assert.*;
+import static org.springframework.http.MediaType.*;
 
 /**
  * Test the effect of exceptions at different stages of request processing by
@@ -87,8 +85,7 @@ public void noHandler() throws Exception {
 		StepVerifier.create(publisher)
 				.consumeErrorWith(error -> {
 					assertThat(error, instanceOf(ResponseStatusException.class));
-					assertThat(error.getMessage(),
-							is("Request failure [status: 404, reason: \"No matching handler\"]"));
+					assertThat(error.getMessage(), is("Response status 404 with reason \"No matching handler\""));
 				})
 				.verify();
 	}

spring-webflux/src/test/java/org/springframework/web/reactive/result/method/InvocableHandlerMethodTests.java

@@ -32,15 +32,11 @@
 import org.springframework.web.reactive.HandlerResult;
 import org.springframework.web.server.UnsupportedMediaTypeStatusException;
 
-import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
 import static org.mockito.Mockito.any;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-import static org.springframework.web.method.ResolvableMethod.on;
+import static org.mockito.Mockito.*;
+import static org.springframework.web.method.ResolvableMethod.*;
 
 /**
  * Unit tests for {@link InvocableHandlerMethod}.
@@ -109,7 +105,7 @@ public void resolverThrowsException() throws Exception {
 			fail("Expected UnsupportedMediaTypeStatusException");
 		}
 		catch (UnsupportedMediaTypeStatusException ex) {
-			assertThat(ex.getMessage(), is("Request failure [status: 415, reason: \"boo\"]"));
+			assertThat(ex.getMessage(), is("Response status 415 with reason \"boo\""));
 		}
 	}
 

spring-webflux/src/test/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMappingTests.java

@@ -48,24 +48,21 @@
 import org.springframework.web.reactive.BindingContext;
 import org.springframework.web.reactive.HandlerMapping;
 import org.springframework.web.reactive.HandlerResult;
-import org.springframework.web.reactive.result.method.RequestMappingInfo.BuilderConfiguration;
+import org.springframework.web.reactive.result.method.RequestMappingInfo.*;
 import org.springframework.web.server.MethodNotAllowedException;
 import org.springframework.web.server.NotAcceptableStatusException;
 import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.server.ServerWebInputException;
 import org.springframework.web.server.UnsupportedMediaTypeStatusException;
 import org.springframework.web.server.support.HttpRequestPathHelper;
 
-import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
-import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.get;
-import static org.springframework.web.bind.annotation.RequestMethod.GET;
-import static org.springframework.web.bind.annotation.RequestMethod.HEAD;
-import static org.springframework.web.bind.annotation.RequestMethod.OPTIONS;
-import static org.springframework.web.method.MvcAnnotationPredicates.getMapping;
-import static org.springframework.web.method.MvcAnnotationPredicates.requestMapping;
-import static org.springframework.web.method.ResolvableMethod.on;
-import static org.springframework.web.reactive.result.method.RequestMappingInfo.paths;
+import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.*;
+import static org.springframework.web.bind.annotation.RequestMethod.*;
+import static org.springframework.web.method.MvcAnnotationPredicates.*;
+import static org.springframework.web.method.ResolvableMethod.*;
+import static org.springframework.web.reactive.result.method.RequestMappingInfo.*;
 
 /**
  * Unit tests for {@link RequestMappingInfoHandlerMapping}.
@@ -165,8 +162,7 @@ public void getHandlerTestInvalidContentType() throws Exception {
 		Mono<Object> mono = this.handlerMapping.getHandler(exchange);
 
 		assertError(mono, UnsupportedMediaTypeStatusException.class,
-				ex -> assertEquals("Request failure [status: 415, " +
-						"reason: \"Invalid mime type \"bogus\": does not contain '/'\"]",
+				ex -> assertEquals("Response status 415 with reason \"Invalid mime type \"bogus\": does not contain '/'\"",
 						ex.getMessage()));
 	}
 

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/annotation/ResponseStatusExceptionResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -49,7 +49,8 @@
  * @author Rossen Stoyanchev
  * @author Sam Brannen
  * @since 3.0
- * @see AnnotatedElementUtils#findMergedAnnotation
+ * @see ResponseStatus
+ * @see ResponseStatusException
  */
 public class ResponseStatusExceptionResolver extends AbstractHandlerExceptionResolver implements MessageSourceAware {
 
@@ -99,9 +100,8 @@ protected ModelAndView doResolveException(HttpServletRequest request, HttpServle
 	 * @param ex the exception
 	 * @return an empty ModelAndView, i.e. exception resolved
 	 */
-	protected ModelAndView resolveResponseStatus(ResponseStatus responseStatus,
-			HttpServletRequest request, HttpServletResponse response, Object handler, Exception ex)
-			throws Exception {
+	protected ModelAndView resolveResponseStatus(ResponseStatus responseStatus, HttpServletRequest request,
+			HttpServletResponse response, Object handler, Exception ex) throws Exception {
 
 		int statusCode = responseStatus.code().value();
 		String reason = responseStatus.reason();
@@ -125,8 +125,7 @@ protected ModelAndView resolveResponseStatusException(ResponseStatusException ex
 
 		int statusCode = ex.getStatus().value();
 		String reason = ex.getReason();
-		applyStatusAndReason(statusCode, reason, response);
-		return new ModelAndView();
+		return applyStatusAndReason(statusCode, reason, response);
 	}
 
 	/**
@@ -135,19 +134,22 @@ protected ModelAndView resolveResponseStatusException(ResponseStatusException ex
 	 * {@link HttpServletResponse#sendError(int)} or
 	 * {@link HttpServletResponse#sendError(int, String)} if there is a reason
 	 * and then returns an empty ModelAndView.
+	 * @param statusCode the HTTP status code
+	 * @param reason the associated reason (may be {@code null} or empty)
+	 * @param response current HTTP response
 	 * @since 5.0
 	 */
 	protected ModelAndView applyStatusAndReason(int statusCode, String reason, HttpServletResponse response)
 			throws IOException {
 
-		if (this.messageSource != null) {
-			reason = this.messageSource.getMessage(reason, null, reason, LocaleContextHolder.getLocale());
-		}
 		if (!StringUtils.hasLength(reason)) {
 			response.sendError(statusCode);
 		}
 		else {
-			response.sendError(statusCode, reason);
+			String resolvedReason = (this.messageSource != null ?
+					this.messageSource.getMessage(reason, null, reason, LocaleContextHolder.getLocale()) :
+					reason);
+			response.sendError(statusCode, resolvedReason);
 		}
 		return new ModelAndView();
 	}

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/annotation/ResponseStatusExceptionResolverTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -111,23 +111,28 @@ public void nestedException() throws Exception {
 		Exception cause = new StatusCodeAndReasonMessageException();
 		TypeMismatchException ex = new TypeMismatchException("value", ITestBean.class, cause);
 		ModelAndView mav = exceptionResolver.resolveException(request, response, null, ex);
-		assertResolved(mav, 410, null);
+		assertResolved(mav, 410, "gone.reason");
 	}
 
 	@Test
 	public void responseStatusException() throws Exception {
-		ResponseStatusException ex = new ResponseStatusException(HttpStatus.BAD_REQUEST, "The reason");
+		ResponseStatusException ex = new ResponseStatusException(HttpStatus.BAD_REQUEST);
 		ModelAndView mav = exceptionResolver.resolveException(request, response, null, ex);
 		assertResolved(mav, 400, null);
 	}
 
+	@Test  // SPR-15524
+	public void responseStatusExceptionWithReason() throws Exception {
+		ResponseStatusException ex = new ResponseStatusException(HttpStatus.BAD_REQUEST, "The reason");
+		ModelAndView mav = exceptionResolver.resolveException(request, response, null, ex);
+		assertResolved(mav, 400, "The reason");
+	}
+
 
 	private void assertResolved(ModelAndView mav, int status, String reason) {
 		assertTrue("No Empty ModelAndView returned", mav != null && mav.isEmpty());
 		assertEquals(status, response.getStatus());
-		if (reason != null) {
-			assertEquals(reason, response.getErrorMessage());
-		}
+		assertEquals(reason, response.getErrorMessage());
 		assertTrue(response.isCommitted());
 	}