Configurable SentryExceptionResolver Order for #681 (#1008)

Co-authored-by: Maciej Walkowiak <walkowiak.maciej@yahoo.com>
Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com>
Co-authored-by: Manoel Aranda Neto <marandaneto@gmail.com>

.gitignore

@@ -9,3 +9,7 @@ local.properties
 .cxx
 **/sentry-native-local
 target/
+.classpath
+.project
+.settings/
+bin/

CHANGELOG.md

@@ -10,6 +10,7 @@
 * Fix broken NDK integration on 3.1.2 (release failed on packaging a .so file)
 * Increase max cached events to 30 (#1029)
 * Normalize DSN URI (#1030)
+* Enhancement: Make SentryExceptionResolver Order configurable to not send handled web exceptions
 * Enhancement: Resolve HTTP Proxy parameters from the external configuration (#1028)
 
 # 3.1.2

sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java

@@ -8,6 +8,7 @@
 import io.sentry.Sentry;
 import io.sentry.SentryOptions;
 import io.sentry.protocol.SdkVersion;
+import io.sentry.spring.SentryExceptionResolver;
 import io.sentry.spring.SentryUserProvider;
 import io.sentry.spring.SentryUserProviderEventProcessor;
 import io.sentry.spring.SentryWebConfiguration;
@@ -92,7 +93,14 @@ static class HubConfiguration {
     @ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET)
     @Import(SentryWebConfiguration.class)
     @Open
-    static class SentryWebMvcConfiguration {}
+    static class SentryWebMvcConfiguration {
+      @Bean
+      @ConditionalOnMissingBean
+      public @NotNull SentryExceptionResolver sentryExceptionResolver(
+          final @NotNull IHub sentryHub, final @NotNull SentryProperties options) {
+        return new SentryExceptionResolver(sentryHub, options.getExceptionResolverOrder());
+      }
+    }
 
     private static @NotNull SdkVersion createSdkVersion(
         final @NotNull SentryOptions sentryOptions) {

sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java

@@ -15,6 +15,9 @@ public class SentryProperties extends SentryOptions {
   /** Weather to use Git commit id as a release. */
   private boolean useGitCommitIdAsRelease = true;
 
+  /** Report all or only uncaught web exceptions. */
+  private int exceptionResolverOrder = Integer.MIN_VALUE;
+
   /** Logging framework integration properties. */
   private @NotNull Logging logging = new Logging();
 
@@ -26,6 +29,26 @@ public void setUseGitCommitIdAsRelease(boolean useGitCommitIdAsRelease) {
     this.useGitCommitIdAsRelease = useGitCommitIdAsRelease;
   }
 
+  /**
+   * Returns the order used for Spring SentryExceptionResolver, which determines whether all web
+   * exceptions are reported, or only uncaught exceptions.
+   *
+   * @return order to use for Spring SentryExceptionResolver
+   */
+  public int getExceptionResolverOrder() {
+    return exceptionResolverOrder;
+  }
+
+  /**
+   * Sets the order to use for Spring SentryExceptionResolver, which determines whether all web
+   * exceptions are reported, or only uncaught exceptions.
+   *
+   * @param exceptionResolverOrder order to use for Spring SentryExceptionResolver
+   */
+  public void setExceptionResolverOrder(int exceptionResolverOrder) {
+    this.exceptionResolverOrder = exceptionResolverOrder;
+  }
+
   public Logging getLogging() {
     return logging;
   }

sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt

@@ -93,12 +93,13 @@ class SentryAutoConfigurationTest {
             "sentry.attach-threads=true",
             "sentry.attach-stacktrace=true",
             "sentry.server-name=host-001",
+            "sentry.exception-resolver-order=100",
             "sentry.proxy.host=example.proxy.com",
             "sentry.proxy.port=8090",
             "sentry.proxy.user=proxy-user",
             "sentry.proxy.pass=proxy-pass"
         ).run {
-            val options = it.getBean(SentryOptions::class.java)
+            val options = it.getBean(SentryProperties::class.java)
             assertThat(options.readTimeoutMillis).isEqualTo(10)
             assertThat(options.shutdownTimeout).isEqualTo(20)
             assertThat(options.flushTimeoutMillis).isEqualTo(30)
@@ -114,6 +115,7 @@ class SentryAutoConfigurationTest {
             assertThat(options.isAttachThreads).isEqualTo(true)
             assertThat(options.isAttachStacktrace).isEqualTo(true)
             assertThat(options.serverName).isEqualTo("host-001")
+            assertThat(options.exceptionResolverOrder).isEqualTo(100)
             assertThat(options.proxy).isNotNull()
             assertThat(options.proxy!!.host).isEqualTo("example.proxy.com")
             assertThat(options.proxy!!.port).isEqualTo("8090")

sentry-spring/src/main/java/io/sentry/spring/EnableSentry.java

@@ -5,6 +5,7 @@
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 import org.springframework.context.annotation.Import;
+import org.springframework.core.Ordered;
 
 /**
  * Enables Sentry error handling capabilities.
@@ -37,4 +38,11 @@
    * @return true if send default PII or false otherwise.
    */
   boolean sendDefaultPii() default false;
+
+  /**
+   * Determines whether all web exceptions are reported or only uncaught exceptions.
+   *
+   * @return the order to use for {@link SentryExceptionResolver}
+   */
+  int exceptionResolverOrder() default Ordered.HIGHEST_PRECEDENCE;
 }

sentry-spring/src/main/java/io/sentry/spring/SentryExceptionResolver.java

@@ -23,9 +23,11 @@
 @Open
 public class SentryExceptionResolver implements HandlerExceptionResolver, Ordered {
   private final @NotNull IHub hub;
+  private final int order;
 
-  public SentryExceptionResolver(final @NotNull IHub hub) {
+  public SentryExceptionResolver(final @NotNull IHub hub, final int order) {
     this.hub = Objects.requireNonNull(hub, "hub is required");
+    this.order = order;
   }
 
   @Override
@@ -49,7 +51,6 @@ public SentryExceptionResolver(final @NotNull IHub hub) {
 
   @Override
   public int getOrder() {
-    // ensure this resolver runs first so that all exceptions are reported
-    return Integer.MIN_VALUE;
+    return order;
   }
 }

sentry-spring/src/main/java/io/sentry/spring/SentryHubRegistrar.java

@@ -26,6 +26,7 @@ public void registerBeanDefinitions(
     if (annotationAttributes != null && annotationAttributes.containsKey("dsn")) {
       registerSentryOptions(registry, annotationAttributes);
       registerSentryHubBean(registry);
+      registerSentryExceptionResolver(registry, annotationAttributes);
     }
   }
 
@@ -56,6 +57,18 @@ private void registerSentryHubBean(BeanDefinitionRegistry registry) {
     registry.registerBeanDefinition("sentryHub", builder.getBeanDefinition());
   }
 
+  private void registerSentryExceptionResolver(
+      final @NotNull BeanDefinitionRegistry registry,
+      final @NotNull AnnotationAttributes annotationAttributes) {
+    final BeanDefinitionBuilder builder =
+        BeanDefinitionBuilder.genericBeanDefinition(SentryExceptionResolver.class);
+    builder.addConstructorArgReference("sentryHub");
+    int order = annotationAttributes.getNumber("exceptionResolverOrder");
+    builder.addConstructorArgValue(order);
+
+    registry.registerBeanDefinition("sentryExceptionResolver", builder.getBeanDefinition());
+  }
+
   private static @NotNull SdkVersion createSdkVersion() {
     final SentryOptions defaultOptions = new SentryOptions();
     SdkVersion sdkVersion = defaultOptions.getSdkVersion();

sentry-spring/src/main/java/io/sentry/spring/SentryWebConfiguration.java

@@ -27,9 +27,4 @@ public class SentryWebConfiguration {
       final @NotNull IHub sentryHub, final @NotNull SentryOptions sentryOptions) {
     return new SentrySpringRequestListener(sentryHub, sentryOptions);
   }
-
-  @Bean
-  public @NotNull SentryExceptionResolver sentryExceptionResolver(final @NotNull IHub sentryHub) {
-    return new SentryExceptionResolver(sentryHub);
-  }
 }

sentry-spring/src/test/kotlin/io/sentry/spring/EnableSentryTest.kt

@@ -73,9 +73,21 @@ class EnableSentryTest {
     fun `creates SentryExceptionResolver`() {
         contextRunner.run {
             assertThat(it).hasSingleBean(SentryExceptionResolver::class.java)
+            assertThat(it).getBean(SentryExceptionResolver::class.java)
+                .hasFieldOrPropertyWithValue("order", Integer.MIN_VALUE)
         }
     }
 
+    @Test
+    fun `creates SentryExceptionResolver with order set in the @EnableSentry annotation`() {
+        ApplicationContextRunner().withConfiguration(UserConfigurations.of(AppConfigWithExceptionResolverOrderIntegerMaxValue::class.java))
+            .run {
+                assertThat(it).hasSingleBean(SentryExceptionResolver::class.java)
+                assertThat(it).getBean(SentryExceptionResolver::class.java)
+                    .hasFieldOrPropertyWithValue("order", Integer.MAX_VALUE)
+            }
+    }
+
     @EnableSentry(dsn = "http://key@localhost/proj")
     class AppConfig
 
@@ -84,4 +96,7 @@ class EnableSentryTest {
 
     @EnableSentry(dsn = "http://key@localhost/proj", sendDefaultPii = true)
     class AppConfigWithDefaultSendPii
+
+    @EnableSentry(dsn = "http://key@localhost/proj", exceptionResolverOrder = Integer.MAX_VALUE)
+    class AppConfigWithExceptionResolverOrderIntegerMaxValue
 }

sentry-spring/src/test/kotlin/io/sentry/spring/SentrySpringIntegrationTest.kt

@@ -3,10 +3,12 @@ package io.sentry.spring
 import com.nhaarman.mockitokotlin2.mock
 import com.nhaarman.mockitokotlin2.reset
 import com.nhaarman.mockitokotlin2.verify
+import com.nhaarman.mockitokotlin2.verifyZeroInteractions
 import io.sentry.Sentry
 import io.sentry.test.checkEvent
 import io.sentry.transport.ITransport
 import java.lang.RuntimeException
+import java.time.Duration
 import org.assertj.core.api.Assertions.assertThat
 import org.awaitility.kotlin.await
 import org.junit.Before
@@ -19,9 +21,11 @@ import org.springframework.boot.test.web.client.TestRestTemplate
 import org.springframework.boot.web.server.LocalServerPort
 import org.springframework.context.annotation.Bean
 import org.springframework.context.annotation.Configuration
+import org.springframework.core.Ordered
 import org.springframework.http.HttpEntity
 import org.springframework.http.HttpHeaders
 import org.springframework.http.HttpMethod
+import org.springframework.http.ResponseEntity
 import org.springframework.security.config.annotation.web.builders.HttpSecurity
 import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter
 import org.springframework.security.core.userdetails.User
@@ -31,6 +35,8 @@ import org.springframework.security.crypto.factory.PasswordEncoderFactories
 import org.springframework.security.crypto.password.PasswordEncoder
 import org.springframework.security.provisioning.InMemoryUserDetailsManager
 import org.springframework.test.context.junit4.SpringRunner
+import org.springframework.web.bind.annotation.ControllerAdvice
+import org.springframework.web.bind.annotation.ExceptionHandler
 import org.springframework.web.bind.annotation.GetMapping
 import org.springframework.web.bind.annotation.RestController
 
@@ -103,10 +109,21 @@ class SentrySpringIntegrationTest {
             })
         }
     }
+
+    @Test
+    fun `does not send events for handled exceptions`() {
+        val restTemplate = TestRestTemplate().withBasicAuth("user", "password")
+
+        restTemplate.getForEntity("http://localhost:$port/throws-handled", String::class.java)
+
+        await.during(Duration.ofSeconds(2)).untilAsserted {
+            verifyZeroInteractions(transport)
+        }
+    }
 }
 
 @SpringBootApplication
-@EnableSentry(dsn = "http://key@localhost/proj", sendDefaultPii = true)
+@EnableSentry(dsn = "http://key@localhost/proj", sendDefaultPii = true, exceptionResolverOrder = Ordered.LOWEST_PRECEDENCE)
 open class App {
 
     @Bean
@@ -125,6 +142,20 @@ class HelloController {
     fun throws() {
         throw RuntimeException("something went wrong")
     }
+
+    @GetMapping("/throws-handled")
+    fun throwsHandled() {
+        throw CustomException("handled exception")
+    }
+}
+
+class CustomException(message: String) : RuntimeException(message)
+
+@ControllerAdvice
+class ExceptionHandlers {
+
+    @ExceptionHandler(CustomException::class)
+    fun handle(e: CustomException) = ResponseEntity.badRequest().build<Void>()
 }
 
 @Configuration