Add DispatcherHandlerErrorTests

The tests demonstrate failures at various phases of request processing
and the resulting error signals.

spring-web-reactive/src/main/java/org/springframework/core/codec/support/AbstractDecoder.java

@@ -44,6 +44,9 @@ public abstract class AbstractDecoder<T> implements Decoder<T> {
 
 	@Override
 	public boolean canDecode(ResolvableType type, MimeType mimeType, Object... hints) {
+		if (mimeType == null) {
+			return true;
+		}
 		for (MimeType supportedMimeType : this.supportedMimeTypes) {
 			if (supportedMimeType.isCompatibleWith(mimeType)) {
 				return true;

spring-web-reactive/src/main/java/org/springframework/core/codec/support/AbstractEncoder.java

@@ -44,6 +44,9 @@ public abstract class AbstractEncoder<T> implements Encoder<T> {
 
 	@Override
 	public boolean canEncode(ResolvableType type, MimeType mimeType, Object... hints) {
+		if (mimeType == null) {
+			return true;
+		}
 		for (MimeType supportedMimeType : this.supportedMimeTypes) {
 			if (supportedMimeType.isCompatibleWith(mimeType)) {
 				return true;

spring-web-reactive/src/main/java/org/springframework/web/reactive/DispatcherHandler.java

@@ -128,7 +128,7 @@ public class DispatcherHandler implements HttpHandler, ApplicationContextAware {
 				return resultHandler;
 			}
 		}
-		throw new IllegalStateException("No HandlerResultHandler: " + handlerResult.getValue());
+		throw new IllegalStateException("No HandlerResultHandler for " + handlerResult.getValue());
 	}
 
 

spring-web-reactive/src/main/java/org/springframework/web/reactive/method/annotation/RequestMappingHandlerAdapter.java

@@ -26,6 +26,7 @@ import reactor.Publishers;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.core.ResolvableType;
 import org.springframework.core.convert.ConversionService;
+import org.springframework.core.convert.support.DefaultConversionService;
 import org.springframework.http.server.reactive.ServerHttpRequest;
 import org.springframework.http.server.reactive.ServerHttpResponse;
 import org.springframework.core.codec.support.ByteBufferDecoder;
@@ -33,6 +34,7 @@ import org.springframework.core.codec.Decoder;
 import org.springframework.core.codec.support.JacksonJsonDecoder;
 import org.springframework.core.codec.support.JsonObjectDecoder;
 import org.springframework.core.codec.support.StringDecoder;
+import org.springframework.util.ObjectUtils;
 import org.springframework.web.reactive.HandlerAdapter;
 import org.springframework.web.reactive.HandlerResult;
 import org.springframework.web.reactive.method.HandlerMethodArgumentResolver;
@@ -45,9 +47,9 @@ import org.springframework.web.method.HandlerMethod;
  */
 public class RequestMappingHandlerAdapter implements HandlerAdapter, InitializingBean {
 
-	private List<HandlerMethodArgumentResolver> argumentResolvers;
+	private final List<HandlerMethodArgumentResolver> argumentResolvers = new ArrayList<>();
 
-	private ConversionService conversionService;
+	private ConversionService conversionService = new DefaultConversionService();
 
 
 	public void setArgumentResolvers(List<HandlerMethodArgumentResolver> resolvers) {
@@ -70,12 +72,11 @@ public class RequestMappingHandlerAdapter implements HandlerAdapter, Initializin
 
 	@Override
 	public void afterPropertiesSet() throws Exception {
-		if (this.argumentResolvers == null) {
+		if (ObjectUtils.isEmpty(this.argumentResolvers)) {
 
 			List<Decoder<?>> decoders = Arrays.asList(new ByteBufferDecoder(),
 					new StringDecoder(), new JacksonJsonDecoder(new JsonObjectDecoder()));
 
-			this.argumentResolvers = new ArrayList<>();
 			this.argumentResolvers.add(new RequestParamArgumentResolver());
 			this.argumentResolvers.add(new RequestBodyArgumentResolver(decoders, this.conversionService));
 		}

spring-web-reactive/src/main/java/org/springframework/web/reactive/method/annotation/ResponseBodyResultHandler.java

@@ -135,10 +135,20 @@ public class ResponseBodyResultHandler implements HandlerResultHandler, Ordered
 			return Publishers.empty();
 		}
 
+		Publisher<?> publisher;
+		ResolvableType elementType;
 		ResolvableType returnType = result.getValueType();
+		if (this.conversionService.canConvert(returnType.getRawClass(), Publisher.class)) {
+			publisher = this.conversionService.convert(value, Publisher.class);
+			elementType = returnType.getGeneric(0);
+		}
+		else {
+			publisher = Publishers.just(value);
+			elementType = returnType;
+		}
 
 		List<MediaType> requestedMediaTypes = getAcceptableMediaTypes(request);
-		List<MediaType> producibleMediaTypes = getProducibleMediaTypes(returnType);
+		List<MediaType> producibleMediaTypes = getProducibleMediaTypes(elementType);
 
 		if (producibleMediaTypes.isEmpty()) {
 			producibleMediaTypes.add(MediaType.ALL);
@@ -172,16 +182,6 @@ public class ResponseBodyResultHandler implements HandlerResultHandler, Ordered
 		}
 
 		if (selectedMediaType != null) {
-			Publisher<?> publisher;
-			ResolvableType elementType;
-			if (this.conversionService.canConvert(returnType.getRawClass(), Publisher.class)) {
-				publisher = this.conversionService.convert(value, Publisher.class);
-				elementType = returnType.getGeneric(0);
-			}
-			else {
-				publisher = Publishers.just(value);
-				elementType = returnType;
-			}
 			Encoder<?> encoder = resolveEncoder(elementType, selectedMediaType);
 			if (encoder != null) {
 				response.getHeaders().setContentType(selectedMediaType);

spring-web-reactive/src/main/resources/log4j.properties

@@ -1,6 +1,6 @@
 log4j.rootCategory=WARN, stdout
 
-log4j.logger.org.springframework.reactive=DEBUG
+log4j.logger.org.springframework.http=DEBUG
 log4j.logger.org.springframework.web=DEBUG
 log4j.logger.reactor=INFO
 

spring-web-reactive/src/test/java/org/springframework/http/server/reactive/ErrorHandlingHttpHandlerTests.java

@@ -115,25 +115,24 @@ public class ErrorHandlingHttpHandlerTests {
 
 	private static class TestHttpHandler implements HttpHandler {
 
-		private final Throwable exception;
+		private final RuntimeException exception;
 
 		private final boolean raise;
 
 
-		public TestHttpHandler(Throwable exception) {
+		public TestHttpHandler(RuntimeException exception) {
 			this(exception, false);
 		}
 
-		public TestHttpHandler(Throwable exception, boolean raise) {
+		public TestHttpHandler(RuntimeException exception, boolean raise) {
 			this.exception = exception;
 			this.raise = raise;
-			assertTrue(exception instanceof RuntimeException || !this.raise);
 		}
 
 		@Override
 		public Publisher<Void> handle(ServerHttpRequest request, ServerHttpResponse response) {
 			if (this.raise) {
-				throw (RuntimeException) exception;
+				throw this.exception;
 			}
 			return Publishers.error(this.exception);
 		}

spring-web-reactive/src/test/java/org/springframework/http/server/reactive/FilterChainHttpHandlerTests.java

@@ -16,14 +16,26 @@
 package org.springframework.http.server.reactive;
 
 
+import java.util.Arrays;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.log4j.spi.LoggerFactory;
 import org.junit.Before;
 import org.junit.Test;
 import org.reactivestreams.Publisher;
 import reactor.Publishers;
+import reactor.core.publisher.PublisherFactory;
+import reactor.fn.timer.Timer;
 import reactor.rx.Streams;
 
+import org.springframework.scheduling.TaskScheduler;
+import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler;
+import org.springframework.util.ObjectUtils;
+
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
@@ -33,6 +45,9 @@ import static org.mockito.Mockito.mock;
  */
 public class FilterChainHttpHandlerTests {
 
+	private static Log logger = LogFactory.getLog(FilterChainHttpHandlerTests.class);
+
+
 	private ServerHttpRequest request;
 
 	private ServerHttpResponse response;
@@ -89,10 +104,24 @@ public class FilterChainHttpHandlerTests {
 		assertFalse(handler.invoked());
 	}
 
+	@Test
+	public void asyncFilter() throws Exception {
+		StubHandler handler = new StubHandler();
+		AsyncFilter filter = new AsyncFilter();
+		FilterChainHttpHandler filterHandler = new FilterChainHttpHandler(handler, filter);
+
+		Publisher<Void> voidPublisher = filterHandler.handle(this.request, this.response);
+		Streams.wrap(voidPublisher).toList().await(10, TimeUnit.SECONDS);
+
+		assertTrue(filter.invoked());
+		assertTrue(handler.invoked());
+	}
+
+
 
 	private static class TestFilter implements HttpFilter {
 
-		private boolean invoked;
+		private volatile boolean invoked;
 
 
 		public boolean invoked() {
@@ -124,9 +153,25 @@ public class FilterChainHttpHandlerTests {
 		}
 	}
 
+	private static class AsyncFilter extends TestFilter {
+
+		@Override
+		public Publisher<Void> doFilter(ServerHttpRequest req, ServerHttpResponse res, HttpFilterChain chain) {
+			return Publishers.concatMap(doAsyncWork(), asyncResult -> {
+				logger.debug("Async result: " + asyncResult);
+				return chain.filter(req, res);
+			});
+		}
+
+		private Publisher<String> doAsyncWork() {
+			return Publishers.just("123");
+		}
+	}
+
+
 	private static class StubHandler implements HttpHandler {
 
-		private boolean invoked;
+		private volatile boolean invoked;
 
 		public boolean invoked() {
 			return this.invoked;
@@ -134,6 +179,7 @@ public class FilterChainHttpHandlerTests {
 
 		@Override
 		public Publisher<Void> handle(ServerHttpRequest req, ServerHttpResponse res) {
+			logger.trace("StubHandler invoked.");
 			this.invoked = true;
 			return Publishers.empty();
 		}

spring-web-reactive/src/test/java/org/springframework/http/server/reactive/MockServerHttpResponse.java

@@ -57,7 +57,7 @@ public class MockServerHttpResponse implements ServerHttpResponse {
 	@Override
 	public Publisher<Void> setBody(Publisher<ByteBuffer> body) {
 		this.body = body;
-		return Publishers.empty();
+		return Publishers.concatMap(body, b -> Publishers.empty());
 	}
 
 	public Publisher<ByteBuffer> getBody() {

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

@@ -0,0 +1,282 @@
+/*
+ * Copyright 2002-2015 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.web.reactive;
+
+import java.net.URI;
+import java.nio.ByteBuffer;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.reactivestreams.Publisher;
+import reactor.Publishers;
+import reactor.rx.Streams;
+import reactor.rx.action.Signal;
+
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.core.codec.Encoder;
+import org.springframework.core.codec.support.StringEncoder;
+import org.springframework.core.convert.support.DefaultConversionService;
+import org.springframework.http.HttpMethod;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.MediaType;
+import org.springframework.http.server.reactive.ErrorHandlingHttpHandler;
+import org.springframework.http.server.reactive.FilterChainHttpHandler;
+import org.springframework.http.server.reactive.HttpExceptionHandler;
+import org.springframework.http.server.reactive.HttpFilter;
+import org.springframework.http.server.reactive.HttpFilterChain;
+import org.springframework.http.server.reactive.HttpHandler;
+import org.springframework.http.server.reactive.MockServerHttpRequest;
+import org.springframework.http.server.reactive.MockServerHttpResponse;
+import org.springframework.http.server.reactive.ServerHttpRequest;
+import org.springframework.http.server.reactive.ServerHttpResponse;
+import org.springframework.stereotype.Controller;
+import org.springframework.web.HttpMediaTypeNotAcceptableException;
+import org.springframework.web.bind.annotation.RequestBody;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.ResponseBody;
+import org.springframework.web.reactive.method.annotation.RequestMappingHandlerAdapter;
+import org.springframework.web.reactive.method.annotation.RequestMappingHandlerMapping;
+import org.springframework.web.reactive.method.annotation.ResponseBodyResultHandler;
+
+import static org.hamcrest.CoreMatchers.startsWith;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThat;
+
+/**
+ * Test the effect of exceptions at different stages of request processing by
+ * checking the error signals on the completion publisher.
+ *
+ * @author Rossen Stoyanchev
+ */
+@SuppressWarnings({"ThrowableResultOfMethodCallIgnored", "ThrowableInstanceNeverThrown"})
+public class DispatcherHandlerErrorTests {
+
+	public static final IllegalStateException EXCEPTION = new IllegalStateException("boo");
+
+
+	private DispatcherHandler dispatcherHandler;
+
+	private MockServerHttpRequest request;
+
+	private MockServerHttpResponse response;
+
+
+	@Before
+	public void setUp() throws Exception {
+		AnnotationConfigApplicationContext appContext = new AnnotationConfigApplicationContext();
+		appContext.register(TestConfig.class);
+		appContext.refresh();
+
+		this.dispatcherHandler = new DispatcherHandler();
+		this.dispatcherHandler.setApplicationContext(appContext);
+
+		this.request = new MockServerHttpRequest(HttpMethod.GET, new URI("/"));
+		this.response = new MockServerHttpResponse();
+	}
+
+
+	@Test
+	public void noHandler() throws Exception {
+		this.request.setUri(new URI("/does-not-exist"));
+
+		Publisher<Void> publisher = this.dispatcherHandler.handle(this.request, this.response);
+		Throwable ex = awaitErrorSignal(publisher);
+
+		assertEquals(HandlerNotFoundException.class, ex.getClass());
+	}
+
+	@Test
+	public void noResolverForArgument() throws Exception {
+		this.request.setUri(new URI("/uknown-argument-type"));
+
+		Publisher<Void> publisher = this.dispatcherHandler.handle(this.request, this.response);
+		Throwable ex = awaitErrorSignal(publisher);
+
+		assertEquals(IllegalStateException.class, ex.getClass());
+		assertThat(ex.getMessage(), startsWith("No resolver for argument [0]"));
+	}
+
+	@Test
+	public void controllerMethodError() throws Exception {
+		this.request.setUri(new URI("/error-signal"));
+
+		Publisher<Void> publisher = this.dispatcherHandler.handle(this.request, this.response);
+		Throwable ex = awaitErrorSignal(publisher);
+
+		assertSame(EXCEPTION, ex);
+	}
+
+	@Test
+	public void controllerMethodWithThrownException() throws Exception {
+		this.request.setUri(new URI("/raise-exception"));
+
+		Publisher<Void> publisher = this.dispatcherHandler.handle(this.request, this.response);
+		Throwable ex = awaitErrorSignal(publisher);
+
+		assertSame(EXCEPTION, ex);
+	}
+
+	@Test
+	public void noHandlerResultHandler() throws Exception {
+		this.request.setUri(new URI("/unknown-return-type"));
+
+		Publisher<Void> publisher = this.dispatcherHandler.handle(this.request, this.response);
+		Throwable ex = awaitErrorSignal(publisher);
+
+		assertEquals(IllegalStateException.class, ex.getClass());
+		assertThat(ex.getMessage(), startsWith("No HandlerResultHandler"));
+	}
+
+	@Test
+	public void notAcceptable() throws Exception {
+		this.request.setUri(new URI("/request-body"));
+		this.request.getHeaders().setAccept(Collections.singletonList(MediaType.APPLICATION_JSON));
+		this.request.setBody(Publishers.just(ByteBuffer.wrap("body".getBytes("UTF-8"))));
+
+		Publisher<Void> publisher = this.dispatcherHandler.handle(this.request, this.response);
+		Throwable ex = awaitErrorSignal(publisher);
+
+		assertEquals(HttpMediaTypeNotAcceptableException.class, ex.getClass());
+	}
+
+	@Test
+	public void requestBodyError() throws Exception {
+		this.request.setUri(new URI("/request-body"));
+		this.request.setBody(Publishers.error(EXCEPTION));
+
+		Publisher<Void> publisher = this.dispatcherHandler.handle(this.request, this.response);
+		Throwable ex = awaitErrorSignal(publisher);
+
+		assertSame(EXCEPTION, ex);
+	}
+
+	@Test
+	public void dispatcherHandlerWithHttpExceptionHandler() throws Exception {
+		this.request.setUri(new URI("/uknown-argument-type"));
+
+		HttpExceptionHandler exceptionHandler = new ServerError500ExceptionHandler();
+		HttpHandler httpHandler = new ErrorHandlingHttpHandler(this.dispatcherHandler, exceptionHandler);
+		Publisher<Void> publisher = httpHandler.handle(this.request, this.response);
+
+		Streams.wrap(publisher).toList().await(5, TimeUnit.SECONDS);
+		assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, this.response.getStatus());
+	}
+
+	@Test
+	public void filterChainWithHttpExceptionHandler() throws Exception {
+		this.request.setUri(new URI("/uknown-argument-type"));
+
+		HttpHandler httpHandler;
+		httpHandler = new FilterChainHttpHandler(this.dispatcherHandler, new TestHttpFilter());
+		httpHandler = new ErrorHandlingHttpHandler(httpHandler, new ServerError500ExceptionHandler());
+		Publisher<Void> publisher = httpHandler.handle(this.request, this.response);
+
+		Streams.wrap(publisher).toList().await(5, TimeUnit.SECONDS);
+		assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, this.response.getStatus());
+	}
+
+
+	private Throwable awaitErrorSignal(Publisher<?> publisher) throws Exception {
+		Signal<?> signal = Streams.wrap(publisher).materialize().toList().await(5, TimeUnit.SECONDS).get(0);
+		assertEquals("Unexpected signal: " + signal, Signal.Type.ERROR, signal.getType());
+		return signal.getThrowable();
+	}
+
+
+	@Configuration
+	@SuppressWarnings("unused")
+	static class TestConfig {
+
+		@Bean
+		public RequestMappingHandlerMapping handlerMapping() {
+			return new RequestMappingHandlerMapping();
+		}
+
+		@Bean
+		public RequestMappingHandlerAdapter handlerAdapter() {
+			return new RequestMappingHandlerAdapter();
+		}
+
+		@Bean
+		public ResponseBodyResultHandler resultHandler() {
+			List<Encoder<?>> encoders = Collections.singletonList(new StringEncoder());
+			return new ResponseBodyResultHandler(encoders, new DefaultConversionService());
+		}
+
+		@Bean
+		public TestController testController() {
+			return new TestController();
+		}
+	}
+
+	@Controller
+	@SuppressWarnings("unused")
+	private static class TestController {
+
+		@RequestMapping("/uknown-argument-type")
+		public void uknownArgumentType(Foo arg) {
+		}
+
+		@RequestMapping("/error-signal")
+		@ResponseBody
+		public Publisher<String> errorSignal() {
+			return Publishers.error(EXCEPTION);
+		}
+
+		@RequestMapping("/raise-exception")
+		public void raiseException() throws Exception {
+			throw EXCEPTION;
+		}
+
+		@RequestMapping("/unknown-return-type")
+		public Foo unknownReturnType() throws Exception {
+			return new Foo();
+		}
+
+		@RequestMapping("/request-body")
+		@ResponseBody
+		public Publisher<String> requestBody(@RequestBody Publisher<String> body) {
+			return Publishers.map(body, s -> "hello " + s);
+		}
+	}
+
+	private static class Foo {
+	}
+
+	private static class ServerError500ExceptionHandler implements HttpExceptionHandler {
+
+		@Override
+		public Publisher<Void> handle(ServerHttpRequest request, ServerHttpResponse response, Throwable ex) {
+			response.setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR);
+			return Publishers.empty();
+		}
+	}
+
+	private static class TestHttpFilter implements HttpFilter {
+
+		@Override
+		public Publisher<Void> filter(ServerHttpRequest req, ServerHttpResponse res, HttpFilterChain chain) {
+			return chain.filter(req, res);
+		}
+	}
+
+}