From f717389172199103fa363e95337ab59ba35eba90 Mon Sep 17 00:00:00 2001 From: Wim Deblauwe Date: Wed, 24 Apr 2024 15:55:30 +0200 Subject: [PATCH] Handle exceptions from filter chain This commit adds a FilterChainExceptionHandlerFilter that allows to handle an exception thrown from a Filter in the same way as we do for exceptions thrown from controllers. It is not enabled by default for backwards compatibility. Use `error.handling.handle-filter-chain-exceptions=true` to enable it. Fixes #87 --- .../AbstractErrorHandlingConfiguration.java | 20 +++++ .../ErrorHandlingFacade.java | 45 +++++++++++ .../ErrorHandlingProperties.java | 10 +++ .../GlobalErrorWebExceptionHandler.java | 39 ++-------- .../ReactiveErrorHandlingConfiguration.java | 10 +-- .../ErrorHandlingControllerAdvice.java | 42 ++--------- .../FilterChainExceptionHandlerFilter.java | 37 +++++++++ .../ServletErrorHandlingConfiguration.java | 32 +++++--- ...FilterChainExceptionHandlerFilterTest.java | 75 +++++++++++++++++++ 9 files changed, 225 insertions(+), 85 deletions(-) create mode 100644 src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/ErrorHandlingFacade.java create mode 100644 src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/FilterChainExceptionHandlerFilter.java create mode 100644 src/test/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/FilterChainExceptionHandlerFilterTest.java diff --git a/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/AbstractErrorHandlingConfiguration.java b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/AbstractErrorHandlingConfiguration.java index 7ba0cd0..04b4833 100644 --- a/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/AbstractErrorHandlingConfiguration.java +++ b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/AbstractErrorHandlingConfiguration.java @@ -6,10 +6,30 @@ import io.github.wimdeblauwe.errorhandlingspringbootstarter.mapper.ErrorCodeMapper; import io.github.wimdeblauwe.errorhandlingspringbootstarter.mapper.ErrorMessageMapper; import io.github.wimdeblauwe.errorhandlingspringbootstarter.mapper.HttpStatusMapper; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.context.annotation.Bean; +import org.springframework.core.annotation.AnnotationAwareOrderComparator; + +import java.util.List; public abstract class AbstractErrorHandlingConfiguration { + private static final Logger LOGGER = LoggerFactory.getLogger(AbstractErrorHandlingConfiguration.class); + + @Bean + @ConditionalOnMissingBean + public ErrorHandlingFacade errorHandlingFacade(List handlers, + FallbackApiExceptionHandler fallbackHandler, + LoggingService loggingService, + List responseCustomizers) { + handlers.sort(AnnotationAwareOrderComparator.INSTANCE); + LOGGER.info("Error Handling Spring Boot Starter active with {} handlers", handlers.size()); + LOGGER.debug("Handlers: {}", handlers); + + return new ErrorHandlingFacade(handlers, fallbackHandler, loggingService, responseCustomizers); + } + @Bean @ConditionalOnMissingBean public LoggingService loggingService(ErrorHandlingProperties properties) { diff --git a/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/ErrorHandlingFacade.java b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/ErrorHandlingFacade.java new file mode 100644 index 0000000..91b46d2 --- /dev/null +++ b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/ErrorHandlingFacade.java @@ -0,0 +1,45 @@ +package io.github.wimdeblauwe.errorhandlingspringbootstarter; + +import java.util.List; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ErrorHandlingFacade { + private static final Logger LOGGER = LoggerFactory.getLogger(ErrorHandlingFacade.class); + + private final List handlers; + private final FallbackApiExceptionHandler fallbackHandler; + private final LoggingService loggingService; + private final List responseCustomizers; + + public ErrorHandlingFacade(List handlers, FallbackApiExceptionHandler fallbackHandler, LoggingService loggingService, + List responseCustomizers) { + this.handlers = handlers; + this.fallbackHandler = fallbackHandler; + this.loggingService = loggingService; + this.responseCustomizers = responseCustomizers; + } + + public ApiErrorResponse handle(Throwable exception) { + ApiErrorResponse errorResponse = null; + for (ApiExceptionHandler handler : handlers) { + if (handler.canHandle(exception)) { + errorResponse = handler.handle(exception); + break; + } + } + + if (errorResponse == null) { + errorResponse = fallbackHandler.handle(exception); + } + + for (ApiErrorResponseCustomizer responseCustomizer : responseCustomizers) { + responseCustomizer.customize(errorResponse); + } + + loggingService.logException(errorResponse, exception); + + return errorResponse; + } +} diff --git a/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/ErrorHandlingProperties.java b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/ErrorHandlingProperties.java index e8ed532..9206aa1 100644 --- a/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/ErrorHandlingProperties.java +++ b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/ErrorHandlingProperties.java @@ -39,6 +39,8 @@ public class ErrorHandlingProperties { private boolean searchSuperClassHierarchy = false; + private boolean handleFilterChainExceptions = false; + public boolean isEnabled() { return enabled; } @@ -143,6 +145,14 @@ public void setAddPathToError(boolean addPathToError) { this.addPathToError = addPathToError; } + public boolean isHandleFilterChainExceptions() { + return handleFilterChainExceptions; + } + + public void setHandleFilterChainExceptions(boolean handleFilterChainExceptions) { + this.handleFilterChainExceptions = handleFilterChainExceptions; + } + public enum ExceptionLogging { NO_LOGGING, MESSAGE_ONLY, diff --git a/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/reactive/GlobalErrorWebExceptionHandler.java b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/reactive/GlobalErrorWebExceptionHandler.java index 4db4ab9..e385e63 100644 --- a/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/reactive/GlobalErrorWebExceptionHandler.java +++ b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/reactive/GlobalErrorWebExceptionHandler.java @@ -1,6 +1,9 @@ package io.github.wimdeblauwe.errorhandlingspringbootstarter.reactive; -import io.github.wimdeblauwe.errorhandlingspringbootstarter.*; +import io.github.wimdeblauwe.errorhandlingspringbootstarter.ApiErrorResponse; +import io.github.wimdeblauwe.errorhandlingspringbootstarter.ApiErrorResponseCustomizer; +import io.github.wimdeblauwe.errorhandlingspringbootstarter.ApiExceptionHandler; +import io.github.wimdeblauwe.errorhandlingspringbootstarter.ErrorHandlingFacade; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.boot.autoconfigure.web.ErrorProperties; @@ -13,30 +16,20 @@ import org.springframework.web.reactive.function.server.*; import reactor.core.publisher.Mono; -import java.util.List; import java.util.Locale; public class GlobalErrorWebExceptionHandler extends DefaultErrorWebExceptionHandler { private static final Logger LOGGER = LoggerFactory.getLogger(GlobalErrorWebExceptionHandler.class); - private final List handlers; - private final FallbackApiExceptionHandler fallbackHandler; - private final LoggingService loggingService; - private final List responseCustomizers; + private final ErrorHandlingFacade errorHandlingFacade; public GlobalErrorWebExceptionHandler(ErrorAttributes errorAttributes, WebProperties.Resources resources, ErrorProperties errorProperties, ApplicationContext applicationContext, - List handlers, - FallbackApiExceptionHandler fallbackHandler, - LoggingService loggingService, - List responseCustomizers) { + ErrorHandlingFacade errorHandlingFacade) { super(errorAttributes, resources, errorProperties, applicationContext); - this.handlers = handlers; - this.fallbackHandler = fallbackHandler; - this.loggingService = loggingService; - this.responseCustomizers = responseCustomizers; + this.errorHandlingFacade = errorHandlingFacade; } @Override @@ -55,23 +48,7 @@ public Mono handleException(ServerRequest request) { LOGGER.debug("webRequest: {}", request); LOGGER.debug("locale: {}", locale); - ApiErrorResponse errorResponse = null; - for (ApiExceptionHandler handler : handlers) { - if (handler.canHandle(exception)) { - errorResponse = handler.handle(exception); - break; - } - } - - if (errorResponse == null) { - errorResponse = fallbackHandler.handle(exception); - } - - for (ApiErrorResponseCustomizer responseCustomizer : responseCustomizers) { - responseCustomizer.customize(errorResponse); - } - - loggingService.logException(errorResponse, exception); + ApiErrorResponse errorResponse = errorHandlingFacade.handle(exception); return ServerResponse.status(errorResponse.getHttpStatus()) .contentType(MediaType.APPLICATION_JSON) diff --git a/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/reactive/ReactiveErrorHandlingConfiguration.java b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/reactive/ReactiveErrorHandlingConfiguration.java index 66a56c5..0bee249 100644 --- a/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/reactive/ReactiveErrorHandlingConfiguration.java +++ b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/reactive/ReactiveErrorHandlingConfiguration.java @@ -67,19 +67,13 @@ public GlobalErrorWebExceptionHandler globalErrorWebExceptionHandler(ErrorAttrib ObjectProvider viewResolvers, ServerCodecConfigurer serverCodecConfigurer, ApplicationContext applicationContext, - LoggingService loggingService, - List handlers, - FallbackApiExceptionHandler fallbackApiExceptionHandler, - List responseCustomizers) { + ErrorHandlingFacade errorHandlingFacade) { GlobalErrorWebExceptionHandler exceptionHandler = new GlobalErrorWebExceptionHandler(errorAttributes, webProperties.getResources(), serverProperties.getError(), applicationContext, - handlers, - fallbackApiExceptionHandler, - loggingService, - responseCustomizers); + errorHandlingFacade); exceptionHandler.setViewResolvers(viewResolvers.orderedStream().collect(Collectors.toList())); exceptionHandler.setMessageWriters(serverCodecConfigurer.getWriters()); exceptionHandler.setMessageReaders(serverCodecConfigurer.getReaders()); diff --git a/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/ErrorHandlingControllerAdvice.java b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/ErrorHandlingControllerAdvice.java index dda8ffd..3059f17 100644 --- a/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/ErrorHandlingControllerAdvice.java +++ b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/ErrorHandlingControllerAdvice.java @@ -1,17 +1,16 @@ package io.github.wimdeblauwe.errorhandlingspringbootstarter.servlet; -import io.github.wimdeblauwe.errorhandlingspringbootstarter.*; +import io.github.wimdeblauwe.errorhandlingspringbootstarter.ApiErrorResponse; +import io.github.wimdeblauwe.errorhandlingspringbootstarter.ErrorHandlingFacade; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; -import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ControllerAdvice; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.context.request.WebRequest; -import java.util.List; import java.util.Locale; @ControllerAdvice(annotations = RestController.class) @@ -19,23 +18,10 @@ public class ErrorHandlingControllerAdvice { private static final Logger LOGGER = LoggerFactory.getLogger(ErrorHandlingControllerAdvice.class); - private final List handlers; - private final FallbackApiExceptionHandler fallbackHandler; - private final LoggingService loggingService; - private final List responseCustomizers; + private final ErrorHandlingFacade errorHandlingFacade; - public ErrorHandlingControllerAdvice(List handlers, - FallbackApiExceptionHandler fallbackHandler, - LoggingService loggingService, - List responseCustomizers) { - this.handlers = handlers; - this.fallbackHandler = fallbackHandler; - this.loggingService = loggingService; - this.responseCustomizers = responseCustomizers; - this.handlers.sort(AnnotationAwareOrderComparator.INSTANCE); - - LOGGER.info("Error Handling Spring Boot Starter active with {} handlers", this.handlers.size()); - LOGGER.debug("Handlers: {}", this.handlers); + public ErrorHandlingControllerAdvice(ErrorHandlingFacade errorHandlingFacade) { + this.errorHandlingFacade = errorHandlingFacade; } @ExceptionHandler @@ -43,23 +29,7 @@ public ResponseEntity handleException(Throwable exception, WebRequest webRequ LOGGER.debug("webRequest: {}", webRequest); LOGGER.debug("locale: {}", locale); - ApiErrorResponse errorResponse = null; - for (ApiExceptionHandler handler : handlers) { - if (handler.canHandle(exception)) { - errorResponse = handler.handle(exception); - break; - } - } - - if (errorResponse == null) { - errorResponse = fallbackHandler.handle(exception); - } - - for (ApiErrorResponseCustomizer responseCustomizer : responseCustomizers) { - responseCustomizer.customize(errorResponse); - } - - loggingService.logException(errorResponse, exception); + ApiErrorResponse errorResponse = errorHandlingFacade.handle(exception); return ResponseEntity.status(errorResponse.getHttpStatus()) .body(errorResponse); diff --git a/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/FilterChainExceptionHandlerFilter.java b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/FilterChainExceptionHandlerFilter.java new file mode 100644 index 0000000..78b52a6 --- /dev/null +++ b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/FilterChainExceptionHandlerFilter.java @@ -0,0 +1,37 @@ +package io.github.wimdeblauwe.errorhandlingspringbootstarter.servlet; + +import com.fasterxml.jackson.databind.ObjectMapper; +import io.github.wimdeblauwe.errorhandlingspringbootstarter.ApiErrorResponse; +import io.github.wimdeblauwe.errorhandlingspringbootstarter.ErrorHandlingFacade; +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.springframework.web.filter.OncePerRequestFilter; + +import java.io.IOException; + +public class FilterChainExceptionHandlerFilter extends OncePerRequestFilter { + + private final ErrorHandlingFacade errorHandlingFacade; + private final ObjectMapper objectMapper; + + public FilterChainExceptionHandlerFilter(ErrorHandlingFacade errorHandlingFacade, ObjectMapper objectMapper) { + this.errorHandlingFacade = errorHandlingFacade; + this.objectMapper = objectMapper; + } + + @Override + protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) + throws ServletException, IOException { + + try { + filterChain.doFilter(request, response); + } catch (Exception ex) { + ApiErrorResponse errorResponse = errorHandlingFacade.handle(ex); + response.setStatus(errorResponse.getHttpStatus().value()); + var jsonResponseBody = objectMapper.writeValueAsString(errorResponse); + response.getWriter().write(jsonResponseBody); + } + } +} diff --git a/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/ServletErrorHandlingConfiguration.java b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/ServletErrorHandlingConfiguration.java index 88b36a5..9edd78b 100644 --- a/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/ServletErrorHandlingConfiguration.java +++ b/src/main/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/ServletErrorHandlingConfiguration.java @@ -1,5 +1,6 @@ package io.github.wimdeblauwe.errorhandlingspringbootstarter.servlet; +import com.fasterxml.jackson.databind.ObjectMapper; import io.github.wimdeblauwe.errorhandlingspringbootstarter.*; import io.github.wimdeblauwe.errorhandlingspringbootstarter.handler.MissingRequestValueExceptionHandler; import io.github.wimdeblauwe.errorhandlingspringbootstarter.mapper.ErrorCodeMapper; @@ -10,11 +11,11 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.web.servlet.FilterRegistrationBean; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Import; import org.springframework.context.annotation.PropertySource; - -import java.util.List; +import org.springframework.core.Ordered; @AutoConfiguration @ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET) @@ -38,13 +39,24 @@ public MissingRequestValueExceptionHandler missingRequestValueExceptionHandler(H @Bean @ConditionalOnMissingBean - public ErrorHandlingControllerAdvice errorHandlingControllerAdvice(List handlers, - FallbackApiExceptionHandler fallbackApiExceptionHandler, - LoggingService loggingService, - List responseCustomizers) { - return new ErrorHandlingControllerAdvice(handlers, - fallbackApiExceptionHandler, - loggingService, - responseCustomizers); + public ErrorHandlingControllerAdvice errorHandlingControllerAdvice(ErrorHandlingFacade errorHandlingFacade) { + return new ErrorHandlingControllerAdvice(errorHandlingFacade); + } + + @Bean + @ConditionalOnProperty("error.handling.handle-filter-chain-exceptions") + public FilterChainExceptionHandlerFilter filterChainExceptionHandlerFilter(ErrorHandlingFacade errorHandlingFacade, ObjectMapper objectMapper) { + return new FilterChainExceptionHandlerFilter(errorHandlingFacade, objectMapper); + } + + @Bean + @ConditionalOnProperty("error.handling.handle-filter-chain-exceptions") + public FilterRegistrationBean filterChainExceptionHandlerFilterFilterRegistrationBean(FilterChainExceptionHandlerFilter filterChainExceptionHandlerFilter) { + FilterRegistrationBean registrationBean = new FilterRegistrationBean<>(); + registrationBean.setFilter(filterChainExceptionHandlerFilter); + registrationBean.addUrlPatterns("/*"); + registrationBean.setOrder(Ordered.HIGHEST_PRECEDENCE); + + return registrationBean; } } diff --git a/src/test/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/FilterChainExceptionHandlerFilterTest.java b/src/test/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/FilterChainExceptionHandlerFilterTest.java new file mode 100644 index 0000000..9953f68 --- /dev/null +++ b/src/test/java/io/github/wimdeblauwe/errorhandlingspringbootstarter/servlet/FilterChainExceptionHandlerFilterTest.java @@ -0,0 +1,75 @@ +package io.github.wimdeblauwe.errorhandlingspringbootstarter.servlet; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; +import org.springframework.boot.test.context.TestConfiguration; +import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.context.annotation.Bean; +import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.filter.OncePerRequestFilter; + +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@WebMvcTest +@ContextConfiguration(classes = {ServletErrorHandlingConfiguration.class, + FilterChainExceptionHandlerFilterTest.TestController.class, + FilterChainExceptionHandlerFilterTest.TestConfig.class}) +@TestPropertySource(properties = "error.handling.handle-filter-chain-exceptions=true") +public class FilterChainExceptionHandlerFilterTest { + + @Autowired + private MockMvc mockMvc; + + @Test + @WithMockUser + void test() throws Exception { + mockMvc.perform(get("/test/filter-chain")) + .andExpect(status().is5xxServerError()) + .andExpect(jsonPath("code").value("RUNTIME")) + .andExpect(jsonPath("message").value("Error in filter")); + + } + + @RestController + @RequestMapping("/test/filter-chain") + public static class TestController { + + @GetMapping + public void doSomething() { + } + } + + @TestConfiguration + static class TestConfig { + + @Bean + public FilterRegistrationBean filter() { + FilterRegistrationBean registrationBean = new FilterRegistrationBean<>(); + registrationBean.setFilter(new ThrowErrorFilter()); + registrationBean.addUrlPatterns("/test/filter-chain"); + registrationBean.setOrder(2); + + return registrationBean; + } + } + + static class ThrowErrorFilter extends OncePerRequestFilter { + + @Override + protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) { + throw new RuntimeException("Error in filter"); + } + } +}