Suggested change to fix logging of handled exceptions

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.
- *
+ * <p>
  * 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;
     }
 }
+

bugsnag-spring/src/main/java/com/bugsnag/MvcConfiguration.java

@@ -1,36 +1,48 @@
 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.WebMvcConfigurerAdapter;
 
+import java.util.List;
 import javax.annotation.PostConstruct;
 
 /**
  * If spring-webmvc is loaded, add configuration for reporting unhandled exceptions.
  */
 @Configuration
 @Conditional(SpringWebMvcLoadedCondition.class)
-class MvcConfiguration {
+class MvcConfiguration extends WebMvcConfigurerAdapter {
 
     @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.
      */
     @PostConstruct
     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}
+     * <p>
+     * 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})
+     * <p>
+     * Therefore, exceptions that are handled by these handlers should not be handled by Bugsnag.
+     * Only unhandled exceptions shall be sent to Bugsnag.
+     */
+    @Override
+    public void extendHandlerExceptionResolvers(List<HandlerExceptionResolver> exceptionResolvers) {
+        final int position = exceptionResolvers.isEmpty() ? 0 : exceptionResolvers.size() - 1;
+        exceptionResolvers.add(position, new BugsnagMvcExceptionHandler(bugsnag));
+    }
 }

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);

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
      */

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 {
+}

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());
+    }
+}

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 {
+}