From 49e29406d98aaccb1302060fe831d430dacac3cc Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 9 Oct 2026 06:27:45 +0200 Subject: [PATCH 1/2] fix(spring): Handle transaction name provider failures Keep Spring requests and event capture running when a transaction name provider throws. Use an unlabeled custom transaction as the privacy-safe fallback and continue to later providers when resolving combined names. Apply the same behavior across the Spring, Spring Jakarta, and Spring 7 integrations. Co-Authored-By: Claude --- sentry-spring-7/api/sentry-spring-7.api | 5 ++ .../spring7/SentryExceptionResolver.java | 4 +- ...tryRequestHttpServletRequestProcessor.java | 15 ++++- .../io/sentry/spring7/SentrySpringFilter.java | 3 +- .../spring7/TransactionNameProviderUtils.java | 51 ++++++++++++++++ .../CombinedTransactionNameProvider.java | 55 +++++++++++++---- .../spring7/tracing/SentryTracingFilter.java | 11 +++- .../spring7/SentryExceptionResolverTest.kt | 20 +++++++ ...yRequestHttpServletRequestProcessorTest.kt | 33 ++++++++++- .../TransactionNameProviderUtilsTest.kt | 38 ++++++++++++ .../CombinedTransactionNameProviderTest.kt | 59 +++++++++++++++++++ .../tracing/SentryTracingFilterTest.kt | 20 +++++++ .../api/sentry-spring-jakarta.api | 5 ++ .../jakarta/SentryExceptionResolver.java | 4 +- ...tryRequestHttpServletRequestProcessor.java | 15 ++++- .../spring/jakarta/SentrySpringFilter.java | 3 +- .../jakarta/TransactionNameProviderUtils.java | 51 ++++++++++++++++ .../CombinedTransactionNameProvider.java | 55 +++++++++++++---- .../jakarta/tracing/SentryTracingFilter.java | 11 +++- .../jakarta/SentryExceptionResolverTest.kt | 20 +++++++ ...yRequestHttpServletRequestProcessorTest.kt | 33 ++++++++++- .../TransactionNameProviderUtilsTest.kt | 38 ++++++++++++ .../CombinedTransactionNameProviderTest.kt | 59 +++++++++++++++++++ .../tracing/SentryTracingFilterTest.kt | 20 +++++++ sentry-spring/api/sentry-spring.api | 5 ++ .../spring/SentryExceptionResolver.java | 4 +- ...tryRequestHttpServletRequestProcessor.java | 15 ++++- .../io/sentry/spring/SentrySpringFilter.java | 3 +- .../spring/TransactionNameProviderUtils.java | 51 ++++++++++++++++ .../CombinedTransactionNameProvider.java | 53 +++++++++++++---- .../spring/tracing/SentryTracingFilter.java | 11 +++- .../spring/SentryExceptionResolverTest.kt | 20 +++++++ ...yRequestHttpServletRequestProcessorTest.kt | 33 ++++++++++- .../TransactionNameProviderUtilsTest.kt | 38 ++++++++++++ .../CombinedTransactionNameProviderTest.kt | 59 +++++++++++++++++++ .../spring/tracing/SentryTracingFilterTest.kt | 20 +++++++ 36 files changed, 887 insertions(+), 53 deletions(-) create mode 100644 sentry-spring-7/src/main/java/io/sentry/spring7/TransactionNameProviderUtils.java create mode 100644 sentry-spring-7/src/test/kotlin/io/sentry/spring7/TransactionNameProviderUtilsTest.kt create mode 100644 sentry-spring-7/src/test/kotlin/io/sentry/spring7/tracing/CombinedTransactionNameProviderTest.kt create mode 100644 sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/TransactionNameProviderUtils.java create mode 100644 sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/TransactionNameProviderUtilsTest.kt create mode 100644 sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProviderTest.kt create mode 100644 sentry-spring/src/main/java/io/sentry/spring/TransactionNameProviderUtils.java create mode 100644 sentry-spring/src/test/kotlin/io/sentry/spring/TransactionNameProviderUtilsTest.kt create mode 100644 sentry-spring/src/test/kotlin/io/sentry/spring/tracing/CombinedTransactionNameProviderTest.kt diff --git a/sentry-spring-7/api/sentry-spring-7.api b/sentry-spring-7/api/sentry-spring-7.api index 3b2872afc67..9c55ded782d 100644 --- a/sentry-spring-7/api/sentry-spring-7.api +++ b/sentry-spring-7/api/sentry-spring-7.api @@ -104,6 +104,11 @@ public final class io/sentry/spring7/SpringSecuritySentryUserProvider : io/sentr public fun provideUser ()Lio/sentry/protocol/User; } +public final class io/sentry/spring7/TransactionNameProviderUtils { + public static fun provideTransactionName (Lio/sentry/spring7/tracing/TransactionNameProvider;Ljakarta/servlet/http/HttpServletRequest;Lio/sentry/ILogger;)Ljava/lang/String; + public static fun provideTransactionNameAndSource (Lio/sentry/spring7/tracing/TransactionNameProvider;Ljakarta/servlet/http/HttpServletRequest;Lio/sentry/ILogger;Lio/sentry/spring7/tracing/TransactionNameWithSource;)Lio/sentry/spring7/tracing/TransactionNameWithSource; +} + public final class io/sentry/spring7/cache/SentryCacheBeanPostProcessor : org/springframework/beans/factory/config/BeanPostProcessor, org/springframework/core/PriorityOrdered { public fun ()V public fun getOrder ()I diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/SentryExceptionResolver.java b/sentry-spring-7/src/main/java/io/sentry/spring7/SentryExceptionResolver.java index f7110aa0f21..4fe1f0c84e5 100644 --- a/sentry-spring-7/src/main/java/io/sentry/spring7/SentryExceptionResolver.java +++ b/sentry-spring-7/src/main/java/io/sentry/spring7/SentryExceptionResolver.java @@ -75,7 +75,9 @@ protected SentryEvent createEvent( new ExceptionMechanismException(mechanism, ex, Thread.currentThread()); final SentryEvent event = new SentryEvent(throwable); event.setLevel(SentryLevel.FATAL); - event.setTransaction(transactionNameProvider.provideTransactionName(request)); + event.setTransaction( + TransactionNameProviderUtils.provideTransactionName( + transactionNameProvider, request, scopes.getOptions().getLogger())); return event; } diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/SentryRequestHttpServletRequestProcessor.java b/sentry-spring-7/src/main/java/io/sentry/spring7/SentryRequestHttpServletRequestProcessor.java index 680f560fffc..2154539c8f5 100644 --- a/sentry-spring-7/src/main/java/io/sentry/spring7/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-spring-7/src/main/java/io/sentry/spring7/SentryRequestHttpServletRequestProcessor.java @@ -2,6 +2,8 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.Hint; +import io.sentry.ILogger; +import io.sentry.ScopesAdapter; import io.sentry.SentryEvent; import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.spring7.tracing.TransactionNameProvider; @@ -15,19 +17,30 @@ public class SentryRequestHttpServletRequestProcessor implements SentryEventProcessor { private final @NotNull TransactionNameProvider transactionNameProvider; private final @NotNull HttpServletRequest request; + private final @NotNull ILogger logger; public SentryRequestHttpServletRequestProcessor( final @NotNull TransactionNameProvider transactionNameProvider, final @NotNull HttpServletRequest request) { + this(transactionNameProvider, request, ScopesAdapter.getInstance().getOptions().getLogger()); + } + + SentryRequestHttpServletRequestProcessor( + final @NotNull TransactionNameProvider transactionNameProvider, + final @NotNull HttpServletRequest request, + final @NotNull ILogger logger) { this.transactionNameProvider = Objects.requireNonNull(transactionNameProvider, "transactionNameProvider is required"); this.request = Objects.requireNonNull(request, "request is required"); + this.logger = Objects.requireNonNull(logger, "logger is required"); } @Override public @NotNull SentryEvent process(final @NotNull SentryEvent event, final @NotNull Hint hint) { if (event.getTransaction() == null) { - event.setTransaction(transactionNameProvider.provideTransactionName(request)); + event.setTransaction( + TransactionNameProviderUtils.provideTransactionName( + transactionNameProvider, request, logger)); } return event; } diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/SentrySpringFilter.java b/sentry-spring-7/src/main/java/io/sentry/spring7/SentrySpringFilter.java index de766504aff..869eb1e3f5b 100644 --- a/sentry-spring-7/src/main/java/io/sentry/spring7/SentrySpringFilter.java +++ b/sentry-spring-7/src/main/java/io/sentry/spring7/SentrySpringFilter.java @@ -91,7 +91,8 @@ private void configureScope( // transaction name is known at the later stage of request processing, thus it cannot // be set on the scope scope.addEventProcessor( - new SentryRequestHttpServletRequestProcessor(transactionNameProvider, request)); + new SentryRequestHttpServletRequestProcessor( + transactionNameProvider, request, scopes.getOptions().getLogger())); // only if request caches body, add an event processor that sets body on the event // body is not on the scope, to avoid using memory when no event is triggered during // request processing diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/TransactionNameProviderUtils.java b/sentry-spring-7/src/main/java/io/sentry/spring7/TransactionNameProviderUtils.java new file mode 100644 index 00000000000..2bff1737d6b --- /dev/null +++ b/sentry-spring-7/src/main/java/io/sentry/spring7/TransactionNameProviderUtils.java @@ -0,0 +1,51 @@ +package io.sentry.spring7; + +import static io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME; + +import io.sentry.ILogger; +import io.sentry.SentryLevel; +import io.sentry.spring7.tracing.TransactionNameProvider; +import io.sentry.spring7.tracing.TransactionNameWithSource; +import io.sentry.util.ExceptionUtils; +import jakarta.servlet.http.HttpServletRequest; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class TransactionNameProviderUtils { + private TransactionNameProviderUtils() {} + + public static @Nullable String provideTransactionName( + final @NotNull TransactionNameProvider provider, + final @NotNull HttpServletRequest request, + final @NotNull ILogger logger) { + try { + return provider.provideTransactionName(request); + } catch (Throwable e) { + ExceptionUtils.rethrowIfFatal(e); + logCallbackError(logger, e); + return DEFAULT_TRANSACTION_NAME; + } + } + + public static @NotNull TransactionNameWithSource provideTransactionNameAndSource( + final @NotNull TransactionNameProvider provider, + final @NotNull HttpServletRequest request, + final @NotNull ILogger logger, + final @NotNull TransactionNameWithSource fallbackOnError) { + try { + return provider.provideTransactionNameAndSource(request); + } catch (Throwable e) { + ExceptionUtils.rethrowIfFatal(e); + logCallbackError(logger, e); + return fallbackOnError; + } + } + + private static void logCallbackError( + final @NotNull ILogger logger, final @NotNull Throwable throwable) { + logger.log( + SentryLevel.ERROR, "The TransactionNameProvider callback threw an exception.", throwable); + } +} diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/tracing/CombinedTransactionNameProvider.java b/sentry-spring-7/src/main/java/io/sentry/spring7/tracing/CombinedTransactionNameProvider.java index a211ee6c759..f60f98a013d 100644 --- a/sentry-spring-7/src/main/java/io/sentry/spring7/tracing/CombinedTransactionNameProvider.java +++ b/sentry-spring-7/src/main/java/io/sentry/spring7/tracing/CombinedTransactionNameProvider.java @@ -1,6 +1,11 @@ package io.sentry.spring7.tracing; +import static io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME; + +import io.sentry.ScopesAdapter; +import io.sentry.SentryLevel; import io.sentry.protocol.TransactionNameSource; +import io.sentry.util.ExceptionUtils; import jakarta.servlet.http.HttpServletRequest; import java.util.List; import org.jetbrains.annotations.ApiStatus; @@ -21,15 +26,21 @@ public CombinedTransactionNameProvider(final @NotNull List() private val transactionNameProvider = mock() + private val options = SentryOptions() private val request = mock() private val response = mock() + init { + whenever(scopes.options).thenReturn(options) + } + @Test fun `when handles exception, sets wrapped exception for event`() { val eventCaptor = argumentCaptor() @@ -70,6 +77,19 @@ class SentryExceptionResolverTest { verify(transactionNameProvider).provideTransactionName(request) } + @Test + fun `when transaction name provider fails, captures event with safe fallback`() { + val failure = RuntimeException("provider failed") + whenever(transactionNameProvider.provideTransactionName(any())).thenThrow(failure) + val eventCaptor = argumentCaptor() + whenever(scopes.captureEvent(eventCaptor.capture(), any())).thenReturn(null) + + SentryExceptionResolver(scopes, transactionNameProvider, 1) + .resolveException(request, response, null, RuntimeException("request failed")) + + assertThat(eventCaptor.firstValue.transaction).isEqualTo(DEFAULT_TRANSACTION_NAME) + } + @Test fun `when handles exception, provides spring resolver hint`() { val hintCaptor = argumentCaptor() diff --git a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SentryRequestHttpServletRequestProcessorTest.kt b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SentryRequestHttpServletRequestProcessorTest.kt index 4c2276275f7..6270e8e045f 100644 --- a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SentryRequestHttpServletRequestProcessorTest.kt +++ b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/SentryRequestHttpServletRequestProcessorTest.kt @@ -1,15 +1,20 @@ package io.sentry.spring7 import io.sentry.Hint +import io.sentry.ILogger import io.sentry.IScopes import io.sentry.SentryEvent import io.sentry.SentryOptions +import io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME import io.sentry.spring7.tracing.SpringMvcTransactionNameProvider +import io.sentry.spring7.tracing.TransactionNameProvider import jakarta.servlet.http.HttpServletRequest import java.net.URI import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertNotNull +import org.assertj.core.api.Assertions.assertThat import org.mockito.kotlin.mock import org.mockito.kotlin.whenever import org.springframework.mock.web.MockServletContext @@ -19,13 +24,15 @@ import org.springframework.web.servlet.HandlerMapping class SentryRequestHttpServletRequestProcessorTest { private class Fixture { val scopes = mock() + val logger = mock() fun getSut( request: HttpServletRequest, options: SentryOptions = SentryOptions(), + provider: TransactionNameProvider = SpringMvcTransactionNameProvider(), ): SentryRequestHttpServletRequestProcessor { whenever(scopes.options).thenReturn(options) - return SentryRequestHttpServletRequestProcessor(SpringMvcTransactionNameProvider(), request) + return SentryRequestHttpServletRequestProcessor(provider, request, logger) } } @@ -46,6 +53,30 @@ class SentryRequestHttpServletRequestProcessorTest { assertEquals("GET /some-path", event.transaction) } + @Test + fun `provider failure uses safe transaction name`() { + val failure = RuntimeException("provider failed") + val request = mock() + val eventProcessor = + fixture.getSut(request, provider = TransactionNameProvider { throw failure }) + val event = SentryEvent() + + eventProcessor.process(event, Hint()) + + assertThat(event.transaction).isEqualTo(DEFAULT_TRANSACTION_NAME) + } + + @Test + fun `fatal provider failure propagates`() { + val failure = OutOfMemoryError("fatal") + val request = mock() + val eventProcessor = + fixture.getSut(request, provider = TransactionNameProvider { throw failure }) + + assertThat(assertFailsWith { eventProcessor.process(SentryEvent(), Hint()) }) + .isSameAs(failure) + } + @Test fun `when event has transaction name set, does not overwrite transaction name with value from the current request`() { val request = diff --git a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/TransactionNameProviderUtilsTest.kt b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/TransactionNameProviderUtilsTest.kt new file mode 100644 index 00000000000..80b275d8cca --- /dev/null +++ b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/TransactionNameProviderUtilsTest.kt @@ -0,0 +1,38 @@ +package io.sentry.spring7 + +import io.sentry.ILogger +import io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME +import io.sentry.spring7.tracing.TransactionNameProvider +import jakarta.servlet.http.HttpServletRequest +import kotlin.test.Test +import kotlin.test.assertFailsWith +import org.assertj.core.api.Assertions.assertThat +import org.mockito.kotlin.mock + +class TransactionNameProviderUtilsTest { + private val request = mock() + private val logger = mock() + + @Test + fun `single provider failure uses error fallback`() { + val failure = RuntimeException("provider failed") + val provider = TransactionNameProvider { throw failure } + + val result = TransactionNameProviderUtils.provideTransactionName(provider, request, logger) + + assertThat(result).isEqualTo(DEFAULT_TRANSACTION_NAME) + } + + @Test + fun `single provider fatal failure propagates`() { + val failure = OutOfMemoryError("fatal") + val provider = TransactionNameProvider { throw failure } + + assertThat( + assertFailsWith { + TransactionNameProviderUtils.provideTransactionName(provider, request, logger) + } + ) + .isSameAs(failure) + } +} diff --git a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/tracing/CombinedTransactionNameProviderTest.kt b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/tracing/CombinedTransactionNameProviderTest.kt new file mode 100644 index 00000000000..3b8af5066a3 --- /dev/null +++ b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/tracing/CombinedTransactionNameProviderTest.kt @@ -0,0 +1,59 @@ +package io.sentry.spring7.tracing + +import io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME +import io.sentry.protocol.TransactionNameSource +import jakarta.servlet.http.HttpServletRequest +import kotlin.test.Test +import kotlin.test.assertFailsWith +import org.assertj.core.api.Assertions.assertThat +import org.mockito.kotlin.mock + +class CombinedTransactionNameProviderTest { + private val request = mock() + + @Test + fun `continues after provider failure`() { + val failingProvider = TransactionNameProvider { throw RuntimeException("provider failed") } + val successfulProvider = + object : TransactionNameProvider { + override fun provideTransactionName(request: HttpServletRequest) = "GET /users/{id}" + + override fun provideTransactionSource() = TransactionNameSource.ROUTE + } + val provider = CombinedTransactionNameProvider(listOf(failingProvider, successfulProvider)) + + val result = provider.provideTransactionNameAndSource(request) + + assertThat(result.transactionName).isEqualTo("GET /users/{id}") + assertThat(result.transactionNameSource).isEqualTo(TransactionNameSource.ROUTE) + } + + @Test + fun `uses safe fallback when none succeed after failure`() { + val failingProvider = TransactionNameProvider { throw RuntimeException("provider failed") } + val emptyProvider = TransactionNameProvider { null } + val provider = CombinedTransactionNameProvider(listOf(failingProvider, emptyProvider)) + + val result = provider.provideTransactionNameAndSource(request) + + assertThat(result.transactionName).isEqualTo(DEFAULT_TRANSACTION_NAME) + assertThat(result.transactionNameSource).isEqualTo(TransactionNameSource.CUSTOM) + } + + @Test + fun `preserves null when none resolve without failure`() { + val provider = CombinedTransactionNameProvider(listOf(TransactionNameProvider { null })) + + assertThat(provider.provideTransactionName(request)).isNull() + } + + @Test + fun `fatal provider failure propagates`() { + val failure = OutOfMemoryError("fatal") + val provider = + CombinedTransactionNameProvider(listOf(TransactionNameProvider { throw failure })) + + assertThat(assertFailsWith { provider.provideTransactionName(request) }) + .isSameAs(failure) + } +} diff --git a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/tracing/SentryTracingFilterTest.kt b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/tracing/SentryTracingFilterTest.kt index 9778a6d0154..fae0f19ebe6 100644 --- a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/tracing/SentryTracingFilterTest.kt +++ b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/tracing/SentryTracingFilterTest.kt @@ -9,6 +9,7 @@ import io.sentry.SpanId import io.sentry.SpanStatus import io.sentry.TraceContext import io.sentry.TransactionContext +import io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME import io.sentry.TransactionOptions import io.sentry.protocol.SentryId import io.sentry.protocol.SentryTransaction @@ -140,6 +141,25 @@ class SentryTracingFilterTest { ) } + @Test + fun `transaction name provider failure uses safe fallback and request continues`() { + val filter = fixture.getSut() + val failure = RuntimeException("provider failed") + whenever(fixture.transactionNameProvider.provideTransactionNameAndSource(fixture.request)) + .thenThrow(failure) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.chain).doFilter(fixture.request, fixture.response) + verify(fixture.scopes) + .captureTransaction( + check { assertThat(it.transaction).isEqualTo(DEFAULT_TRANSACTION_NAME) }, + anyOrNull(), + anyOrNull(), + anyOrNull(), + ) + } + @Test fun `sets correct span status based on the response status`() { val filter = fixture.getSut(status = 500) diff --git a/sentry-spring-jakarta/api/sentry-spring-jakarta.api b/sentry-spring-jakarta/api/sentry-spring-jakarta.api index be94039b754..0bdcca50de8 100644 --- a/sentry-spring-jakarta/api/sentry-spring-jakarta.api +++ b/sentry-spring-jakarta/api/sentry-spring-jakarta.api @@ -104,6 +104,11 @@ public final class io/sentry/spring/jakarta/SpringSecuritySentryUserProvider : i public fun provideUser ()Lio/sentry/protocol/User; } +public final class io/sentry/spring/jakarta/TransactionNameProviderUtils { + public static fun provideTransactionName (Lio/sentry/spring/jakarta/tracing/TransactionNameProvider;Ljakarta/servlet/http/HttpServletRequest;Lio/sentry/ILogger;)Ljava/lang/String; + public static fun provideTransactionNameAndSource (Lio/sentry/spring/jakarta/tracing/TransactionNameProvider;Ljakarta/servlet/http/HttpServletRequest;Lio/sentry/ILogger;Lio/sentry/spring/jakarta/tracing/TransactionNameWithSource;)Lio/sentry/spring/jakarta/tracing/TransactionNameWithSource; +} + public final class io/sentry/spring/jakarta/cache/SentryCacheBeanPostProcessor : org/springframework/beans/factory/config/BeanPostProcessor, org/springframework/core/PriorityOrdered { public fun ()V public fun getOrder ()I diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryExceptionResolver.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryExceptionResolver.java index efa98ef581b..9beba4d94fd 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryExceptionResolver.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryExceptionResolver.java @@ -75,7 +75,9 @@ protected SentryEvent createEvent( new ExceptionMechanismException(mechanism, ex, Thread.currentThread()); final SentryEvent event = new SentryEvent(throwable); event.setLevel(SentryLevel.FATAL); - event.setTransaction(transactionNameProvider.provideTransactionName(request)); + event.setTransaction( + TransactionNameProviderUtils.provideTransactionName( + transactionNameProvider, request, scopes.getOptions().getLogger())); return event; } diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestHttpServletRequestProcessor.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestHttpServletRequestProcessor.java index b18b5a914e7..e87aff165d7 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryRequestHttpServletRequestProcessor.java @@ -2,6 +2,8 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.Hint; +import io.sentry.ILogger; +import io.sentry.ScopesAdapter; import io.sentry.SentryEvent; import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.spring.jakarta.tracing.TransactionNameProvider; @@ -15,19 +17,30 @@ public class SentryRequestHttpServletRequestProcessor implements SentryEventProcessor { private final @NotNull TransactionNameProvider transactionNameProvider; private final @NotNull HttpServletRequest request; + private final @NotNull ILogger logger; public SentryRequestHttpServletRequestProcessor( final @NotNull TransactionNameProvider transactionNameProvider, final @NotNull HttpServletRequest request) { + this(transactionNameProvider, request, ScopesAdapter.getInstance().getOptions().getLogger()); + } + + SentryRequestHttpServletRequestProcessor( + final @NotNull TransactionNameProvider transactionNameProvider, + final @NotNull HttpServletRequest request, + final @NotNull ILogger logger) { this.transactionNameProvider = Objects.requireNonNull(transactionNameProvider, "transactionNameProvider is required"); this.request = Objects.requireNonNull(request, "request is required"); + this.logger = Objects.requireNonNull(logger, "logger is required"); } @Override public @NotNull SentryEvent process(final @NotNull SentryEvent event, final @NotNull Hint hint) { if (event.getTransaction() == null) { - event.setTransaction(transactionNameProvider.provideTransactionName(request)); + event.setTransaction( + TransactionNameProviderUtils.provideTransactionName( + transactionNameProvider, request, logger)); } return event; } diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentrySpringFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentrySpringFilter.java index 3ce21ed4ce5..ae26af643ad 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentrySpringFilter.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentrySpringFilter.java @@ -91,7 +91,8 @@ private void configureScope( // transaction name is known at the later stage of request processing, thus it cannot // be set on the scope scope.addEventProcessor( - new SentryRequestHttpServletRequestProcessor(transactionNameProvider, request)); + new SentryRequestHttpServletRequestProcessor( + transactionNameProvider, request, scopes.getOptions().getLogger())); // only if request caches body, add an event processor that sets body on the event // body is not on the scope, to avoid using memory when no event is triggered during // request processing diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/TransactionNameProviderUtils.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/TransactionNameProviderUtils.java new file mode 100644 index 00000000000..35035cb7737 --- /dev/null +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/TransactionNameProviderUtils.java @@ -0,0 +1,51 @@ +package io.sentry.spring.jakarta; + +import static io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME; + +import io.sentry.ILogger; +import io.sentry.SentryLevel; +import io.sentry.spring.jakarta.tracing.TransactionNameProvider; +import io.sentry.spring.jakarta.tracing.TransactionNameWithSource; +import io.sentry.util.ExceptionUtils; +import jakarta.servlet.http.HttpServletRequest; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class TransactionNameProviderUtils { + private TransactionNameProviderUtils() {} + + public static @Nullable String provideTransactionName( + final @NotNull TransactionNameProvider provider, + final @NotNull HttpServletRequest request, + final @NotNull ILogger logger) { + try { + return provider.provideTransactionName(request); + } catch (Throwable e) { + ExceptionUtils.rethrowIfFatal(e); + logCallbackError(logger, e); + return DEFAULT_TRANSACTION_NAME; + } + } + + public static @NotNull TransactionNameWithSource provideTransactionNameAndSource( + final @NotNull TransactionNameProvider provider, + final @NotNull HttpServletRequest request, + final @NotNull ILogger logger, + final @NotNull TransactionNameWithSource fallbackOnError) { + try { + return provider.provideTransactionNameAndSource(request); + } catch (Throwable e) { + ExceptionUtils.rethrowIfFatal(e); + logCallbackError(logger, e); + return fallbackOnError; + } + } + + private static void logCallbackError( + final @NotNull ILogger logger, final @NotNull Throwable throwable) { + logger.log( + SentryLevel.ERROR, "The TransactionNameProvider callback threw an exception.", throwable); + } +} diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java index 1e142e958dc..e9ca80b4f21 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProvider.java @@ -1,6 +1,11 @@ package io.sentry.spring.jakarta.tracing; +import static io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME; + +import io.sentry.ScopesAdapter; +import io.sentry.SentryLevel; import io.sentry.protocol.TransactionNameSource; +import io.sentry.util.ExceptionUtils; import jakarta.servlet.http.HttpServletRequest; import java.util.List; import org.jetbrains.annotations.ApiStatus; @@ -21,15 +26,21 @@ public CombinedTransactionNameProvider(final @NotNull List() private val transactionNameProvider = mock() + private val options = SentryOptions() private val request = mock() private val response = mock() + init { + whenever(scopes.options).thenReturn(options) + } + @Test fun `when handles exception, sets wrapped exception for event`() { val eventCaptor = argumentCaptor() @@ -70,6 +77,19 @@ class SentryExceptionResolverTest { verify(transactionNameProvider).provideTransactionName(request) } + @Test + fun `when transaction name provider fails, captures event with safe fallback`() { + val failure = RuntimeException("provider failed") + whenever(transactionNameProvider.provideTransactionName(any())).thenThrow(failure) + val eventCaptor = argumentCaptor() + whenever(scopes.captureEvent(eventCaptor.capture(), any())).thenReturn(null) + + SentryExceptionResolver(scopes, transactionNameProvider, 1) + .resolveException(request, response, null, RuntimeException("request failed")) + + assertThat(eventCaptor.firstValue.transaction).isEqualTo(DEFAULT_TRANSACTION_NAME) + } + @Test fun `when handles exception, provides spring resolver hint`() { val hintCaptor = argumentCaptor() diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentryRequestHttpServletRequestProcessorTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentryRequestHttpServletRequestProcessorTest.kt index a63f00a788f..818090afc25 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentryRequestHttpServletRequestProcessorTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/SentryRequestHttpServletRequestProcessorTest.kt @@ -1,15 +1,20 @@ package io.sentry.spring.jakarta import io.sentry.Hint +import io.sentry.ILogger import io.sentry.IScopes import io.sentry.SentryEvent import io.sentry.SentryOptions +import io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME import io.sentry.spring.jakarta.tracing.SpringMvcTransactionNameProvider +import io.sentry.spring.jakarta.tracing.TransactionNameProvider import jakarta.servlet.http.HttpServletRequest import java.net.URI import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertNotNull +import org.assertj.core.api.Assertions.assertThat import org.mockito.kotlin.mock import org.mockito.kotlin.whenever import org.springframework.mock.web.MockServletContext @@ -19,13 +24,15 @@ import org.springframework.web.servlet.HandlerMapping class SentryRequestHttpServletRequestProcessorTest { private class Fixture { val scopes = mock() + val logger = mock() fun getSut( request: HttpServletRequest, options: SentryOptions = SentryOptions(), + provider: TransactionNameProvider = SpringMvcTransactionNameProvider(), ): SentryRequestHttpServletRequestProcessor { whenever(scopes.options).thenReturn(options) - return SentryRequestHttpServletRequestProcessor(SpringMvcTransactionNameProvider(), request) + return SentryRequestHttpServletRequestProcessor(provider, request, logger) } } @@ -46,6 +53,30 @@ class SentryRequestHttpServletRequestProcessorTest { assertEquals("GET /some-path", event.transaction) } + @Test + fun `provider failure uses safe transaction name`() { + val failure = RuntimeException("provider failed") + val request = mock() + val eventProcessor = + fixture.getSut(request, provider = TransactionNameProvider { throw failure }) + val event = SentryEvent() + + eventProcessor.process(event, Hint()) + + assertThat(event.transaction).isEqualTo(DEFAULT_TRANSACTION_NAME) + } + + @Test + fun `fatal provider failure propagates`() { + val failure = OutOfMemoryError("fatal") + val request = mock() + val eventProcessor = + fixture.getSut(request, provider = TransactionNameProvider { throw failure }) + + assertThat(assertFailsWith { eventProcessor.process(SentryEvent(), Hint()) }) + .isSameAs(failure) + } + @Test fun `when event has transaction name set, does not overwrite transaction name with value from the current request`() { val request = diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/TransactionNameProviderUtilsTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/TransactionNameProviderUtilsTest.kt new file mode 100644 index 00000000000..c1a91c15503 --- /dev/null +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/TransactionNameProviderUtilsTest.kt @@ -0,0 +1,38 @@ +package io.sentry.spring.jakarta + +import io.sentry.ILogger +import io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME +import io.sentry.spring.jakarta.tracing.TransactionNameProvider +import jakarta.servlet.http.HttpServletRequest +import kotlin.test.Test +import kotlin.test.assertFailsWith +import org.assertj.core.api.Assertions.assertThat +import org.mockito.kotlin.mock + +class TransactionNameProviderUtilsTest { + private val request = mock() + private val logger = mock() + + @Test + fun `single provider failure uses error fallback`() { + val failure = RuntimeException("provider failed") + val provider = TransactionNameProvider { throw failure } + + val result = TransactionNameProviderUtils.provideTransactionName(provider, request, logger) + + assertThat(result).isEqualTo(DEFAULT_TRANSACTION_NAME) + } + + @Test + fun `single provider fatal failure propagates`() { + val failure = OutOfMemoryError("fatal") + val provider = TransactionNameProvider { throw failure } + + assertThat( + assertFailsWith { + TransactionNameProviderUtils.provideTransactionName(provider, request, logger) + } + ) + .isSameAs(failure) + } +} diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProviderTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProviderTest.kt new file mode 100644 index 00000000000..29b17c2a4b5 --- /dev/null +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/CombinedTransactionNameProviderTest.kt @@ -0,0 +1,59 @@ +package io.sentry.spring.jakarta.tracing + +import io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME +import io.sentry.protocol.TransactionNameSource +import jakarta.servlet.http.HttpServletRequest +import kotlin.test.Test +import kotlin.test.assertFailsWith +import org.assertj.core.api.Assertions.assertThat +import org.mockito.kotlin.mock + +class CombinedTransactionNameProviderTest { + private val request = mock() + + @Test + fun `continues after provider failure`() { + val failingProvider = TransactionNameProvider { throw RuntimeException("provider failed") } + val successfulProvider = + object : TransactionNameProvider { + override fun provideTransactionName(request: HttpServletRequest) = "GET /users/{id}" + + override fun provideTransactionSource() = TransactionNameSource.ROUTE + } + val provider = CombinedTransactionNameProvider(listOf(failingProvider, successfulProvider)) + + val result = provider.provideTransactionNameAndSource(request) + + assertThat(result.transactionName).isEqualTo("GET /users/{id}") + assertThat(result.transactionNameSource).isEqualTo(TransactionNameSource.ROUTE) + } + + @Test + fun `uses safe fallback when none succeed after failure`() { + val failingProvider = TransactionNameProvider { throw RuntimeException("provider failed") } + val emptyProvider = TransactionNameProvider { null } + val provider = CombinedTransactionNameProvider(listOf(failingProvider, emptyProvider)) + + val result = provider.provideTransactionNameAndSource(request) + + assertThat(result.transactionName).isEqualTo(DEFAULT_TRANSACTION_NAME) + assertThat(result.transactionNameSource).isEqualTo(TransactionNameSource.CUSTOM) + } + + @Test + fun `preserves null when none resolve without failure`() { + val provider = CombinedTransactionNameProvider(listOf(TransactionNameProvider { null })) + + assertThat(provider.provideTransactionName(request)).isNull() + } + + @Test + fun `fatal provider failure propagates`() { + val failure = OutOfMemoryError("fatal") + val provider = + CombinedTransactionNameProvider(listOf(TransactionNameProvider { throw failure })) + + assertThat(assertFailsWith { provider.provideTransactionName(request) }) + .isSameAs(failure) + } +} diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt index ffdd2b9ad75..61fabb6d478 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt @@ -9,6 +9,7 @@ import io.sentry.SpanId import io.sentry.SpanStatus import io.sentry.TraceContext import io.sentry.TransactionContext +import io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME import io.sentry.TransactionOptions import io.sentry.protocol.SentryId import io.sentry.protocol.SentryTransaction @@ -140,6 +141,25 @@ class SentryTracingFilterTest { ) } + @Test + fun `transaction name provider failure uses safe fallback and request continues`() { + val filter = fixture.getSut() + val failure = RuntimeException("provider failed") + whenever(fixture.transactionNameProvider.provideTransactionNameAndSource(fixture.request)) + .thenThrow(failure) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.chain).doFilter(fixture.request, fixture.response) + verify(fixture.scopes) + .captureTransaction( + check { assertThat(it.transaction).isEqualTo(DEFAULT_TRANSACTION_NAME) }, + anyOrNull(), + anyOrNull(), + anyOrNull(), + ) + } + @Test fun `sets correct span status based on the response status`() { val filter = fixture.getSut(status = 500) diff --git a/sentry-spring/api/sentry-spring.api b/sentry-spring/api/sentry-spring.api index 7c8ff495f3b..7b36f268231 100644 --- a/sentry-spring/api/sentry-spring.api +++ b/sentry-spring/api/sentry-spring.api @@ -104,6 +104,11 @@ public final class io/sentry/spring/SpringSecuritySentryUserProvider : io/sentry public fun provideUser ()Lio/sentry/protocol/User; } +public final class io/sentry/spring/TransactionNameProviderUtils { + public static fun provideTransactionName (Lio/sentry/spring/tracing/TransactionNameProvider;Ljavax/servlet/http/HttpServletRequest;Lio/sentry/ILogger;)Ljava/lang/String; + public static fun provideTransactionNameAndSource (Lio/sentry/spring/tracing/TransactionNameProvider;Ljavax/servlet/http/HttpServletRequest;Lio/sentry/ILogger;Lio/sentry/spring/tracing/TransactionNameWithSource;)Lio/sentry/spring/tracing/TransactionNameWithSource; +} + public final class io/sentry/spring/cache/SentryCacheBeanPostProcessor : org/springframework/beans/factory/config/BeanPostProcessor, org/springframework/core/PriorityOrdered { public fun ()V public fun getOrder ()I 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 ba0586eade5..968c64ff353 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SentryExceptionResolver.java +++ b/sentry-spring/src/main/java/io/sentry/spring/SentryExceptionResolver.java @@ -75,7 +75,9 @@ protected SentryEvent createEvent( new ExceptionMechanismException(mechanism, ex, Thread.currentThread()); final SentryEvent event = new SentryEvent(throwable); event.setLevel(SentryLevel.FATAL); - event.setTransaction(transactionNameProvider.provideTransactionName(request)); + event.setTransaction( + TransactionNameProviderUtils.provideTransactionName( + transactionNameProvider, request, scopes.getOptions().getLogger())); return event; } diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentryRequestHttpServletRequestProcessor.java b/sentry-spring/src/main/java/io/sentry/spring/SentryRequestHttpServletRequestProcessor.java index 829e39b8508..047572bd46b 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SentryRequestHttpServletRequestProcessor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/SentryRequestHttpServletRequestProcessor.java @@ -2,6 +2,8 @@ import com.jakewharton.nopen.annotation.Open; import io.sentry.Hint; +import io.sentry.ILogger; +import io.sentry.ScopesAdapter; import io.sentry.SentryEvent; import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.spring.tracing.TransactionNameProvider; @@ -15,19 +17,30 @@ public class SentryRequestHttpServletRequestProcessor implements SentryEventProcessor { private final @NotNull TransactionNameProvider transactionNameProvider; private final @NotNull HttpServletRequest request; + private final @NotNull ILogger logger; public SentryRequestHttpServletRequestProcessor( final @NotNull TransactionNameProvider transactionNameProvider, final @NotNull HttpServletRequest request) { + this(transactionNameProvider, request, ScopesAdapter.getInstance().getOptions().getLogger()); + } + + SentryRequestHttpServletRequestProcessor( + final @NotNull TransactionNameProvider transactionNameProvider, + final @NotNull HttpServletRequest request, + final @NotNull ILogger logger) { this.transactionNameProvider = Objects.requireNonNull(transactionNameProvider, "transactionNameProvider is required"); this.request = Objects.requireNonNull(request, "request is required"); + this.logger = Objects.requireNonNull(logger, "logger is required"); } @Override public @NotNull SentryEvent process(final @NotNull SentryEvent event, final @NotNull Hint hint) { if (event.getTransaction() == null) { - event.setTransaction(transactionNameProvider.provideTransactionName(request)); + event.setTransaction( + TransactionNameProviderUtils.provideTransactionName( + transactionNameProvider, request, logger)); } return event; } diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentrySpringFilter.java b/sentry-spring/src/main/java/io/sentry/spring/SentrySpringFilter.java index be893b67a5a..5108238f4ad 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/SentrySpringFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/SentrySpringFilter.java @@ -91,7 +91,8 @@ private void configureScope( // transaction name is known at the later stage of request processing, thus it cannot // be set on the scope scope.addEventProcessor( - new SentryRequestHttpServletRequestProcessor(transactionNameProvider, request)); + new SentryRequestHttpServletRequestProcessor( + transactionNameProvider, request, scopes.getOptions().getLogger())); // only if request caches body, add an event processor that sets body on the event // body is not on the scope, to avoid using memory when no event is triggered during // request processing diff --git a/sentry-spring/src/main/java/io/sentry/spring/TransactionNameProviderUtils.java b/sentry-spring/src/main/java/io/sentry/spring/TransactionNameProviderUtils.java new file mode 100644 index 00000000000..9e726ae98a7 --- /dev/null +++ b/sentry-spring/src/main/java/io/sentry/spring/TransactionNameProviderUtils.java @@ -0,0 +1,51 @@ +package io.sentry.spring; + +import static io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME; + +import io.sentry.ILogger; +import io.sentry.SentryLevel; +import io.sentry.spring.tracing.TransactionNameProvider; +import io.sentry.spring.tracing.TransactionNameWithSource; +import io.sentry.util.ExceptionUtils; +import javax.servlet.http.HttpServletRequest; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +@ApiStatus.Internal +public final class TransactionNameProviderUtils { + private TransactionNameProviderUtils() {} + + public static @Nullable String provideTransactionName( + final @NotNull TransactionNameProvider provider, + final @NotNull HttpServletRequest request, + final @NotNull ILogger logger) { + try { + return provider.provideTransactionName(request); + } catch (Throwable e) { + ExceptionUtils.rethrowIfFatal(e); + logCallbackError(logger, e); + return DEFAULT_TRANSACTION_NAME; + } + } + + public static @NotNull TransactionNameWithSource provideTransactionNameAndSource( + final @NotNull TransactionNameProvider provider, + final @NotNull HttpServletRequest request, + final @NotNull ILogger logger, + final @NotNull TransactionNameWithSource fallbackOnError) { + try { + return provider.provideTransactionNameAndSource(request); + } catch (Throwable e) { + ExceptionUtils.rethrowIfFatal(e); + logCallbackError(logger, e); + return fallbackOnError; + } + } + + private static void logCallbackError( + final @NotNull ILogger logger, final @NotNull Throwable throwable) { + logger.log( + SentryLevel.ERROR, "The TransactionNameProvider callback threw an exception.", throwable); + } +} diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java index 384000b36b5..12b24963d5c 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/CombinedTransactionNameProvider.java @@ -1,6 +1,11 @@ package io.sentry.spring.tracing; +import static io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME; + +import io.sentry.ScopesAdapter; +import io.sentry.SentryLevel; import io.sentry.protocol.TransactionNameSource; +import io.sentry.util.ExceptionUtils; import java.util.List; import javax.servlet.http.HttpServletRequest; import org.jetbrains.annotations.ApiStatus; @@ -22,14 +27,20 @@ public CombinedTransactionNameProvider(final @NotNull List() private val transactionNameProvider = mock() + private val options = SentryOptions() private val request = mock() private val response = mock() + init { + whenever(scopes.options).thenReturn(options) + } + @Test fun `when handles exception, sets wrapped exception for event`() { val eventCaptor = argumentCaptor() @@ -70,6 +77,19 @@ class SentryExceptionResolverTest { verify(transactionNameProvider).provideTransactionName(request) } + @Test + fun `when transaction name provider fails, captures event with safe fallback`() { + val failure = RuntimeException("provider failed") + whenever(transactionNameProvider.provideTransactionName(any())).thenThrow(failure) + val eventCaptor = argumentCaptor() + whenever(scopes.captureEvent(eventCaptor.capture(), any())).thenReturn(null) + + SentryExceptionResolver(scopes, transactionNameProvider, 1) + .resolveException(request, response, null, RuntimeException("request failed")) + + assertThat(eventCaptor.firstValue.transaction).isEqualTo(DEFAULT_TRANSACTION_NAME) + } + @Test fun `when handles exception, provides spring resolver hint`() { val hintCaptor = argumentCaptor() diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/SentryRequestHttpServletRequestProcessorTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/SentryRequestHttpServletRequestProcessorTest.kt index 6ef40aaf3df..acccca29955 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/SentryRequestHttpServletRequestProcessorTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/SentryRequestHttpServletRequestProcessorTest.kt @@ -1,15 +1,20 @@ package io.sentry.spring import io.sentry.Hint +import io.sentry.ILogger import io.sentry.IScopes import io.sentry.SentryEvent import io.sentry.SentryOptions +import io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME import io.sentry.spring.tracing.SpringMvcTransactionNameProvider +import io.sentry.spring.tracing.TransactionNameProvider import java.net.URI import javax.servlet.http.HttpServletRequest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertNotNull +import org.assertj.core.api.Assertions.assertThat import org.mockito.kotlin.mock import org.mockito.kotlin.whenever import org.springframework.mock.web.MockServletContext @@ -19,13 +24,15 @@ import org.springframework.web.servlet.HandlerMapping class SentryRequestHttpServletRequestProcessorTest { private class Fixture { val scopes = mock() + val logger = mock() fun getSut( request: HttpServletRequest, options: SentryOptions = SentryOptions(), + provider: TransactionNameProvider = SpringMvcTransactionNameProvider(), ): SentryRequestHttpServletRequestProcessor { whenever(scopes.options).thenReturn(options) - return SentryRequestHttpServletRequestProcessor(SpringMvcTransactionNameProvider(), request) + return SentryRequestHttpServletRequestProcessor(provider, request, logger) } } @@ -46,6 +53,30 @@ class SentryRequestHttpServletRequestProcessorTest { assertEquals("GET /some-path", event.transaction) } + @Test + fun `provider failure uses safe transaction name`() { + val failure = RuntimeException("provider failed") + val request = mock() + val eventProcessor = + fixture.getSut(request, provider = TransactionNameProvider { throw failure }) + val event = SentryEvent() + + eventProcessor.process(event, Hint()) + + assertThat(event.transaction).isEqualTo(DEFAULT_TRANSACTION_NAME) + } + + @Test + fun `fatal provider failure propagates`() { + val failure = OutOfMemoryError("fatal") + val request = mock() + val eventProcessor = + fixture.getSut(request, provider = TransactionNameProvider { throw failure }) + + assertThat(assertFailsWith { eventProcessor.process(SentryEvent(), Hint()) }) + .isSameAs(failure) + } + @Test fun `when event has transaction name set, does not overwrite transaction name with value from the current request`() { val request = diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/TransactionNameProviderUtilsTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/TransactionNameProviderUtilsTest.kt new file mode 100644 index 00000000000..550a1438e82 --- /dev/null +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/TransactionNameProviderUtilsTest.kt @@ -0,0 +1,38 @@ +package io.sentry.spring + +import io.sentry.ILogger +import io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME +import io.sentry.spring.tracing.TransactionNameProvider +import javax.servlet.http.HttpServletRequest +import kotlin.test.Test +import kotlin.test.assertFailsWith +import org.assertj.core.api.Assertions.assertThat +import org.mockito.kotlin.mock + +class TransactionNameProviderUtilsTest { + private val request = mock() + private val logger = mock() + + @Test + fun `single provider failure uses error fallback`() { + val failure = RuntimeException("provider failed") + val provider = TransactionNameProvider { throw failure } + + val result = TransactionNameProviderUtils.provideTransactionName(provider, request, logger) + + assertThat(result).isEqualTo(DEFAULT_TRANSACTION_NAME) + } + + @Test + fun `single provider fatal failure propagates`() { + val failure = OutOfMemoryError("fatal") + val provider = TransactionNameProvider { throw failure } + + assertThat( + assertFailsWith { + TransactionNameProviderUtils.provideTransactionName(provider, request, logger) + } + ) + .isSameAs(failure) + } +} diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/CombinedTransactionNameProviderTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/CombinedTransactionNameProviderTest.kt new file mode 100644 index 00000000000..0c61b5f9a60 --- /dev/null +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/CombinedTransactionNameProviderTest.kt @@ -0,0 +1,59 @@ +package io.sentry.spring.tracing + +import io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME +import io.sentry.protocol.TransactionNameSource +import javax.servlet.http.HttpServletRequest +import kotlin.test.Test +import kotlin.test.assertFailsWith +import org.assertj.core.api.Assertions.assertThat +import org.mockito.kotlin.mock + +class CombinedTransactionNameProviderTest { + private val request = mock() + + @Test + fun `continues after provider failure`() { + val failingProvider = TransactionNameProvider { throw RuntimeException("provider failed") } + val successfulProvider = + object : TransactionNameProvider { + override fun provideTransactionName(request: HttpServletRequest) = "GET /users/{id}" + + override fun provideTransactionSource() = TransactionNameSource.ROUTE + } + val provider = CombinedTransactionNameProvider(listOf(failingProvider, successfulProvider)) + + val result = provider.provideTransactionNameAndSource(request) + + assertThat(result.transactionName).isEqualTo("GET /users/{id}") + assertThat(result.transactionNameSource).isEqualTo(TransactionNameSource.ROUTE) + } + + @Test + fun `uses safe fallback when none succeed after failure`() { + val failingProvider = TransactionNameProvider { throw RuntimeException("provider failed") } + val emptyProvider = TransactionNameProvider { null } + val provider = CombinedTransactionNameProvider(listOf(failingProvider, emptyProvider)) + + val result = provider.provideTransactionNameAndSource(request) + + assertThat(result.transactionName).isEqualTo(DEFAULT_TRANSACTION_NAME) + assertThat(result.transactionNameSource).isEqualTo(TransactionNameSource.CUSTOM) + } + + @Test + fun `preserves null when none resolve without failure`() { + val provider = CombinedTransactionNameProvider(listOf(TransactionNameProvider { null })) + + assertThat(provider.provideTransactionName(request)).isNull() + } + + @Test + fun `fatal provider failure propagates`() { + val failure = OutOfMemoryError("fatal") + val provider = + CombinedTransactionNameProvider(listOf(TransactionNameProvider { throw failure })) + + assertThat(assertFailsWith { provider.provideTransactionName(request) }) + .isSameAs(failure) + } +} diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt index f942da342c5..acfcdb4ebab 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt @@ -9,6 +9,7 @@ import io.sentry.SpanId import io.sentry.SpanStatus import io.sentry.TraceContext import io.sentry.TransactionContext +import io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME import io.sentry.TransactionOptions import io.sentry.protocol.SentryId import io.sentry.protocol.SentryTransaction @@ -140,6 +141,25 @@ class SentryTracingFilterTest { ) } + @Test + fun `transaction name provider failure uses safe fallback and request continues`() { + val filter = fixture.getSut() + val failure = RuntimeException("provider failed") + whenever(fixture.transactionNameProvider.provideTransactionNameAndSource(fixture.request)) + .thenThrow(failure) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.chain).doFilter(fixture.request, fixture.response) + verify(fixture.scopes) + .captureTransaction( + check { assertThat(it.transaction).isEqualTo(DEFAULT_TRANSACTION_NAME) }, + anyOrNull(), + anyOrNull(), + anyOrNull(), + ) + } + @Test fun `sets correct span status based on the response status`() { val filter = fixture.getSut(status = 500) From 109d07e4b48bd07f18b96efd2d079b3b3e69c437 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 9 Oct 2026 06:28:29 +0200 Subject: [PATCH 2/2] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc054e0be18..aa0177caf71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- Keep Spring requests and event capture running when a `TransactionNameProvider` throws ([#6241](https://github.com/getsentry/sentry-java/pull/6241)) + ### Features - Add support for Android Navigation 3 through the new `sentry-android-navigation3` library ([#6233](https://github.com/getsentry/sentry-java/pull/6233))