diff --git a/CHANGELOG.md b/CHANGELOG.md index 8436b20dec..ebdd0bb55a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - Fix visionOS compilation ([#6676](https://github.com/getsentry/sentry-react-native/pull/6676)) - A throwing `beforeBreadcrumb` now drops the breadcrumb, and a throwing `tracesSampler` now falls back to the configured `tracesSampleRate` ([#6675](https://github.com/getsentry/sentry-react-native/pull/6675)) +- Stop uploading on-error Session Replays for errors dropped by `sampleRate` ([#6685](https://github.com/getsentry/sentry-react-native/pull/6685)) ### Internal diff --git a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index 63ba50cfca..e2bfbaf159 100644 --- a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -481,10 +481,29 @@ public void fetchNativeFramesDelay( public void captureReplay(boolean isHardCrash, Promise promise) { Sentry.getCurrentScopes().getOptions().getReplayController().captureReplay(isHardCrash); - promise.resolve(getCurrentReplayId()); + // Resolve with the scope's replayId, which is populated only when a replay + // was actually sent. Returning the controller's buffered id here would make + // JS treat an on-error sampling miss as a successful flush, diverging from + // iOS (which resolves nil unless a replay was captured). + promise.resolve(getReplayIdFromScope()); } public @Nullable String getCurrentReplayId() { + // Prefer the replay controller's id: it is assigned when recording starts + // (buffer or session) and is therefore available BEFORE a replay is + // flushed, so a buffered (on-error) replay can be linked to the event in + // `beforeSend`. The scope's replayId is only populated once a replay is + // sent, so it stays empty while a buffer replay is recording (issue #6598). + final @NotNull SentryId controllerId = + Sentry.getCurrentScopes().getOptions().getReplayController().getReplayId(); + if (controllerId != SentryId.EMPTY_ID) { + return controllerId.toString(); + } + + return getReplayIdFromScope(); + } + + private @Nullable String getReplayIdFromScope() { final @Nullable IScope scope = InternalSentrySdk.getCurrentScope(); if (scope == null) { return null; diff --git a/packages/core/android/src/test/java/io/sentry/react/RNSentryReplayIdTest.java b/packages/core/android/src/test/java/io/sentry/react/RNSentryReplayIdTest.java new file mode 100644 index 0000000000..c621837fc1 --- /dev/null +++ b/packages/core/android/src/test/java/io/sentry/react/RNSentryReplayIdTest.java @@ -0,0 +1,166 @@ +package io.sentry.react; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.content.pm.PackageInfo; +import android.content.pm.PackageManager; +import com.facebook.react.bridge.Promise; +import com.facebook.react.bridge.ReactApplicationContext; +import io.sentry.IScope; +import io.sentry.IScopes; +import io.sentry.ReplayController; +import io.sentry.Sentry; +import io.sentry.SentryOptions; +import io.sentry.android.core.InternalSentrySdk; +import io.sentry.protocol.SentryId; +import org.junit.Before; +import org.junit.Test; +import org.mockito.MockedStatic; + +/** + * Coverage for {@link RNSentryModuleImpl#getCurrentReplayId()} and its buffer (on-error) replay + * lookup added for https://github.com/getsentry/sentry-react-native/issues/6598. + * + *

A buffer replay's id is assigned by the {@link ReplayController} when recording starts, but + * the scope's replayId is only populated once a replay is sent. The JS mobile replay integration + * must be able to read the buffered id BEFORE the replay is flushed, so {@code + * getCurrentReplayId()} prefers the controller's id and only falls back to the scope. + */ +public class RNSentryReplayIdTest { + + private RNSentryModuleImpl module; + + @Before + public void setUp() throws Exception { + ReactApplicationContext reactContext = mock(ReactApplicationContext.class); + PackageManager packageManager = mock(PackageManager.class); + when(packageManager.getPackageInfo(anyString(), anyInt())).thenReturn(new PackageInfo()); + when(reactContext.getPackageManager()).thenReturn(packageManager); + when(reactContext.getPackageName()).thenReturn("com.test.app"); + module = new RNSentryModuleImpl(reactContext); + } + + /** + * Wires {@code Sentry.getCurrentScopes().getOptions().getReplayController()} to return the id and + * hands back the mocked controller so callers can verify interactions with it. + */ + private ReplayController stubControllerReplayId( + final MockedStatic sentry, final SentryId id) { + final ReplayController replayController = mock(ReplayController.class); + when(replayController.getReplayId()).thenReturn(id); + final SentryOptions options = mock(SentryOptions.class); + when(options.getReplayController()).thenReturn(replayController); + final IScopes scopes = mock(IScopes.class); + when(scopes.getOptions()).thenReturn(options); + sentry.when(Sentry::getCurrentScopes).thenReturn(scopes); + return replayController; + } + + @Test + public void prefersReplayControllerIdWhenBuffering() { + // A buffer replay is recording: the controller exposes its id even though the scope has none. + final SentryId bufferedId = new SentryId(); + + try (MockedStatic sentry = mockStatic(Sentry.class); + MockedStatic internal = mockStatic(InternalSentrySdk.class)) { + stubControllerReplayId(sentry, bufferedId); + // Scope has no replay id yet — the fix must not depend on it. + final IScope scope = mock(IScope.class); + when(scope.getReplayId()).thenReturn(SentryId.EMPTY_ID); + internal.when(InternalSentrySdk::getCurrentScope).thenReturn(scope); + + assertEquals(bufferedId.toString(), module.getCurrentReplayId()); + } + } + + @Test + public void fallsBackToScopeIdWhenControllerEmpty() { + // A full-session replay was sent: the controller reports empty, the id lives on the scope. + final SentryId scopeId = new SentryId(); + + try (MockedStatic sentry = mockStatic(Sentry.class); + MockedStatic internal = mockStatic(InternalSentrySdk.class)) { + stubControllerReplayId(sentry, SentryId.EMPTY_ID); + final IScope scope = mock(IScope.class); + when(scope.getReplayId()).thenReturn(scopeId); + internal.when(InternalSentrySdk::getCurrentScope).thenReturn(scope); + + assertEquals(scopeId.toString(), module.getCurrentReplayId()); + } + } + + @Test + public void returnsNullWhenControllerEmptyAndScopeEmpty() { + try (MockedStatic sentry = mockStatic(Sentry.class); + MockedStatic internal = mockStatic(InternalSentrySdk.class)) { + stubControllerReplayId(sentry, SentryId.EMPTY_ID); + final IScope scope = mock(IScope.class); + when(scope.getReplayId()).thenReturn(SentryId.EMPTY_ID); + internal.when(InternalSentrySdk::getCurrentScope).thenReturn(scope); + + assertNull(module.getCurrentReplayId()); + } + } + + @Test + public void returnsNullWhenControllerEmptyAndScopeNull() { + try (MockedStatic sentry = mockStatic(Sentry.class); + MockedStatic internal = mockStatic(InternalSentrySdk.class)) { + stubControllerReplayId(sentry, SentryId.EMPTY_ID); + internal.when(InternalSentrySdk::getCurrentScope).thenReturn(null); + + assertNull(module.getCurrentReplayId()); + } + } + + @Test + public void captureReplayResolvesNullOnSamplingMissEvenWhileBuffering() { + // On an on-error sampling miss the controller still holds the buffered id, but the scope has + // none because nothing was uploaded. captureReplay must resolve null (matching iOS), rather + // than leak the buffered id as if a replay had been sent. + final SentryId bufferedId = new SentryId(); + + try (MockedStatic sentry = mockStatic(Sentry.class); + MockedStatic internal = mockStatic(InternalSentrySdk.class)) { + final ReplayController replayController = stubControllerReplayId(sentry, bufferedId); + final IScope scope = mock(IScope.class); + when(scope.getReplayId()).thenReturn(SentryId.EMPTY_ID); + internal.when(InternalSentrySdk::getCurrentScope).thenReturn(scope); + + final Promise promise = mock(Promise.class); + module.captureReplay(true, promise); + + verify(replayController).captureReplay(true); + verify(promise).resolve(null); + } + } + + @Test + public void captureReplayResolvesScopeIdWhenReplayWasSent() { + // When a replay is actually sent the scope carries its id; captureReplay resolves that, not the + // controller's id, so JS learns the real uploaded replay id. + final SentryId bufferedId = new SentryId(); + final SentryId scopeId = new SentryId(); + + try (MockedStatic sentry = mockStatic(Sentry.class); + MockedStatic internal = mockStatic(InternalSentrySdk.class)) { + final ReplayController replayController = stubControllerReplayId(sentry, bufferedId); + final IScope scope = mock(IScope.class); + when(scope.getReplayId()).thenReturn(scopeId); + internal.when(InternalSentrySdk::getCurrentScope).thenReturn(scope); + + final Promise promise = mock(Promise.class); + module.captureReplay(false, promise); + + verify(replayController).captureReplay(false); + verify(promise).resolve(scopeId.toString()); + } + } +} diff --git a/packages/core/src/js/replay/mobilereplay.ts b/packages/core/src/js/replay/mobilereplay.ts index 2ea2bed89f..3129542af7 100644 --- a/packages/core/src/js/replay/mobilereplay.ts +++ b/packages/core/src/js/replay/mobilereplay.ts @@ -345,7 +345,25 @@ export const mobileReplayIntegration = (initOptions: MobileReplayOptions = defau return nativeReplayId; } - async function processEvent(event: ErrorEvent, hint: EventHint): Promise { + // Error `sampleRate` sampling runs AFTER `beforeSend` in `@sentry/core` + // (since 10.70.0, getsentry/sentry-javascript#22819). Flushing the buffered + // replay inside `beforeSend` therefore uploads a replay even for errors that + // are then dropped by `sampleRate`, orphaning the replay (issue #6598). + // + // The work is split in two: + // 1. `tagEventWithReplayId` runs in the `beforeSend` wrapper and only links + // the event to the buffered replay id (no flush). + // 2. `flushReplayForSentEvent` runs in `afterSendEvent`, which fires only + // for events that survive sampling and are actually sent, and performs + // the native flush there. + // + // Trade-off: the link is tagged optimistically from the buffered id before the + // native `replaysOnErrorSampleRate` roll (which still happens at flush time in + // `captureReplay`). If that roll misses, the event carries a `replay_id` for a + // replay that is never uploaded. A fully-correct fix requires the native SDKs + // to decouple the on-error sampling decision from the buffer upload. + + function tagEventWithReplayId(event: ErrorEvent, hint: EventHint): ErrorEvent { const hasException = event.exception?.values && event.exception.values.length > 0; if (!hasException) { // Event is not an error, will not capture replay @@ -370,39 +388,58 @@ export const mobileReplayIntegration = (initOptions: MobileReplayOptions = defau } } - const replayId = await NATIVE.captureReplay(isHardCrash(event)); + // Read the buffered replay id WITHOUT flushing. The native bridge returns + // the id assigned when recording started (buffer or full session), so the + // event can be linked to the replay that will be flushed after sampling. + const replayId = NATIVE.getCurrentReplayId(); if (replayId) { updateCachedReplayId(replayId); - debug.log( - `[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} Captured recording replay ${replayId} for event ${event.event_id}.`, - ); // Add replay_id to error event contexts to link replays to events/traces event.contexts = event.contexts || {}; event.contexts.replay = { ...event.contexts.replay, replay_id: replayId, }; + debug.log( + `[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} linked replay ${replayId} to event ${event.event_id}; flush deferred until after sampling.`, + ); } else { - // Check if there's an ongoing recording and update cache if found - const recordingReplayId = NATIVE.getCurrentReplayId(); - if (recordingReplayId) { - updateCachedReplayId(recordingReplayId); + updateCachedReplayId(null); + debug.log(`[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} no active recording for event ${event.event_id}.`); + } + + return event; + } + + async function flushReplayForSentEvent(event: Event): Promise { + const eventId = event.event_id; + // Only flush for events that were linked to a buffered replay in + // `beforeSend`. The link lives on the event itself, so no shared bookkeeping + // is needed: events dropped by sampling never reach this hook and cannot + // affect the flush decision for other events. + if (!eventId || !event.contexts?.replay?.replay_id) { + return; + } + + try { + const replayId = await NATIVE.captureReplay(isHardCrash(event)); + if (replayId) { + updateCachedReplayId(replayId); debug.log( - `[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} assign already recording replay ${recordingReplayId} for event ${event.event_id}.`, + `[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} flushed recording replay ${replayId} for sent event ${eventId}.`, ); - // Add replay_id to error event contexts to link replays to events/traces - event.contexts = event.contexts || {}; - event.contexts.replay = { - ...event.contexts.replay, - replay_id: recordingReplayId, - }; } else { - updateCachedReplayId(null); - debug.log(`[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} not sampled for event ${event.event_id}.`); + // No replay was uploaded (e.g. an on-error sampling miss). Re-read the + // current recording id so the cache stops exposing an id that was never + // uploaded; it resolves to the still-active buffer id, or null. + updateCachedReplayId(NATIVE.getCurrentReplayId()); + debug.log( + `[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} not sampled for event ${eventId} (replaysOnErrorSampleRate).`, + ); } + } catch (error) { + debug.error(`[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} Failed to flush replay for sent event ${eventId}`, error); } - - return event; } function setup(client: Client): void { @@ -498,12 +535,21 @@ export const mobileReplayIntegration = (initOptions: MobileReplayOptions = defau } } try { - return await processEvent(result, hint); + return tagEventWithReplayId(result, hint); } catch (error) { - debug.error(`[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} Failed to process event for replay`, error); + debug.error(`[Sentry] ${MOBILE_REPLAY_INTEGRATION_NAME} Failed to link event to replay`, error); return result; } }; + + // Flush the buffered replay only for events that survive sampling. This hook + // fires after the error `sampleRate` roll in `@sentry/core`, so an error + // dropped by sampling never triggers a replay upload (issue #6598). + client.on('afterSendEvent', (event: Event) => { + flushReplayForSentEvent(event).then(undefined, () => { + // errors are logged inside flushReplayForSentEvent + }); + }); } function getReplayId(): string | null { diff --git a/packages/core/test/replay/mobilereplay.sampling.test.ts b/packages/core/test/replay/mobilereplay.sampling.test.ts new file mode 100644 index 0000000000..37ec7c7d7a --- /dev/null +++ b/packages/core/test/replay/mobilereplay.sampling.test.ts @@ -0,0 +1,128 @@ +import type { Event } from '@sentry/core'; + +import { afterEach, beforeEach, describe, expect, it, jest } from '@jest/globals'; +import { Client, createTransport } from '@sentry/core'; + +import { mobileReplayIntegration } from '../../src/js/replay/mobilereplay'; +import * as environment from '../../src/js/utils/environment'; +import { NATIVE } from '../../src/js/wrapper'; + +jest.mock('../../src/js/wrapper'); + +/** + * Regression coverage for https://github.com/getsentry/sentry-react-native/issues/6598 + * + * Drives the REAL @sentry/core capture pipeline so the production ordering + * applies: _prepareEvent -> beforeSend -> sampleRate drop -> sendEvent + * (-> afterSendEvent). The mobile replay integration links the event to the + * buffered replay id inside the wrapped `beforeSend`, but defers the native + * replay flush to `afterSendEvent`, which only fires for events that survive + * sampling and are actually sent. So an error dropped by `sampleRate` must not + * flush (and orphan) a replay. + */ + +const BUFFERED_REPLAY_ID = 'buffered-replay-id'; + +// Minimal concrete client over the real @sentry/core base pipeline. +class TestClient extends Client { + public eventFromException(exception: any): PromiseLike { + return Promise.resolve({ + event_id: 'test-event-id', + exception: { values: [{ type: 'Error', value: String(exception?.message ?? exception) }] }, + }); + } + public eventFromMessage(message: string): PromiseLike { + return Promise.resolve({ event_id: 'test-event-id', message }); + } +} + +function makeClient(sampleRate: number, sentEnvelopes: unknown[]): TestClient { + return new TestClient({ + dsn: 'https://public@example.com/1', + enableSend: true, + sampleRate, + integrations: [], + stackParser: () => [], + transport: opts => + createTransport(opts, req => { + sentEnvelopes.push(req.body); + return Promise.resolve({ statusCode: 200 }); + }), + }); +} + +describe('Issue 6598 — on-error replay must not orphan when sampleRate drops the error', () => { + let mockCaptureReplay: jest.MockedFunction; + let mockGetCurrentReplayId: jest.MockedFunction; + + beforeEach(() => { + jest.clearAllMocks(); + jest.spyOn(environment, 'isExpoGo').mockReturnValue(false); + jest.spyOn(environment, 'notMobileOs').mockReturnValue(false); + mockCaptureReplay = NATIVE.captureReplay as jest.MockedFunction; + mockCaptureReplay.mockResolvedValue('test-replay-id'); + mockGetCurrentReplayId = NATIVE.getCurrentReplayId as jest.MockedFunction; + // A buffer (on-error) replay is recording: the native bridge returns the + // buffered id, but nothing has been flushed/uploaded yet. + mockGetCurrentReplayId.mockReturnValue(BUFFERED_REPLAY_ID); + }); + + afterEach(() => jest.restoreAllMocks()); + + it('control: sampleRate = 1.0 → event sent, replay flushed, and event linked to the replay', async () => { + const sent: unknown[] = []; + const client = makeClient(1.0, sent); + + let sentEvent: Event | undefined; + client.on('beforeSendEvent', (event: Event) => { + sentEvent = event; + }); + + const integration = mobileReplayIntegration(); + integration.setup?.(client); + + client.captureException(new Error('boom')); + await client.flush(2000); + + // Event survived sampling and reached the transport... + expect(sent.length).toBe(1); + // ...the event was linked to the buffered replay in beforeSend... + expect(sentEvent?.contexts?.replay?.replay_id).toBe(BUFFERED_REPLAY_ID); + // ...and the buffered replay was flushed exactly once (in afterSendEvent). + expect(mockCaptureReplay).toHaveBeenCalledTimes(1); + }); + + it('fix: sampleRate = 0.0 → event dropped and replay is NOT flushed (no orphan)', async () => { + const sent: unknown[] = []; + const client = makeClient(0.0, sent); // 0.0 => always dropped by sampleRate + + const integration = mobileReplayIntegration(); + integration.setup?.(client); + + client.captureException(new Error('boom')); + await client.flush(2000); + + // The event never reached the transport (dropped by sampleRate after beforeSend)... + expect(sent.length).toBe(0); + // ...and crucially the native replay buffer was NOT flushed, so no dangling + // replay is uploaded and no replay quota is burned. This is the fix. + expect(mockCaptureReplay).not.toHaveBeenCalled(); + }); + + it('does not flush when there is no active recording', async () => { + mockGetCurrentReplayId.mockReturnValue(null); + const sent: unknown[] = []; + const client = makeClient(1.0, sent); + + const integration = mobileReplayIntegration(); + integration.setup?.(client); + + client.captureException(new Error('boom')); + await client.flush(2000); + + // Event is still sent, but with no buffered replay there is nothing to link + // or flush. + expect(sent.length).toBe(1); + expect(mockCaptureReplay).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/test/replay/mobilereplay.test.ts b/packages/core/test/replay/mobilereplay.test.ts index f7421d29ee..f13e604ba9 100644 --- a/packages/core/test/replay/mobilereplay.test.ts +++ b/packages/core/test/replay/mobilereplay.test.ts @@ -49,8 +49,22 @@ describe('Mobile Replay Integration', () => { jest.restoreAllMocks(); }); + // Let deferred async work (the native flush in `afterSendEvent`) settle. + const flushAsync = (): Promise => new Promise(resolve => setImmediate(resolve)); + + // The native replay flush now happens in the `afterSendEvent` client hook, + // which only fires for events that survive sampling and are actually sent. + // Fire it explicitly to simulate an event being sent, then wait for the + // deferred flush to settle. + async function fireAfterSendEvent(event: Event): Promise { + const call = mockOn.mock.calls.find(c => c[0] === 'afterSendEvent'); + const handler = call?.[1] as ((event: Event, response?: unknown) => void) | undefined; + handler?.(event); + await flushAsync(); + } + describe('beforeSend wrapping', () => { - it('should capture replay after beforeSend processes the event', async () => { + it('links the event to the buffered replay in beforeSend and flushes it on send', async () => { const integration = mobileReplayIntegration(); integration.setup?.(mockClient); @@ -64,9 +78,14 @@ describe('Mobile Replay Integration', () => { const result = await clientOptions.beforeSend?.(event, hint); + // beforeSend only links the event to the buffered replay; it does not flush. expect(result).toBeDefined(); - expect(mockCaptureReplay).toHaveBeenCalled(); expect(result?.contexts?.replay?.replay_id).toBe('test-replay-id'); + expect(mockCaptureReplay).not.toHaveBeenCalled(); + + // The flush happens once the event survives sampling and is sent. + await fireAfterSendEvent(result as Event); + expect(mockCaptureReplay).toHaveBeenCalledTimes(1); }); it('should not capture replay when beforeSend returns null', async () => { @@ -115,9 +134,11 @@ describe('Mobile Replay Integration', () => { expect(result).toBeDefined(); expect(userBeforeSend).toHaveBeenCalledWith(event, hint); - expect(mockCaptureReplay).toHaveBeenCalled(); expect(result?.tags).toEqual({ modified: 'true' }); expect(result?.contexts?.replay?.replay_id).toBe('test-replay-id'); + + await fireAfterSendEvent(result as Event); + expect(mockCaptureReplay).toHaveBeenCalled(); }); it('should work when no user beforeSend is provided', async () => { @@ -135,8 +156,10 @@ describe('Mobile Replay Integration', () => { const result = await clientOptions.beforeSend?.(event, hint); expect(result).toBeDefined(); - expect(mockCaptureReplay).toHaveBeenCalled(); expect(result?.contexts?.replay?.replay_id).toBe('test-replay-id'); + + await fireAfterSendEvent(result as Event); + expect(mockCaptureReplay).toHaveBeenCalled(); }); it('should not process non-error events', async () => { @@ -156,9 +179,11 @@ describe('Mobile Replay Integration', () => { expect(result?.contexts?.replay).toBeUndefined(); }); - it('should handle errors in processEvent and return original event', async () => { - // Mock captureReplay to throw an error BEFORE setting up integration - mockCaptureReplay.mockRejectedValue(new Error('Native bridge error')); + it('should handle errors while linking the replay and return the original event', async () => { + // First call (during setup) succeeds; the call inside beforeSend throws. + mockGetCurrentReplayId.mockReturnValueOnce('test-replay-id').mockImplementation(() => { + throw new Error('Native bridge error'); + }); const integration = mobileReplayIntegration(); integration.setup?.(mockClient); @@ -173,10 +198,9 @@ describe('Mobile Replay Integration', () => { const result = await clientOptions.beforeSend?.(event, hint); - // Should return the original event even when processEvent fails + // Should return the original event even when linking fails expect(result).toBeDefined(); expect(result?.event_id).toBe('test-event-id'); - expect(mockCaptureReplay).toHaveBeenCalled(); }); it('should not crash the event pipeline when processEvent throws', async () => { @@ -219,6 +243,8 @@ describe('Mobile Replay Integration', () => { expect(result).toBeDefined(); expect(beforeErrorSampling).toHaveBeenCalledWith(event, hint); + + await fireAfterSendEvent(result as Event); expect(mockCaptureReplay).toHaveBeenCalled(); }); @@ -262,6 +288,8 @@ describe('Mobile Replay Integration', () => { expect(result).toBeDefined(); expect(beforeErrorSampling).toHaveBeenCalledWith(event, hint); + + await fireAfterSendEvent(result as Event); expect(mockCaptureReplay).toHaveBeenCalled(); }); @@ -280,6 +308,8 @@ describe('Mobile Replay Integration', () => { const result = await clientOptions.beforeSend?.(event, hint); expect(result).toBeDefined(); + + await fireAfterSendEvent(result as Event); expect(mockCaptureReplay).toHaveBeenCalled(); }); @@ -292,6 +322,11 @@ describe('Mobile Replay Integration', () => { const integration = mobileReplayIntegration({ beforeErrorSampling }); integration.setup?.(mockClient); + // Capture the afterSendEvent handler before the mid-test mock clear wipes + // the registration record. + const afterSendEventCall = mockOn.mock.calls.find(c => c[0] === 'afterSendEvent'); + const afterSendEvent = afterSendEventCall![1] as (event: Event) => void; + // Test with handled error const handledEvent = { event_id: 'handled-event-id', @@ -311,6 +346,10 @@ describe('Mobile Replay Integration', () => { expect(result1).toBeDefined(); expect(beforeErrorSampling).toHaveBeenCalledWith(handledEvent, hint); + + // The handled error was filtered out, so it is never linked or flushed. + afterSendEvent(result1 as Event); + await flushAsync(); expect(mockCaptureReplay).not.toHaveBeenCalled(); jest.clearAllMocks(); @@ -333,6 +372,9 @@ describe('Mobile Replay Integration', () => { expect(result2).toBeDefined(); expect(beforeErrorSampling).toHaveBeenCalledWith(unhandledEvent, hint); + + afterSendEvent(result2 as Event); + await flushAsync(); expect(mockCaptureReplay).toHaveBeenCalled(); }); @@ -373,7 +415,9 @@ describe('Mobile Replay Integration', () => { expect(result).toBeDefined(); expect(beforeErrorSampling).toHaveBeenCalledWith(event, hint); + // Should proceed with replay capture despite callback error + await fireAfterSendEvent(result as Event); expect(mockCaptureReplay).toHaveBeenCalled(); }); @@ -393,9 +437,12 @@ describe('Mobile Replay Integration', () => { const hint: EventHint = {}; // Should not throw - await expect(clientOptions.beforeSend?.(event, hint)).resolves.toBeDefined(); + const result = await clientOptions.beforeSend?.(event, hint); + expect(result).toBeDefined(); expect(beforeErrorSampling).toHaveBeenCalled(); + + await fireAfterSendEvent(result as Event); expect(mockCaptureReplay).toHaveBeenCalled(); }); @@ -425,9 +472,11 @@ describe('Mobile Replay Integration', () => { expect(result).toBeDefined(); expect(userBeforeSend).toHaveBeenCalledWith(event, hint); expect(beforeErrorSampling).toHaveBeenCalled(); - expect(mockCaptureReplay).toHaveBeenCalled(); expect(result?.tags).toEqual({ modified: 'true' }); expect(result?.contexts?.replay?.replay_id).toBe('test-replay-id'); + + await fireAfterSendEvent(result as Event); + expect(mockCaptureReplay).toHaveBeenCalled(); }); it('should not capture replay when user beforeSend drops event even if beforeErrorSampling returns true', async () => { @@ -456,8 +505,8 @@ describe('Mobile Replay Integration', () => { }); }); - describe('captureReplay returns null (native capture failed)', () => { - it('should not set replay_id when captureReplay returns null and no ongoing recording', async () => { + describe('native replay flush on send', () => { + it('does not link or flush when there is no active recording', async () => { mockCaptureReplay.mockResolvedValue(null); mockGetCurrentReplayId.mockReturnValue(null); @@ -480,14 +529,18 @@ describe('Mobile Replay Integration', () => { const result = await clientOptions.beforeSend?.(event, hint); + // No buffered recording => nothing to link... expect(result).toBeDefined(); - expect(mockCaptureReplay).toHaveBeenCalledWith(true); // isHardCrash expect(result?.contexts?.replay?.replay_id).toBeUndefined(); + + // ...and nothing to flush, even after the event is sent. + await fireAfterSendEvent(result as Event); + expect(mockCaptureReplay).not.toHaveBeenCalled(); }); - it('should use ongoing recording when captureReplay returns null but recording exists', async () => { + it('links the event to the ongoing recording and flushes it as a hard crash on send', async () => { mockCaptureReplay.mockResolvedValue(null); - // First call during setup returns initial ID, second call during processEvent returns ongoing ID + // First call during setup returns no ID, the call inside beforeSend returns the ongoing ID. mockGetCurrentReplayId.mockReturnValueOnce(null).mockReturnValue('ongoing-replay-id'); const integration = mobileReplayIntegration(); @@ -509,15 +562,18 @@ describe('Mobile Replay Integration', () => { const result = await clientOptions.beforeSend?.(event, hint); + // The event is linked to the ongoing recording in beforeSend. expect(result).toBeDefined(); - expect(mockCaptureReplay).toHaveBeenCalled(); - // Should fall back to ongoing recording ID expect(result?.contexts?.replay?.replay_id).toBe('ongoing-replay-id'); + + // The flush happens on send, propagating the hard-crash flag. + await fireAfterSendEvent(result as Event); + expect(mockCaptureReplay).toHaveBeenCalledWith(true); // isHardCrash }); - it('should set replay_id when captureReplay succeeds', async () => { + it('updates the cached replay id from the flushed replay after send', async () => { mockCaptureReplay.mockResolvedValue('new-replay-id'); - mockGetCurrentReplayId.mockReturnValue(null); + mockGetCurrentReplayId.mockReturnValue('buffered-replay-id'); const integration = mobileReplayIntegration(); integration.setup?.(mockClient); @@ -538,9 +594,72 @@ describe('Mobile Replay Integration', () => { const result = await clientOptions.beforeSend?.(event, hint); + // The event is linked to the buffered id at beforeSend time (stable through flush). expect(result).toBeDefined(); + expect(result?.contexts?.replay?.replay_id).toBe('buffered-replay-id'); + + // Once flushed on send, the cache reflects the id returned by the native flush. + await fireAfterSendEvent(result as Event); expect(mockCaptureReplay).toHaveBeenCalled(); - expect(result?.contexts?.replay?.replay_id).toBe('new-replay-id'); + expect(integration.getReplayId()).toBe('new-replay-id'); + }); + + it('re-reads the current recording id when the flush uploads nothing (on-error sampling miss)', async () => { + // A buffered replay is linked in beforeSend, but the native flush uploads + // nothing (an on-error sampling miss resolves null). The cache must not be + // left exposing the linked id as if it had been uploaded. + mockGetCurrentReplayId.mockReturnValue('buffered-replay-id'); + mockCaptureReplay.mockResolvedValue(null); + + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + const event = { + event_id: 'test-event-id', + exception: { + values: [{ type: 'Error', value: 'Test error', mechanism: { handled: false, type: 'onerror' } }], + }, + } as ErrorEvent; + + const result = await clientOptions.beforeSend?.(event, {}); + expect(result?.contexts?.replay?.replay_id).toBe('buffered-replay-id'); + + // After the miss, the still-active recording reports a fresh id. + mockGetCurrentReplayId.mockReturnValue('still-recording-id'); + await fireAfterSendEvent(result as Event); + + // The cache was refreshed from the current recording, not left stale. + expect(integration.getReplayId()).toBe('still-recording-id'); + }); + + it('flushes a sent event regardless of how many other events were linked first', async () => { + // Regression for the eviction race: the flush decision must live on the + // event itself, so a linked event still flushes after many other events + // (e.g. errors later dropped by sampling that never reach afterSendEvent) + // were linked in beforeSend. + mockGetCurrentReplayId.mockReturnValue('buffered-replay-id'); + + const integration = mobileReplayIntegration(); + integration.setup?.(mockClient); + + const makeEvent = (id: string) => + ({ + event_id: id, + exception: { values: [{ type: 'Error', value: 'Test error' }] }, + }) as ErrorEvent; + + // Link the first event but do not send it yet. + const first = await clientOptions.beforeSend?.(makeEvent('first-event-id'), {}); + expect(first?.contexts?.replay?.replay_id).toBe('buffered-replay-id'); + + // Link many more events afterwards without sending them. + for (let i = 0; i < 200; i++) { + await clientOptions.beforeSend?.(makeEvent(`event-${i}`), {}); + } + + // The first event is finally sent: it must still flush its replay. + await fireAfterSendEvent(first as Event); + expect(mockCaptureReplay).toHaveBeenCalledTimes(1); }); }); @@ -851,9 +970,13 @@ describe('Mobile Replay Integration', () => { } as ErrorEvent; const hint: EventHint = {}; - await clientOptions.beforeSend?.(event, hint); + const result = await clientOptions.beforeSend?.(event, hint); + + // Before the flush, the cache holds the buffered id linked in beforeSend. + expect(integration.getReplayId()).toBe(initialReplayId); - // Verify cache was updated by checking getReplayId + // The flush on send returns the final id and updates the cache. + await fireAfterSendEvent(result as Event); expect(integration.getReplayId()).toBe(newReplayId); // Extract the createDsc handler BEFORE clearing mocks