From de730676564d8e8802e1bc050918fc90fc2b071a Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Fri, 12 Feb 2021 19:50:35 +0100 Subject: [PATCH 1/4] Change the default SentryExceptionResolver behavior to not send errors handled with Spring exception handlers. --- .../sentry/spring/boot/SentryProperties.java | 2 +- .../boot/it/SentrySpringIntegrationTest.kt | 30 +++++++++++++++++++ .../java/io/sentry/spring/EnableSentry.java | 3 +- .../io/sentry/spring/EnableSentryTest.kt | 2 +- .../spring/SentrySpringIntegrationTest.kt | 3 +- 5 files changed, 34 insertions(+), 6 deletions(-) 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 ae5ddbb9ea4..876aa2c60a9 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 @@ -19,7 +19,7 @@ public class SentryProperties extends SentryOptions { private boolean enableTracing; /** Report all or only uncaught web exceptions. */ - private int exceptionResolverOrder = Integer.MIN_VALUE; + private int exceptionResolverOrder = 1; /** Logging framework integration properties. */ private @NotNull Logging logging = new Logging(); diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/it/SentrySpringIntegrationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/it/SentrySpringIntegrationTest.kt index 8f0b5d93e90..8e76f088730 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/it/SentrySpringIntegrationTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/it/SentrySpringIntegrationTest.kt @@ -5,6 +5,7 @@ import com.nhaarman.mockitokotlin2.anyOrNull import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.reset import com.nhaarman.mockitokotlin2.verify +import com.nhaarman.mockitokotlin2.verifyZeroInteractions import com.nhaarman.mockitokotlin2.whenever import io.sentry.ITransportFactory import io.sentry.Sentry @@ -12,6 +13,7 @@ import io.sentry.spring.tracing.SentrySpan 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 @@ -28,6 +30,7 @@ import org.springframework.context.annotation.Configuration 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 @@ -38,6 +41,8 @@ import org.springframework.security.crypto.password.PasswordEncoder import org.springframework.security.provisioning.InMemoryUserDetailsManager import org.springframework.stereotype.Service 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 @@ -137,6 +142,17 @@ class SentrySpringIntegrationTest { }, anyOrNull()) } } + + @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 @@ -168,6 +184,11 @@ class HelloController(private val helloService: HelloService) { throw RuntimeException("something went wrong") } + @GetMapping("/throws-handled") + fun throwsHandled() { + throw CustomException("handled exception") + } + @GetMapping("/performance") fun performance() { helloService.throws() @@ -211,3 +232,12 @@ open class SecurityConfiguration : WebSecurityConfigurerAdapter() { return InMemoryUserDetailsManager(user) } } + +class CustomException(message: String) : RuntimeException(message) + +@ControllerAdvice +class ExceptionHandlers { + + @ExceptionHandler(CustomException::class) + fun handle(e: CustomException) = ResponseEntity.badRequest().build() +} 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 7957ebdfc01..3cd9305a513 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/EnableSentry.java +++ b/sentry-spring/src/main/java/io/sentry/spring/EnableSentry.java @@ -5,7 +5,6 @@ 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. @@ -44,5 +43,5 @@ * * @return the order to use for {@link SentryExceptionResolver} */ - int exceptionResolverOrder() default Ordered.HIGHEST_PRECEDENCE; + int exceptionResolverOrder() default 1; } 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 d7533b7fd40..84f41a92915 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/EnableSentryTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/EnableSentryTest.kt @@ -70,7 +70,7 @@ class EnableSentryTest { contextRunner.run { assertThat(it).hasSingleBean(SentryExceptionResolver::class.java) assertThat(it).getBean(SentryExceptionResolver::class.java) - .hasFieldOrPropertyWithValue("order", Integer.MIN_VALUE) + .hasFieldOrPropertyWithValue("order", 1) } } 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 0e40229f468..55cedaef9a4 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/SentrySpringIntegrationTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/SentrySpringIntegrationTest.kt @@ -25,7 +25,6 @@ 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 @@ -140,7 +139,7 @@ class SentrySpringIntegrationTest { } @SpringBootApplication -@EnableSentry(dsn = "http://key@localhost/proj", sendDefaultPii = true, exceptionResolverOrder = Ordered.LOWEST_PRECEDENCE) +@EnableSentry(dsn = "http://key@localhost/proj", sendDefaultPii = true) open class App { private val transport = mock() From 929157640ea466f08742d06fefd84a8dfe09a649 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Fri, 12 Feb 2021 19:54:29 +0100 Subject: [PATCH 2/4] Changelog. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78d8e784af6..aaea4f79dd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * Enchancement: Support @SentrySpan and @SentryTransaction on classes and interfaces. (#1243) * Enchancement: Do not serialize empty collections and maps (#1245) * Ref: Simplify RestTemplate instrumentation (#1246) +* Enchancement: SentryExceptionResolver should not send handled errors by default # 4.1.0 From 7dcf08abfdedf59706ec792673bbb0306f9ba218 Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Fri, 12 Feb 2021 19:56:24 +0100 Subject: [PATCH 3/4] Changelog. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aaea4f79dd4..2c0ba9800b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ * Enchancement: Support @SentrySpan and @SentryTransaction on classes and interfaces. (#1243) * Enchancement: Do not serialize empty collections and maps (#1245) * Ref: Simplify RestTemplate instrumentation (#1246) -* Enchancement: SentryExceptionResolver should not send handled errors by default +* Enchancement: SentryExceptionResolver should not send handled errors by default (#1248). # 4.1.0 From 183db8c27e834a42e30958f404041d6215bdaa3a Mon Sep 17 00:00:00 2001 From: Maciej Walkowiak Date: Mon, 15 Feb 2021 22:09:17 +0100 Subject: [PATCH 4/4] Polish. --- CHANGELOG.md | 3 +++ .../spring/boot/it/SentrySpringIntegrationTest.kt | 12 +++++++----- .../mockito-extensions/org.mockito.plugins.MockMaker | 1 + 3 files changed, 11 insertions(+), 5 deletions(-) create mode 100644 sentry-spring-boot-starter/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c0ba9800b1..1174f348f08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,11 @@ * Enchancement: Support @SentrySpan and @SentryTransaction on classes and interfaces. (#1243) * Enchancement: Do not serialize empty collections and maps (#1245) * Ref: Simplify RestTemplate instrumentation (#1246) + +Breaking Changes: * Enchancement: SentryExceptionResolver should not send handled errors by default (#1248). + # 4.1.0 * Improve Kotlin compatibility for SdkVersion (#1213) diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/it/SentrySpringIntegrationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/it/SentrySpringIntegrationTest.kt index 8e76f088730..2efc2a69720 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/it/SentrySpringIntegrationTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/it/SentrySpringIntegrationTest.kt @@ -3,17 +3,17 @@ package io.sentry.spring.boot.it import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.anyOrNull import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.never import com.nhaarman.mockitokotlin2.reset import com.nhaarman.mockitokotlin2.verify -import com.nhaarman.mockitokotlin2.verifyZeroInteractions import com.nhaarman.mockitokotlin2.whenever +import io.sentry.IHub import io.sentry.ITransportFactory import io.sentry.Sentry import io.sentry.spring.tracing.SentrySpan 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 @@ -23,6 +23,7 @@ import org.slf4j.LoggerFactory import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.autoconfigure.SpringBootApplication import org.springframework.boot.test.context.SpringBootTest +import org.springframework.boot.test.mock.mockito.SpyBean import org.springframework.boot.test.web.client.TestRestTemplate import org.springframework.boot.web.server.LocalServerPort import org.springframework.context.annotation.Bean @@ -57,6 +58,9 @@ class SentrySpringIntegrationTest { @Autowired lateinit var transport: ITransport + @SpyBean + lateinit var hub: IHub + @LocalServerPort lateinit var port: Integer @@ -149,9 +153,7 @@ class SentrySpringIntegrationTest { restTemplate.getForEntity("http://localhost:$port/throws-handled", String::class.java) - await.during(Duration.ofSeconds(2)).untilAsserted { - verifyZeroInteractions(transport) - } + verify(hub, never()).captureEvent(any()) } } diff --git a/sentry-spring-boot-starter/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker b/sentry-spring-boot-starter/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker new file mode 100644 index 00000000000..1f0955d450f --- /dev/null +++ b/sentry-spring-boot-starter/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker @@ -0,0 +1 @@ +mock-maker-inline