From 437c127b627aa6ab12c3fd91ab70ee2d1d8c9f54 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 13 Jan 2016 18:00:13 -0500 Subject: [PATCH] Log and handle unresolved exceptions Before this change use of ExceptionHandlingWebHandler did ensure no error signals are allowed to escape (hence relying on runtime behavior). This change ensures the same is done even when ExceptionHandlingWebHandler is not configured for use, at the lowest level which is the WebToHttpHandlerAdapter. --- .../handler/SimpleHandlerResultHandler.java | 4 ++-- .../server/ExceptionHandlingWebHandler.java | 13 +++++++++++-- .../web/server/WebToHttpHandlerAdapter.java | 18 +++++++++++++++++- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/spring-web-reactive/src/main/java/org/springframework/web/reactive/handler/SimpleHandlerResultHandler.java b/spring-web-reactive/src/main/java/org/springframework/web/reactive/handler/SimpleHandlerResultHandler.java index 6c8494ff1d..7c7c4126cd 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/reactive/handler/SimpleHandlerResultHandler.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/reactive/handler/SimpleHandlerResultHandler.java @@ -62,8 +62,8 @@ public class SimpleHandlerResultHandler implements Ordered, HandlerResultHandler @Override public boolean supports(HandlerResult result) { ResolvableType type = result.getResultType(); - return type != null && Void.TYPE.equals(type.getRawClass()) || - (Void.class.isAssignableFrom(type.getGeneric(0).getRawClass()) && isConvertibleToPublisher(type)); + return (type != null && Void.TYPE.equals(type.getRawClass()) || + (isConvertibleToPublisher(type) && Void.class.isAssignableFrom(type.getGeneric(0).getRawClass()))); } private boolean isConvertibleToPublisher(ResolvableType type) { diff --git a/spring-web-reactive/src/main/java/org/springframework/web/server/ExceptionHandlingWebHandler.java b/spring-web-reactive/src/main/java/org/springframework/web/server/ExceptionHandlingWebHandler.java index 2842bc801b..1c6ba76a65 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/server/ExceptionHandlingWebHandler.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/server/ExceptionHandlingWebHandler.java @@ -19,6 +19,8 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import reactor.Mono; import org.springframework.http.HttpStatus; @@ -31,6 +33,9 @@ import org.springframework.http.HttpStatus; */ public class ExceptionHandlingWebHandler extends WebHandlerDecorator { + private static Log logger = LogFactory.getLog(ExceptionHandlingWebHandler.class); + + private final List exceptionHandlers; @@ -51,6 +56,7 @@ public class ExceptionHandlingWebHandler extends WebHandlerDecorator { return this.exceptionHandlers; } + @Override public Mono handle(WebServerExchange exchange) { Mono mono; @@ -63,10 +69,13 @@ public class ExceptionHandlingWebHandler extends WebHandlerDecorator { for (WebExceptionHandler exceptionHandler : this.exceptionHandlers) { mono = mono.otherwise(ex -> exceptionHandler.handle(exchange, ex)); } - return mono.otherwise(ex -> handleUnresolvedException(exchange)); + return mono.otherwise(ex -> handleUnresolvedException(exchange, ex)); } - private Mono handleUnresolvedException(WebServerExchange exchange) { + private Mono handleUnresolvedException(WebServerExchange exchange, Throwable ex) { + if (logger.isDebugEnabled()) { + logger.debug("Could not complete request", ex); + } exchange.getResponse().setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR); return Mono.empty(); } diff --git a/spring-web-reactive/src/main/java/org/springframework/web/server/WebToHttpHandlerAdapter.java b/spring-web-reactive/src/main/java/org/springframework/web/server/WebToHttpHandlerAdapter.java index aace5d7885..ec8b418a40 100644 --- a/spring-web-reactive/src/main/java/org/springframework/web/server/WebToHttpHandlerAdapter.java +++ b/spring-web-reactive/src/main/java/org/springframework/web/server/WebToHttpHandlerAdapter.java @@ -15,8 +15,11 @@ */ package org.springframework.web.server; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import reactor.Mono; +import org.springframework.http.HttpStatus; import org.springframework.http.server.reactive.HttpHandler; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; @@ -29,15 +32,28 @@ import org.springframework.http.server.reactive.ServerHttpResponse; */ public class WebToHttpHandlerAdapter extends WebHandlerDecorator implements HttpHandler { + private static Log logger = LogFactory.getLog(WebToHttpHandlerAdapter.class); + public WebToHttpHandlerAdapter(WebHandler delegate) { super(delegate); } + @Override public Mono handle(ServerHttpRequest request, ServerHttpResponse response) { WebServerExchange exchange = createWebServerExchange(request, response); - return getDelegate().handle(exchange).doOnTerminate((aVoid, ex) -> response.writeHeaders()); + return getDelegate().handle(exchange).otherwise(ex -> { + if (logger.isDebugEnabled()) { + logger.debug("Could not complete request", ex); + } + response.setStatusCode(HttpStatus.INTERNAL_SERVER_ERROR); + return Mono.empty(); + }) + .doOnTerminate((aVoid, ex) -> { + response.writeHeaders(); + }); + } protected WebServerExchange createWebServerExchange(ServerHttpRequest request, ServerHttpResponse response) {