From 4ba3d0736f6f14ba780845ca4095559c8c756aad Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 9 Dec 2015 14:44:05 -0500 Subject: [PATCH] Add HttpExceptionHandler --- .../reactive/ErrorHandlingHttpHandler.java | 73 ++++++++ .../server/reactive/HttpExceptionHandler.java | 43 +++++ .../ErrorHandlingHttpHandlerTests.java | 168 ++++++++++++++++++ .../method/InvocableHandlerMethodTests.java | 54 ++---- 4 files changed, 300 insertions(+), 38 deletions(-) create mode 100644 spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ErrorHandlingHttpHandler.java create mode 100644 spring-web-reactive/src/main/java/org/springframework/http/server/reactive/HttpExceptionHandler.java create mode 100644 spring-web-reactive/src/test/java/org/springframework/http/server/reactive/ErrorHandlingHttpHandlerTests.java diff --git a/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ErrorHandlingHttpHandler.java b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ErrorHandlingHttpHandler.java new file mode 100644 index 0000000000..755e83bb53 --- /dev/null +++ b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/ErrorHandlingHttpHandler.java @@ -0,0 +1,73 @@ +/* + * 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.http.server.reactive; + +import java.util.Arrays; +import java.util.List; + +import org.reactivestreams.Publisher; +import reactor.Publishers; +import reactor.core.publisher.convert.RxJava1Converter; +import rx.Observable; + +import org.springframework.util.Assert; + +/** + * {@link HttpHandler} that delegates to a target {@link HttpHandler} and handles + * any errors from it by invoking one or more {@link HttpExceptionHandler}s + * sequentially until one of them completes successfully. + * + * @author Rossen Stoyanchev + */ +public class ErrorHandlingHttpHandler extends HttpHandlerDecorator { + + private final List exceptionHandlers; + + + public ErrorHandlingHttpHandler(HttpHandler targetHandler, HttpExceptionHandler... exceptionHandlers) { + super(targetHandler); + Assert.notEmpty(exceptionHandlers, "At least one exception handler is required"); + this.exceptionHandlers = Arrays.asList(exceptionHandlers); + } + + + @Override + public Publisher handle(ServerHttpRequest request, ServerHttpResponse response) { + Publisher publisher; + try { + publisher = getDelegate().handle(request, response); + } + catch (Throwable ex) { + publisher = Publishers.error(ex); + } + for (HttpExceptionHandler handler : this.exceptionHandlers) { + publisher = applyExceptionHandler(publisher, handler, request, response); + } + return publisher; + } + + private static Publisher applyExceptionHandler(Publisher publisher, + HttpExceptionHandler handler, ServerHttpRequest request, ServerHttpResponse response) { + + // see https://github.com/reactor/reactor/issues/580 + + Observable observable = RxJava1Converter.from(publisher).onErrorResumeNext(ex -> { + return RxJava1Converter.from(handler.handle(request, response, ex)); + }); + return RxJava1Converter.from(observable); + } + +} diff --git a/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/HttpExceptionHandler.java b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/HttpExceptionHandler.java new file mode 100644 index 0000000000..4fb95c20b6 --- /dev/null +++ b/spring-web-reactive/src/main/java/org/springframework/http/server/reactive/HttpExceptionHandler.java @@ -0,0 +1,43 @@ +/* + * 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.http.server.reactive; + +import org.reactivestreams.Publisher; + +/** + * A contract for resolving exceptions from HTTP request handling. + * + *

{@link ErrorHandlingHttpHandler} provides a way of applying a list + * {@link HttpExceptionHandler}s to a target {@link HttpHandler}. + * + * @author Rossen Stoyanchev + * @see ErrorHandlingHttpHandler + */ +public interface HttpExceptionHandler { + + /** + * Handle the given exception and return a completion Publisher to indicate + * when error handling is complete, or send an error signal if the exception + * was not handled. + * + * @param request the current request + * @param response the current response + * @param ex the exception to handle + * @return Publisher to indicate when exception handling is complete. + */ + Publisher handle(ServerHttpRequest request, ServerHttpResponse response, Throwable ex); + +} diff --git a/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/ErrorHandlingHttpHandlerTests.java b/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/ErrorHandlingHttpHandlerTests.java new file mode 100644 index 0000000000..f59220ac5a --- /dev/null +++ b/spring-web-reactive/src/test/java/org/springframework/http/server/reactive/ErrorHandlingHttpHandlerTests.java @@ -0,0 +1,168 @@ +/* + * 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.http.server.reactive; + + +import java.net.URI; +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.http.HttpMethod; +import org.springframework.http.HttpStatus; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +/** + * @author Rossen Stoyanchev + */ +@SuppressWarnings("ThrowableResultOfMethodCallIgnored") +public class ErrorHandlingHttpHandlerTests { + + private MockServerHttpRequest request; + + private MockServerHttpResponse response; + + + @Before + public void setUp() throws Exception { + this.request = new MockServerHttpRequest(HttpMethod.GET, new URI("http://localhost:8080")); + this.response = new MockServerHttpResponse(); + } + + + @Test + public void handleErrorSignal() throws Exception { + HttpExceptionHandler exceptionHandler = new UnresolvedExceptionHandler(); + HttpHandler targetHandler = new TestHttpHandler(new IllegalStateException("boo")); + HttpHandler handler = new ErrorHandlingHttpHandler(targetHandler, exceptionHandler); + + Publisher publisher = handler.handle(this.request, this.response); + Streams.wrap(publisher).toList().await(5, TimeUnit.SECONDS); + + assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, this.response.getStatus()); + } + + @Test + public void handleErrorSignalWithMultipleHttpErrorHandlers() throws Exception { + HttpExceptionHandler[] exceptionHandlers = new HttpExceptionHandler[] { + new UnresolvedExceptionHandler(), + new UnresolvedExceptionHandler(), + new HttpStatusExceptionHandler(HttpStatus.INTERNAL_SERVER_ERROR), + new UnresolvedExceptionHandler() + }; + HttpHandler targetHandler = new TestHttpHandler(new IllegalStateException("boo")); + HttpHandler httpHandler = new ErrorHandlingHttpHandler(targetHandler, exceptionHandlers); + + Publisher 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 unresolvedException() throws Exception { + HttpExceptionHandler exceptionHandler = new UnresolvedExceptionHandler(); + HttpHandler targetHandler = new TestHttpHandler(new IllegalStateException("boo")); + HttpHandler httpHandler = new ErrorHandlingHttpHandler(targetHandler, exceptionHandler); + + Publisher publisher = httpHandler.handle(this.request, this.response); + Throwable ex = awaitErrorSignal(publisher); + + assertEquals("boo", ex.getMessage()); + assertNull(this.response.getStatus()); + } + + @Test + public void thrownExceptionBecomesErrorSignal() throws Exception { + HttpExceptionHandler exceptionHandler = new HttpStatusExceptionHandler(HttpStatus.INTERNAL_SERVER_ERROR); + HttpHandler targetHandler = new TestHttpHandler(new IllegalStateException("boo"), true); + HttpHandler handler = new ErrorHandlingHttpHandler(targetHandler, exceptionHandler); + + Publisher publisher = handler.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(); + } + + + private static class TestHttpHandler implements HttpHandler { + + private final Throwable exception; + + private final boolean raise; + + + public TestHttpHandler(Throwable exception) { + this(exception, false); + } + + public TestHttpHandler(Throwable exception, boolean raise) { + this.exception = exception; + this.raise = raise; + assertTrue(exception instanceof RuntimeException || !this.raise); + } + + @Override + public Publisher handle(ServerHttpRequest request, ServerHttpResponse response) { + if (this.raise) { + throw (RuntimeException) exception; + } + return Publishers.error(this.exception); + } + } + + + /** Leave the exception unresolved. */ + private static class UnresolvedExceptionHandler implements HttpExceptionHandler { + + @Override + public Publisher handle(ServerHttpRequest request, ServerHttpResponse response, Throwable ex) { + return Publishers.error(ex); + } + } + + /** Set the response status to the given HttpStatus. */ + private static class HttpStatusExceptionHandler implements HttpExceptionHandler { + + private final HttpStatus status; + + public HttpStatusExceptionHandler(HttpStatus status) { + this.status = status; + } + + @Override + public Publisher handle(ServerHttpRequest request, ServerHttpResponse response, Throwable ex) { + response.setStatusCode(this.status); + return Publishers.empty(); + } + } + +} diff --git a/spring-web-reactive/src/test/java/org/springframework/web/reactive/method/InvocableHandlerMethodTests.java b/spring-web-reactive/src/test/java/org/springframework/web/reactive/method/InvocableHandlerMethodTests.java index 8f27b173c3..fbd669e514 100644 --- a/spring-web-reactive/src/test/java/org/springframework/web/reactive/method/InvocableHandlerMethodTests.java +++ b/spring-web-reactive/src/test/java/org/springframework/web/reactive/method/InvocableHandlerMethodTests.java @@ -23,8 +23,6 @@ import java.util.Collections; import java.util.List; import java.util.concurrent.TimeUnit; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.junit.Before; import org.junit.Test; import org.reactivestreams.Publisher; @@ -49,9 +47,6 @@ import static org.mockito.Mockito.when; @SuppressWarnings("ThrowableResultOfMethodCallIgnored") public class InvocableHandlerMethodTests { - private static Log logger = LogFactory.getLog(InvocableHandlerMethodTests.class); - - private ServerHttpRequest request; @@ -66,9 +61,10 @@ public class InvocableHandlerMethodTests { InvocableHandlerMethod hm = createHandlerMethod("noArgs"); Publisher publisher = hm.invokeForRequest(this.request); - Object value = awaitValue(publisher); + List values = Streams.wrap(publisher).toList().await(5, TimeUnit.SECONDS); - assertEquals("success", value); + assertEquals(1, values.size()); + assertEquals("success", values.get(0).getValue()); } @Test @@ -78,9 +74,10 @@ public class InvocableHandlerMethodTests { hm.setHandlerMethodArgumentResolvers(Collections.singletonList(new RequestParamArgumentResolver())); Publisher publisher = hm.invokeForRequest(this.request); - Object value = awaitValue(publisher); + List values = Streams.wrap(publisher).toList().await(5, TimeUnit.SECONDS); - assertEquals("success:null", value); + assertEquals(1, values.size()); + assertEquals("success:null", values.get(0).getValue()); } @Test @@ -89,9 +86,10 @@ public class InvocableHandlerMethodTests { addResolver(hm, Publishers.just("value1")); Publisher publisher = hm.invokeForRequest(this.request); - Object value = awaitValue(publisher); + List values = Streams.wrap(publisher).toList().await(5, TimeUnit.SECONDS); - assertEquals("success:value1", value); + assertEquals(1, values.size()); + assertEquals("success:value1", values.get(0).getValue()); } @Test @@ -100,12 +98,10 @@ public class InvocableHandlerMethodTests { addResolver(hm, Publishers.from(Arrays.asList("value1", "value2", "value3"))); Publisher publisher = hm.invokeForRequest(this.request); - List> signals = awaitSignals(publisher); + List values = Streams.wrap(publisher).toList().await(5, TimeUnit.SECONDS); - assertEquals("Expected only one value: " + signals.toString(), 2, signals.size()); - assertEquals(Signal.Type.NEXT, signals.get(0).getType()); - assertEquals(Signal.Type.COMPLETE, signals.get(1).getType()); - assertEquals("success:value1", signals.get(0).get().getValue()); + assertEquals(1, values.size()); + assertEquals("success:value1", values.get(0).getValue()); } @Test @@ -191,28 +187,10 @@ public class InvocableHandlerMethodTests { handlerMethod.setHandlerMethodArgumentResolvers(Collections.singletonList(resolver)); } - private Object awaitValue(Publisher publisher) throws Exception { - Object object = awaitSignal(publisher, Signal.Type.NEXT).get(); - assertEquals(HandlerResult.class, object.getClass()); - return ((HandlerResult) object).getValue(); - } - - private Throwable awaitErrorSignal(Publisher publisher) throws Exception { - return awaitSignal(publisher, Signal.Type.ERROR).getThrowable(); - } - - @SuppressWarnings("unchecked") - private Signal awaitSignal(Publisher publisher, Signal.Type type) throws Exception { - Signal signal = awaitSignals(publisher).get(0); - if (!type.equals(signal.getType()) && signal.isOnError()) { - logger.error("Unexpected error: ", signal.getThrowable()); - } - assertEquals("Unexpected signal: " + signal, type, signal.getType()); - return signal; - } - - private List> awaitSignals(Publisher publisher) throws InterruptedException { - return Streams.wrap(publisher).materialize().toList().await(5, TimeUnit.SECONDS); + 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(); }