From a5e903aed67d0b4acab22c2735e7eabcd4c1ceb8 Mon Sep 17 00:00:00 2001 From: Tobias Mende Date: Tue, 20 Jul 2021 21:24:09 +0200 Subject: [PATCH] Insert BugsnagMvcExceptionHandler just before the DefaultHandlerExceptionResolver --- .../bugsnag/BugsnagMvcExceptionHandler.java | 7 ++-- .../java/com/bugsnag/MvcConfiguration.java | 34 +++++++++++++------ .../test/java/com/bugsnag/SpringMvcTest.java | 24 +++++++++++++ .../testapp/springboot/TestController.java | 16 +++++++++ .../springboot/TestCustomException.java | 4 +++ .../springboot/TestExceptionHandler.java | 13 +++++++ .../TestResponseStatusException.java | 9 +++++ 7 files changed, 91 insertions(+), 16 deletions(-) create mode 100644 bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestCustomException.java create mode 100644 bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestExceptionHandler.java create mode 100644 bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestResponseStatusException.java diff --git a/bugsnag-spring/src/main/java/com/bugsnag/BugsnagMvcExceptionHandler.java b/bugsnag-spring/src/main/java/com/bugsnag/BugsnagMvcExceptionHandler.java index 36d87856..6526a29f 100644 --- a/bugsnag-spring/src/main/java/com/bugsnag/BugsnagMvcExceptionHandler.java +++ b/bugsnag-spring/src/main/java/com/bugsnag/BugsnagMvcExceptionHandler.java @@ -2,24 +2,20 @@ import com.bugsnag.HandledState.SeverityReasonType; -import org.springframework.core.Ordered; -import org.springframework.core.annotation.Order; import org.springframework.web.servlet.HandlerExceptionResolver; import org.springframework.web.servlet.ModelAndView; import java.util.Collections; - import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; /** * Reports uncaught exceptions thrown from handler mapping or execution to Bugsnag * and then passes the exception to the next handler in the chain. - * + *

* Set to highest precedence so that it should be called before other exception * resolvers. */ -@Order(Ordered.HIGHEST_PRECEDENCE) class BugsnagMvcExceptionHandler implements HandlerExceptionResolver { private final Bugsnag bugsnag; @@ -48,3 +44,4 @@ public ModelAndView resolveException(HttpServletRequest request, return null; } } + diff --git a/bugsnag-spring/src/main/java/com/bugsnag/MvcConfiguration.java b/bugsnag-spring/src/main/java/com/bugsnag/MvcConfiguration.java index f0f27c2f..8708e886 100644 --- a/bugsnag-spring/src/main/java/com/bugsnag/MvcConfiguration.java +++ b/bugsnag-spring/src/main/java/com/bugsnag/MvcConfiguration.java @@ -1,10 +1,12 @@ package com.bugsnag; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; +import org.springframework.web.servlet.HandlerExceptionResolver; +import org.springframework.web.servlet.config.annotation.WebMvcConfigurationSupport; +import java.util.List; import javax.annotation.PostConstruct; /** @@ -12,20 +14,11 @@ */ @Configuration @Conditional(SpringWebMvcLoadedCondition.class) -class MvcConfiguration { +class MvcConfiguration extends WebMvcConfigurationSupport { @Autowired private Bugsnag bugsnag; - /** - * Register an exception resolver to send unhandled reports to Bugsnag - * for uncaught exceptions thrown from request handlers. - */ - @Bean - BugsnagMvcExceptionHandler bugsnagHandlerExceptionResolver() { - return new BugsnagMvcExceptionHandler(bugsnag); - } - /** * Add a callback to assign specified severities for some Spring exceptions. */ @@ -33,4 +26,23 @@ BugsnagMvcExceptionHandler bugsnagHandlerExceptionResolver() { void addExceptionClassCallback() { bugsnag.addCallback(new ExceptionClassCallback()); } + + /** + * Normally, the exceptionResolvers contain the following resolvers in this order: + * - {@link org.springframework.web.servlet.mvc.method.annotation.ExceptionHandlerExceptionResolver} + * - {@link org.springframework.web.servlet.mvc.annotation.ResponseStatusExceptionResolver} + * - {@link org.springframework.web.servlet.mvc.support.DefaultHandlerExceptionResolver} + *

+ * The first two handlers handle exceptions in an application-specific manner. + * (either by @{@link org.springframework.web.bind.annotation.ExceptionHandler} + * or @{@link org.springframework.web.bind.annotation.ResponseStatus}) + *

+ * Therefore, exceptions that are handled by these handlers should not be handled by Bugsnag. + * Only unhandled exceptions shall be sent to Bugsnag. + */ + @Override + protected void extendHandlerExceptionResolvers(List exceptionResolvers) { + final int position = exceptionResolvers.isEmpty() ? 0 : exceptionResolvers.size() - 1; + exceptionResolvers.add(position, new BugsnagMvcExceptionHandler(bugsnag)); + } } diff --git a/bugsnag-spring/src/test/java/com/bugsnag/SpringMvcTest.java b/bugsnag-spring/src/test/java/com/bugsnag/SpringMvcTest.java index 694bb98b..24857a5e 100644 --- a/bugsnag-spring/src/test/java/com/bugsnag/SpringMvcTest.java +++ b/bugsnag-spring/src/test/java/com/bugsnag/SpringMvcTest.java @@ -171,6 +171,20 @@ public void springVersionSetCorrectly() { assertEquals(SpringBootVersion.getVersion(), runtimeVersions.get("springBoot")); } + @Test + public void noBugsnagNotifyOnResponseStatusException() { + callResponseStatusExceptionEndpoint(); + + verifyNoReport(); + } + + @Test + public void noBugsnagNotifyOnExceptionHandledByExceptionHandlerException() { + callResponseStatusExceptionEndpoint(); + + verifyNoReport(); + } + @Test public void unhandledTypeMismatchExceptionSeverityInfo() { callUnhandledTypeMismatchExceptionEndpoint(); @@ -242,6 +256,16 @@ private void callUnhandledTypeMismatchExceptionEndpoint() { "/throw-type-mismatch-exception", String.class); } + private void callResponseStatusExceptionEndpoint() { + this.restTemplate.getForEntity( + "/throw-response-status-exception", String.class); + } + + private void callCustomExceptionEndpoint() { + this.restTemplate.getForEntity( + "/throw-custom-exception", String.class); + } + private void callHandledTypeMismatchExceptionUserSeverityEndpoint() { this.restTemplate.getForEntity( "/handled-type-mismatch-exception-user-severity", String.class); diff --git a/bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestController.java b/bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestController.java index 86e064bf..27c38781 100644 --- a/bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestController.java +++ b/bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestController.java @@ -33,6 +33,22 @@ public void throwTypeMismatchException() { throw new TypeMismatchException("Test", String.class); } + /** + * Throw an exception with @ResponseStatus + */ + @RequestMapping("/throw-response-status-exception") + public void throwResponseStatusException() { + throw new TestResponseStatusException(); + } + + /** + * Throw an exception that is handled by @ExceptionHandler + */ + @RequestMapping("/throw-custom-exception") + public void throwCustomException() { + throw new TestCustomException(); + } + /** * Report a handled exception where the severity reason is exceptionClass */ diff --git a/bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestCustomException.java b/bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestCustomException.java new file mode 100644 index 00000000..d4799660 --- /dev/null +++ b/bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestCustomException.java @@ -0,0 +1,4 @@ +package com.bugsnag.testapp.springboot; + +public class TestCustomException extends RuntimeException { +} diff --git a/bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestExceptionHandler.java b/bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestExceptionHandler.java new file mode 100644 index 00000000..8eacf031 --- /dev/null +++ b/bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestExceptionHandler.java @@ -0,0 +1,13 @@ +package com.bugsnag.testapp.springboot; + +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.ControllerAdvice; +import org.springframework.web.bind.annotation.ExceptionHandler; + +@ControllerAdvice +public class TestExceptionHandler { + @ExceptionHandler(TestCustomException.class) + public ResponseEntity handleTestCustomException(TestCustomException ignored) { + return ResponseEntity.ok(TestCustomException.class.getSimpleName()); + } +} diff --git a/bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestResponseStatusException.java b/bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestResponseStatusException.java new file mode 100644 index 00000000..3d48ef59 --- /dev/null +++ b/bugsnag-spring/src/test/java/com/bugsnag/testapp/springboot/TestResponseStatusException.java @@ -0,0 +1,9 @@ +package com.bugsnag.testapp.springboot; + +import static org.springframework.http.HttpStatus.I_AM_A_TEAPOT; + +import org.springframework.web.bind.annotation.ResponseStatus; + +@ResponseStatus(I_AM_A_TEAPOT) +public class TestResponseStatusException extends RuntimeException { +}