From 3ebe54c0a472ebe0182b5fbba65d72b1b4193168 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 7 Nov 2023 13:56:08 +0100 Subject: [PATCH] feat(replay): Allow to configure `beforeErrorSampling` --- .../replay/errors/beforeErrorSampling/init.js | 18 +++++++ .../replay/errors/beforeErrorSampling/test.ts | 38 ++++++++++++++ .../src/coreHandlers/handleAfterSendEvent.ts | 17 +++++-- packages/replay/src/integration.ts | 2 + packages/replay/src/types/replay.ts | 11 +++++ .../coreHandlers/handleAfterSendEvent.test.ts | 49 +++++++++++++++++++ 6 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 packages/browser-integration-tests/suites/replay/errors/beforeErrorSampling/init.js create mode 100644 packages/browser-integration-tests/suites/replay/errors/beforeErrorSampling/test.ts diff --git a/packages/browser-integration-tests/suites/replay/errors/beforeErrorSampling/init.js b/packages/browser-integration-tests/suites/replay/errors/beforeErrorSampling/init.js new file mode 100644 index 000000000000..a4d39ad78e7e --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/errors/beforeErrorSampling/init.js @@ -0,0 +1,18 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window.Replay = new Sentry.Replay({ + flushMinDelay: 200, + flushMaxDelay: 200, + minReplayDuration: 0, + beforeErrorSampling: event => !event.message.includes('[drop]'), +}); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 1, + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 1.0, + + integrations: [window.Replay], +}); diff --git a/packages/browser-integration-tests/suites/replay/errors/beforeErrorSampling/test.ts b/packages/browser-integration-tests/suites/replay/errors/beforeErrorSampling/test.ts new file mode 100644 index 000000000000..1f42c14e80ad --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/errors/beforeErrorSampling/test.ts @@ -0,0 +1,38 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getReplaySnapshot, shouldSkipReplayTest, waitForReplayRunning } from '../../../../utils/replayHelpers'; + +sentryTest( + '[error-mode] should not flush if error event is ignored in beforeErrorSampling', + async ({ getLocalTestPath, page, browserName, forceFlushReplay }) => { + // Skipping this in webkit because it is flakey there + if (shouldSkipReplayTest() || browserName === 'webkit') { + sentryTest.skip(); + } + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestPath({ testDir: __dirname }); + + await page.goto(url); + await waitForReplayRunning(page); + + await page.click('#drop'); + await forceFlushReplay(); + + expect(await getReplaySnapshot(page)).toEqual( + expect.objectContaining({ + _isEnabled: true, + _isPaused: false, + recordingMode: 'buffer', + }), + ); + }, +); diff --git a/packages/replay/src/coreHandlers/handleAfterSendEvent.ts b/packages/replay/src/coreHandlers/handleAfterSendEvent.ts index 8697498c2c7a..00e3b72d590d 100644 --- a/packages/replay/src/coreHandlers/handleAfterSendEvent.ts +++ b/packages/replay/src/coreHandlers/handleAfterSendEvent.ts @@ -63,12 +63,19 @@ function handleErrorEvent(replay: ReplayContainer, event: ErrorEvent): void { // If error event is tagged with replay id it means it was sampled (when in buffer mode) // Need to be very careful that this does not cause an infinite loop - if (replay.recordingMode === 'buffer' && event.tags && event.tags.replayId) { - setTimeout(() => { - // Capture current event buffer as new replay - void replay.sendBufferedReplayOrFlush(); - }); + if (replay.recordingMode !== 'buffer' || !event.tags || !event.tags.replayId) { + return; } + + const { beforeErrorSampling } = replay.getOptions(); + if (typeof beforeErrorSampling === 'function' && !beforeErrorSampling(event)) { + return; + } + + setTimeout(() => { + // Capture current event buffer as new replay + void replay.sendBufferedReplayOrFlush(); + }); } function isBaseTransportSend(): boolean { diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index bfa236d87dec..559b80532c77 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -90,6 +90,7 @@ export class Replay implements Integration { maskFn, beforeAddRecordingEvent, + beforeErrorSampling, // eslint-disable-next-line deprecation/deprecation blockClass, @@ -178,6 +179,7 @@ export class Replay implements Integration { networkRequestHeaders: _getMergedNetworkHeaders(networkRequestHeaders), networkResponseHeaders: _getMergedNetworkHeaders(networkResponseHeaders), beforeAddRecordingEvent, + beforeErrorSampling, _experiments, }; diff --git a/packages/replay/src/types/replay.ts b/packages/replay/src/types/replay.ts index f8ec28e2f799..38ca35ac8f0e 100644 --- a/packages/replay/src/types/replay.ts +++ b/packages/replay/src/types/replay.ts @@ -1,5 +1,6 @@ import type { Breadcrumb, + ErrorEvent, FetchBreadcrumbHint, HandlerDataFetch, ReplayRecordingData, @@ -211,6 +212,16 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { */ beforeAddRecordingEvent?: BeforeAddRecordingEvent; + /** + * An optional callback to be called before we decide to sample based on an error. + * If specified, this callback will receive an error that was captured by Sentry. + * Return `true` to continue sampling for this error, or `false` to ignore this error for replay sampling. + * Note that returning `true` means that the `replaysOnErrorSampleRate` will be checked, + * not that it will definitely be sampled. + * Use this to filter out groups of errors that should def. not be sampled. + */ + beforeErrorSampling?: (event: ErrorEvent) => boolean; + /** * _experiments allows users to enable experimental or internal features. * We don't consider such features as part of the public API and hence we don't guarantee semver for them. diff --git a/packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts b/packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts index a5d32ae50ca3..cfb03073d12e 100644 --- a/packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts +++ b/packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts @@ -411,4 +411,53 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => { expect(mockSend).toHaveBeenCalledTimes(0); }); + + it('calls beforeErrorSampling if defined', async () => { + const error1 = Error({ event_id: 'err1', tags: { replayId: 'replayid1' } }); + const error2 = Error({ event_id: 'err2', tags: { replayId: 'replayid1' } }); + + const beforeErrorSampling = jest.fn(event => event === error2); + + ({ replay } = await resetSdkMock({ + replayOptions: { + stickySession: false, + beforeErrorSampling, + }, + sentryOptions: { + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 1.0, + }, + })); + + const mockSend = getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance; + + const handler = handleAfterSendEvent(replay); + + expect(replay.recordingMode).toBe('buffer'); + + handler(error1, { statusCode: 200 }); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + expect(beforeErrorSampling).toHaveBeenCalledTimes(1); + + // Not flushed yet + expect(mockSend).toHaveBeenCalledTimes(0); + expect(replay.recordingMode).toBe('buffer'); + expect(Array.from(replay.getContext().errorIds)).toEqual(['err1']); + + handler(error2, { statusCode: 200 }); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + expect(beforeErrorSampling).toHaveBeenCalledTimes(2); + + // Triggers session + expect(mockSend).toHaveBeenCalledTimes(1); + expect(replay.recordingMode).toBe('session'); + expect(replay.isEnabled()).toBe(true); + expect(replay.isPaused()).toBe(false); + }); });