Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

### Fixes

- Prevent a `StackOverflowError` when a `beforeSend`, `beforeBreadcrumb`, or `beforeSendLog` callback triggers another capture (directly or through a logging integration such as Timber) ([#5737](https://github.com/getsentry/sentry-java/pull/5737))
- Captures made from within a user callback (event, transaction, breadcrumb, or log) are now dropped while that callback runs, instead of recursing. Captures made by event processors are unaffected.
- Record byte-level client reports when event processors discard logs or trace metrics ([#5718](https://github.com/getsentry/sentry-java/pull/5718))
- Name the device-info caching thread `SentryDeviceInfoCache` so all threads spawned by the SDK are identifiable ([#5684](https://github.com/getsentry/sentry-java/pull/5684))
- Apply byte-category rate limits to log and trace metric envelope items ([#5716](https://github.com/getsentry/sentry-java/pull/5716))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
package io.sentry.android.timber

import io.sentry.IScopes
import io.sentry.ITransportFactory
import io.sentry.ScopesAdapter
import io.sentry.Sentry
import io.sentry.SentryLevel
import io.sentry.SentryLogLevel
import io.sentry.SentryOptions
import io.sentry.protocol.SdkVersion
import io.sentry.transport.ITransport
import kotlin.test.BeforeTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertTrue
import org.mockito.kotlin.any
import org.mockito.kotlin.mock
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever
import timber.log.Timber

class SentryTimberIntegrationTest {
Expand Down Expand Up @@ -112,4 +117,44 @@ class SentryTimberIntegrationTest {

assertTrue(fixture.options.sdkVersion!!.integrationSet.contains("Timber"))
}

@Test
fun `a beforeSend callback that logs via Timber does not recurse`() {
// End-to-end guard against SDK-CRASHES-JAVA-3T3H style recursion: with a real Sentry instance,
// a beforeSend callback that logs through the planted SentryTimberTree must not loop back into
// capture forever.
val transport = mock<ITransport>()
val transportFactory = mock<ITransportFactory>()
whenever(transportFactory.create(any(), any())).thenReturn(transport)

var beforeSendInvocations = 0
Sentry.init { options ->
options.dsn = "https://key@sentry.io/123"
options.setTransportFactory(transportFactory)
options.beforeSend =
SentryOptions.BeforeSendCallback { event, _ ->
beforeSendInvocations++
Timber.e("logging from beforeSend")
event
}
}
Timber.plant(
SentryTimberTree(
ScopesAdapter.getInstance(),
SentryLevel.ERROR,
SentryLevel.INFO,
SentryLogLevel.INFO,
)
)

try {
Timber.e("outer error")

// Without the core re-entrancy guard this recurses until a StackOverflowError. The nested
// Timber.e is dropped before its own beforeSend, so the callback runs exactly once.
assertEquals(1, beforeSendInvocations)
} finally {
Sentry.close()
}
}
}
6 changes: 6 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -7893,6 +7893,12 @@ public final class io/sentry/util/ScopesUtil {
public static fun printScopesChain (Lio/sentry/IScopes;)V
}

public final class io/sentry/util/SentryCallbackReentrancyGuard {
public static fun enter ()V
public static fun exit ()V
public static fun isActive ()Z
}

public final class io/sentry/util/SentryRandom {
public fun <init> ()V
public static fun current ()Lio/sentry/util/Random;
Expand Down
8 changes: 8 additions & 0 deletions sentry/src/main/java/io/sentry/Scope.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import io.sentry.util.ExceptionUtils;
import io.sentry.util.Objects;
import io.sentry.util.Pair;
import io.sentry.util.SentryCallbackReentrancyGuard;
import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.Collection;
Expand Down Expand Up @@ -465,6 +466,7 @@ public Queue<Breadcrumb> getBreadcrumbs() {
@NotNull Breadcrumb breadcrumb,
final @NotNull Hint hint) {
try {
SentryCallbackReentrancyGuard.enter();
breadcrumb = callback.execute(breadcrumb, hint);
} catch (Throwable e) {
options
Expand All @@ -477,6 +479,8 @@ public Queue<Breadcrumb> getBreadcrumbs() {
if (e.getMessage() != null) {
breadcrumb.setData("sentry:message", e.getMessage());
}
} finally {
SentryCallbackReentrancyGuard.exit();
}
return breadcrumb;
}
Expand All @@ -493,6 +497,10 @@ public void addBreadcrumb(@NotNull Breadcrumb breadcrumb, @Nullable Hint hint) {
if (breadcrumb == null || breadcrumbs instanceof DisabledQueue) {
return;
}
// Drop breadcrumbs added from within a user callback to prevent recursion.
if (SentryCallbackReentrancyGuard.isActive()) {
return;
}
SentryOptions.BeforeBreadcrumbCallback callback = options.getBeforeBreadcrumb();
if (callback != null) {
if (hint == null) {
Expand Down
78 changes: 78 additions & 0 deletions sentry/src/main/java/io/sentry/SentryClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,15 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul
@NotNull SentryEvent event, final @Nullable IScope scope, @Nullable Hint hint) {
Objects.requireNonNull(event, "SentryEvent is required.");

if (SentryCallbackReentrancyGuard.isActive()) {
options
.getLogger()
.log(
SentryLevel.DEBUG,
"Event captured from within a callback (beforeSend/beforeBreadcrumb/beforeSendLog) was dropped to prevent recursion.");
return SentryId.EMPTY_ID;
}

if (hint == null) {
hint = new Hint();
}
Expand Down Expand Up @@ -237,6 +246,7 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul
options.getSessionReplay().getBeforeErrorSampling();
if (beforeErrorSampling != null) {
try {
SentryCallbackReentrancyGuard.enter();
shouldCaptureReplay = beforeErrorSampling.execute(event, hint);
} catch (Throwable e) {
options
Expand All @@ -246,6 +256,8 @@ private boolean shouldApplyScopeData(final @NotNull CheckIn event, final @NotNul
"The beforeErrorSampling callback threw an exception. Proceeding with replay capture.",
e);
shouldCaptureReplay = true;
} finally {
SentryCallbackReentrancyGuard.exit();
}
}
if (shouldCaptureReplay) {
Expand Down Expand Up @@ -300,6 +312,15 @@ private void finalizeTransaction(final @NotNull IScope scope, final @NotNull Hin
@NotNull SentryReplayEvent event, final @Nullable IScope scope, @Nullable Hint hint) {
Objects.requireNonNull(event, "SessionReplay is required.");

if (SentryCallbackReentrancyGuard.isActive()) {
options
.getLogger()
.log(
SentryLevel.DEBUG,
"Replay event captured from within a callback was dropped to prevent recursion.");
return SentryId.EMPTY_ID;
}

if (hint == null) {
hint = new Hint();
}
Expand Down Expand Up @@ -929,11 +950,14 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint
options.getBeforeEnvelopeCallback();
if (beforeEnvelopeCallback != null) {
try {
SentryCallbackReentrancyGuard.enter();
beforeEnvelopeCallback.execute(envelope, hint);
} catch (Throwable e) {
options
.getLogger()
.log(SentryLevel.ERROR, "The BeforeEnvelope callback threw an exception.", e);
} finally {
SentryCallbackReentrancyGuard.exit();
}
}

Expand All @@ -957,6 +981,15 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint
final @Nullable ProfilingTraceData profilingTraceData) {
Objects.requireNonNull(transaction, "Transaction is required.");

if (SentryCallbackReentrancyGuard.isActive()) {
options
.getLogger()
.log(
SentryLevel.DEBUG,
"Transaction captured from within a callback was dropped to prevent recursion.");
return SentryId.EMPTY_ID;
}

if (hint == null) {
hint = new Hint();
}
Expand Down Expand Up @@ -1171,6 +1204,15 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint
@Override
public @NotNull SentryId captureFeedback(
final @NotNull Feedback feedback, @Nullable Hint hint, final @NotNull IScope scope) {
if (SentryCallbackReentrancyGuard.isActive()) {
options
.getLogger()
.log(
SentryLevel.DEBUG,
"Feedback captured from within a callback was dropped to prevent recursion.");
return SentryId.EMPTY_ID;
}

SentryEvent event = new SentryEvent();
event.getContexts().setFeedback(feedback);

Expand Down Expand Up @@ -1281,6 +1323,15 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint
@ApiStatus.Experimental
@Override
public void captureLog(@Nullable SentryLogEvent logEvent, @Nullable IScope scope) {
if (SentryCallbackReentrancyGuard.isActive()) {
options
.getLogger()
.log(
SentryLevel.DEBUG,
"Log captured from within a callback was dropped to prevent recursion.");
return;
}

if (logEvent != null && scope != null) {
logEvent = processLogEvent(logEvent, scope.getEventProcessors());
if (logEvent == null) {
Expand Down Expand Up @@ -1335,6 +1386,15 @@ public void captureMetric(
@Nullable SentryMetricsEvent metricsEvent,
final @Nullable IScope scope,
@Nullable Hint hint) {
if (SentryCallbackReentrancyGuard.isActive()) {
options
.getLogger()
.log(
SentryLevel.DEBUG,
"Metric captured from within a callback was dropped to prevent recursion.");
return;
}

if (hint == null) {
hint = new Hint();
}
Expand Down Expand Up @@ -1595,6 +1655,7 @@ private void sortBreadcrumbsByDate(
final SentryOptions.BeforeSendCallback beforeSend = options.getBeforeSend();
if (beforeSend != null) {
try {
SentryCallbackReentrancyGuard.enter();
event = beforeSend.execute(event, hint);
} catch (Throwable e) {
options
Expand All @@ -1606,6 +1667,8 @@ private void sortBreadcrumbsByDate(

// drop event in case of an error in beforeSend due to PII concerns
event = null;
} finally {
SentryCallbackReentrancyGuard.exit();
}
}
return event;
Expand All @@ -1617,6 +1680,7 @@ private void sortBreadcrumbsByDate(
options.getBeforeSendTransaction();
if (beforeSendTransaction != null) {
try {
SentryCallbackReentrancyGuard.enter();
transaction = beforeSendTransaction.execute(transaction, hint);
} catch (Throwable e) {
options
Expand All @@ -1628,6 +1692,8 @@ private void sortBreadcrumbsByDate(

// drop transaction in case of an error in beforeSend due to PII concerns
transaction = null;
} finally {
SentryCallbackReentrancyGuard.exit();
}
}
return transaction;
Expand All @@ -1638,6 +1704,7 @@ private void sortBreadcrumbsByDate(
final SentryOptions.BeforeSendCallback beforeSendFeedback = options.getBeforeSendFeedback();
if (beforeSendFeedback != null) {
try {
SentryCallbackReentrancyGuard.enter();
event = beforeSendFeedback.execute(event, hint);
} catch (Throwable e) {
options
Expand All @@ -1646,6 +1713,8 @@ private void sortBreadcrumbsByDate(

// drop feedback in case of an error in beforeSend due to PII concerns
event = null;
} finally {
SentryCallbackReentrancyGuard.exit();
}
}
return event;
Expand All @@ -1656,6 +1725,7 @@ private void sortBreadcrumbsByDate(
final SentryOptions.BeforeSendReplayCallback beforeSendReplay = options.getBeforeSendReplay();
if (beforeSendReplay != null) {
try {
SentryCallbackReentrancyGuard.enter();
event = beforeSendReplay.execute(event, hint);
} catch (Throwable e) {
options
Expand All @@ -1667,6 +1737,8 @@ private void sortBreadcrumbsByDate(

// drop event in case of an error in beforeSend due to PII concerns
event = null;
} finally {
SentryCallbackReentrancyGuard.exit();
}
}
return event;
Expand All @@ -1677,6 +1749,7 @@ private void sortBreadcrumbsByDate(
options.getLogs().getBeforeSend();
if (beforeSendLog != null) {
try {
SentryCallbackReentrancyGuard.enter();
event = beforeSendLog.execute(event);
} catch (Throwable e) {
options
Expand All @@ -1688,6 +1761,8 @@ private void sortBreadcrumbsByDate(

// drop event in case of an error in beforeSendLog due to PII concerns
event = null;
} finally {
SentryCallbackReentrancyGuard.exit();
}
}
return event;
Comment on lines 1761 to 1768

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Public methods like captureCheckIn and captureProfileChunk lack the isActive() guard, allowing nested captures from callbacks when they should be dropped.
Severity: HIGH

Suggested Fix

Add the if (!isActive()) { return; } guard at the beginning of the public methods captureCheckIn, captureProfileChunk, and captureEnvelope to ensure that nested calls from within callbacks are correctly dropped.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: sentry/src/main/java/io/sentry/SentryClient.java#L1761-L1768

Potential issue: Several public API methods, including `captureCheckIn`,
`captureProfileChunk`, and `captureEnvelope`, are missing an `isActive()` check at their
entry points. This violates the intended invariant that any new captures initiated from
within a user callback (e.g., `beforeSend`) should be dropped. If a user calls one of
these unguarded methods from a callback, the nested capture will be processed and sent
instead of being ignored. While the internal depth counter prevents recursive callback
execution, it does not prevent the initial capture from proceeding through the pipeline,
leading to unexpected behavior and data being sent when it should be suppressed.

Also affects:

  • sentry/src/main/java/io/sentry/SentryClient.java:1713~1720
  • sentry/src/main/java/io/sentry/SentryClient.java:1725~1731
  • sentry/src/main/java/io/sentry/SentryClient.java:1774~1780
  • sentry/src/main/java/io/sentry/SentryClient.java:1786~1793

Did we get this right? 👍 / 👎 to inform future reviews.

Expand All @@ -1699,6 +1774,7 @@ private void sortBreadcrumbsByDate(
options.getMetrics().getBeforeSend();
if (beforeSendMetric != null) {
try {
SentryCallbackReentrancyGuard.enter();
event = beforeSendMetric.execute(event, hint);
} catch (Throwable e) {
options
Expand All @@ -1710,6 +1786,8 @@ private void sortBreadcrumbsByDate(

// drop event in case of an error in beforeSendMetric due to PII concerns
event = null;
} finally {
SentryCallbackReentrancyGuard.exit();
}
}
return event;
Expand Down
Loading
Loading