diff --git a/.gitignore b/.gitignore index 600d3aefcc..b79a6df2a3 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,7 @@ local.properties .cxx **/sentry-native-local target/ +.classpath +.project +.settings/ +bin/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 3429233d60..4e80f11d78 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java index ba7bcc3955..c289e95deb 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java +++ b/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) { diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java index 1eec2a39ee..04c82c49b7 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java +++ b/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; } diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt index 0c513f733e..d20088b4ee 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt +++ b/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") diff --git a/sentry-spring/src/main/java/io/sentry/spring/EnableSentry.java b/sentry-spring/src/main/java/io/sentry/spring/EnableSentry.java index 09e32f016d..7957ebdfc0 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/EnableSentry.java +++ b/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; } diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentryExceptionResolver.java b/sentry-spring/src/main/java/io/sentry/spring/SentryExceptionResolver.java index 30c76a680a..f264e0ea63 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SentryExceptionResolver.java +++ b/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; } } diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentryHubRegistrar.java b/sentry-spring/src/main/java/io/sentry/spring/SentryHubRegistrar.java index 25e8582f09..bfaaa10193 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SentryHubRegistrar.java +++ b/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(); diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentryWebConfiguration.java b/sentry-spring/src/main/java/io/sentry/spring/SentryWebConfiguration.java index 5da7ff8377..e4f8cc8743 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SentryWebConfiguration.java +++ b/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); - } } diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/EnableSentryTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/EnableSentryTest.kt index 3173a12248..36532e5343 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/EnableSentryTest.kt +++ b/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 } diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/SentrySpringIntegrationTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/SentrySpringIntegrationTest.kt index 40d726c81c..0bfd13b4e8 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/SentrySpringIntegrationTest.kt +++ b/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() } @Configuration