fix(spring): Handle transaction name provider failures

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Fixes
 
+- Keep Spring requests and event capture running when a `TransactionNameProvider` throws ([#6241](https://github.com/getsentry/sentry-java/pull/6241))
 - Keep the Android feedback form available when an options configurator throws ([#6239](https://github.com/getsentry/sentry-java/pull/6239))
 - Keep Spring requests running when a `SentryUserProvider` throws and discard the incomplete user identity ([#6240](https://github.com/getsentry/sentry-java/pull/6240))
 - Drop Apollo 5 spans when `beforeSpan` throws without disrupting the GraphQL request ([#6238](https://github.com/getsentry/sentry-java/pull/6238))

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 <init> ()V
 	public fun getOrder ()I

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

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

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

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

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<TransactionNameProvid
   }
 
   @Override
-  public @Nullable String provideTransactionName(@NotNull HttpServletRequest request) {
-    for (TransactionNameProvider provider : providers) {
-      String transactionName = provider.provideTransactionName(request);
-      if (transactionName != null) {
-        return transactionName;
+  public @Nullable String provideTransactionName(final @NotNull HttpServletRequest request) {
+    boolean callbackFailed = false;
+    for (final TransactionNameProvider provider : providers) {
+      try {
+        final @Nullable String transactionName = provider.provideTransactionName(request);
+        if (transactionName != null) {
+          return transactionName;
+        }
+      } catch (Throwable e) {
+        ExceptionUtils.rethrowIfFatal(e);
+        logCallbackError(e);
+        callbackFailed = true;
       }
     }
-
-    return null;
+    return callbackFailed ? DEFAULT_TRANSACTION_NAME : null;
   }
 
   @Override
@@ -42,14 +53,32 @@ public CombinedTransactionNameProvider(final @NotNull List<TransactionNameProvid
   @Override
   public @NotNull TransactionNameWithSource provideTransactionNameAndSource(
       @NotNull HttpServletRequest request) {
-    for (TransactionNameProvider provider : providers) {
-      String transactionName = provider.provideTransactionName(request);
-      if (transactionName != null) {
-        final @NotNull TransactionNameSource source = provider.provideTransactionSource();
-        return new TransactionNameWithSource(transactionName, source);
+    boolean callbackFailed = false;
+    for (final TransactionNameProvider provider : providers) {
+      try {
+        final @Nullable String transactionName = provider.provideTransactionName(request);
+        if (transactionName != null) {
+          return new TransactionNameWithSource(
+              transactionName, provider.provideTransactionSource());
+        }
+      } catch (Throwable e) {
+        ExceptionUtils.rethrowIfFatal(e);
+        logCallbackError(e);
+        callbackFailed = true;
       }
     }
+    return callbackFailed
+        ? new TransactionNameWithSource(DEFAULT_TRANSACTION_NAME, TransactionNameSource.CUSTOM)
+        : new TransactionNameWithSource(null, TransactionNameSource.CUSTOM);
+  }
 
-    return new TransactionNameWithSource(null, TransactionNameSource.CUSTOM);
+  private static void logCallbackError(final @NotNull Throwable throwable) {
+    ScopesAdapter.getInstance()
+        .getOptions()
+        .getLogger()
+        .log(
+            SentryLevel.ERROR,
+            "The TransactionNameProvider callback threw an exception.",
+            throwable);
   }
 }

sentry-spring-7/src/main/java/io/sentry/spring7/tracing/SentryTracingFilter.java

@@ -1,5 +1,7 @@
 package io.sentry.spring7.tracing;
 
+import static io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME;
+
 import com.jakewharton.nopen.annotation.Open;
 import io.sentry.BaggageHeader;
 import io.sentry.CustomSamplingContext;
@@ -11,6 +13,7 @@
 import io.sentry.TransactionContext;
 import io.sentry.TransactionOptions;
 import io.sentry.protocol.TransactionNameSource;
+import io.sentry.spring7.TransactionNameProviderUtils;
 import io.sentry.util.Objects;
 import io.sentry.util.SpanUtils;
 import jakarta.servlet.FilterChain;
@@ -38,6 +41,8 @@ public class SentryTracingFilter extends OncePerRequestFilter {
 
   private static final String TRACE_ORIGIN = "auto.http.spring7.webmvc";
   private static final String TRANSACTION_ATTR = "sentry.transaction";
+  private static final @NotNull TransactionNameWithSource CALLBACK_ERROR_TRANSACTION_NAME =
+      new TransactionNameWithSource(DEFAULT_TRANSACTION_NAME, TransactionNameSource.CUSTOM);
 
   private final @NotNull TransactionNameProvider transactionNameProvider;
   private final @NotNull IScopes scopes;
@@ -146,7 +151,11 @@ private void doFilterWithTransaction(
       if (shouldFinishTransaction(httpRequest) && transaction != null) {
         // after all filters run, templated path pattern is available in request attribute
         final @NotNull TransactionNameWithSource transactionNameWithSource =
-            transactionNameProvider.provideTransactionNameAndSource(httpRequest);
+            TransactionNameProviderUtils.provideTransactionNameAndSource(
+                transactionNameProvider,
+                httpRequest,
+                scopes.getOptions().getLogger(),
+                CALLBACK_ERROR_TRANSACTION_NAME);
         final @Nullable String transactionName = transactionNameWithSource.getTransactionName();
         final @NotNull TransactionNameSource transactionNameSource =
             transactionNameWithSource.getTransactionNameSource();

sentry-spring-7/src/test/kotlin/io/sentry/spring7/SentryExceptionResolverTest.kt

@@ -4,6 +4,8 @@ import io.sentry.Hint
 import io.sentry.IScopes
 import io.sentry.SentryEvent
 import io.sentry.SentryLevel
+import io.sentry.SentryOptions
+import io.sentry.TransactionContext.DEFAULT_TRANSACTION_NAME
 import io.sentry.exception.ExceptionMechanismException
 import io.sentry.spring7.tracing.TransactionNameProvider
 import jakarta.servlet.http.HttpServletRequest
@@ -19,10 +21,15 @@ import org.mockito.kotlin.whenever
 class SentryExceptionResolverTest {
   private val scopes = mock<IScopes>()
   private val transactionNameProvider = mock<TransactionNameProvider>()
+  private val options = SentryOptions()
 
   private val request = mock<HttpServletRequest>()
   private val response = mock<HttpServletResponse>()
 
+  init {
+    whenever(scopes.options).thenReturn(options)
+  }
+
   @Test
   fun `when handles exception, sets wrapped exception for event`() {
     val eventCaptor = argumentCaptor<SentryEvent>()
@@ -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<SentryEvent>()
+    whenever(scopes.captureEvent(eventCaptor.capture(), any<Hint>())).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<Hint>()

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<IScopes>()
+    val logger = mock<ILogger>()
 
     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<HttpServletRequest>()
+    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<HttpServletRequest>()
+    val eventProcessor =
+      fixture.getSut(request, provider = TransactionNameProvider { throw failure })
+
+    assertThat(assertFailsWith<OutOfMemoryError> { 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 =