diff --git a/packages/browser-integration-tests/suites/replay/errors/droppedError/test.ts b/packages/browser-integration-tests/suites/replay/errors/droppedError/test.ts index 5bcb75bfe619..b63ca9a8b61f 100644 --- a/packages/browser-integration-tests/suites/replay/errors/droppedError/test.ts +++ b/packages/browser-integration-tests/suites/replay/errors/droppedError/test.ts @@ -1,37 +1,25 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../../utils/fixtures'; -import { getExpectedReplayEvent } from '../../../../utils/replayEventTemplates'; -import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; +import { envelopeRequestParser } from '../../../../utils/helpers'; +import { getReplaySnapshot, isReplayEvent, shouldSkipReplayTest } from '../../../../utils/replayHelpers'; -/* - * This scenario currently shows somewhat unexpected behavior from the PoV of a user: - * The error is dropped, but the recording is started and continued anyway. - * If folks only sample error replays, this will lead to a lot of confusion as the resulting replay - * won't contain the error that started it (possibly none or only additional errors that occurred later on). - * - * This is because in error-mode, we start recording as soon as replay's eventProcessor is called with an error. - * If later event processors or beforeSend drop the error, the recording is already started. - * - * We'll need a proper SDK lifecycle hook (WIP) to fix this properly. - * TODO: Once we have lifecycle hooks, we should revisit this test and make sure it behaves as expected. - * This means that the recording should not be started or stopped if the error that triggered it is not sent. - */ sentryTest( - '[error-mode] should start recording if an error occurred although the error was dropped', - async ({ getLocalTestPath, page }) => { + '[error-mode] should not start recording if an error occurred when the error was dropped', + async ({ getLocalTestPath, page, forceFlushReplay }) => { if (shouldSkipReplayTest()) { sentryTest.skip(); } - const reqPromise0 = waitForReplayRequest(page, 0); - const reqPromise1 = waitForReplayRequest(page, 1); - const reqPromise2 = waitForReplayRequest(page, 2); - let callsToSentry = 0; await page.route('https://dsn.ingest.sentry.io/**/*', route => { - callsToSentry++; + const req = route.request(); + const event = envelopeRequestParser(req); + + if (isReplayEvent(event)) { + callsToSentry++; + } return route.fulfill({ status: 200, @@ -43,30 +31,17 @@ sentryTest( const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - await page.click('#go-background'); + await forceFlushReplay(); expect(callsToSentry).toEqual(0); await page.click('#error'); - const req0 = await reqPromise0; - - await page.click('#go-background'); - await reqPromise1; await page.click('#log'); - await page.click('#go-background'); - await reqPromise2; + await forceFlushReplay(); - // Note: The fact that reqPromise1/reqPromise2 are fulfilled prooves that the recording continues - - const event0 = getReplayEvent(req0); + expect(callsToSentry).toEqual(0); - expect(event0).toEqual( - getExpectedReplayEvent({ - contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } }, - // This is by design. A dropped error shouldn't be in this list. - error_ids: [], - replay_type: 'error', - }), - ); + const replay = await getReplaySnapshot(page); + expect(replay.recordingMode).toBe('error'); }, ); diff --git a/packages/browser-integration-tests/suites/replay/errors/errorModeCustomTransport/init.js b/packages/browser-integration-tests/suites/replay/errors/errorModeCustomTransport/init.js new file mode 100644 index 000000000000..dca771f16c87 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/errors/errorModeCustomTransport/init.js @@ -0,0 +1,22 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window.Replay = new Sentry.Replay({ + flushMinDelay: 500, + flushMaxDelay: 500, +}); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 1, + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 1.0, + integrations: [window.Replay], + transport: options => { + const transport = new Sentry.makeXHRTransport(options); + + delete transport.send.__sentry__baseTransport__; + + return transport; + }, +}); diff --git a/packages/browser-integration-tests/suites/replay/errors/errorModeCustomTransport/test.ts b/packages/browser-integration-tests/suites/replay/errors/errorModeCustomTransport/test.ts new file mode 100644 index 000000000000..8efe303afb88 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/errors/errorModeCustomTransport/test.ts @@ -0,0 +1,42 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getReplaySnapshot, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; + +sentryTest( + '[error-mode] should handle errors with custom transport', + async ({ getLocalTestPath, page, forceFlushReplay }) => { + if (shouldSkipReplayTest()) { + sentryTest.skip(); + } + + const promiseReq0 = waitForReplayRequest(page, 0); + const promiseReq1 = waitForReplayRequest(page, 1); + + let callsToSentry = 0; + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + callsToSentry++; + + return route.fulfill({ + // Only error out for error, then succeed + status: callsToSentry === 1 ? 422 : 200, + }); + }); + + const url = await getLocalTestPath({ testDir: __dirname }); + + await page.goto(url); + await forceFlushReplay(); + expect(callsToSentry).toEqual(0); + + await page.click('#error'); + await promiseReq0; + + await forceFlushReplay(); + await promiseReq1; + + const replay = await getReplaySnapshot(page); + expect(replay.recordingMode).toBe('session'); + }, +); diff --git a/packages/browser-integration-tests/suites/replay/errors/errorNotSent/init.js b/packages/browser-integration-tests/suites/replay/errors/errorNotSent/init.js new file mode 100644 index 000000000000..4f8dcac1ea28 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/errors/errorNotSent/init.js @@ -0,0 +1,15 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window.Replay = new Sentry.Replay({ + flushMinDelay: 500, + flushMaxDelay: 500, +}); + +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/errorNotSent/test.ts b/packages/browser-integration-tests/suites/replay/errors/errorNotSent/test.ts new file mode 100644 index 000000000000..963f47e9919d --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/errors/errorNotSent/test.ts @@ -0,0 +1,41 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getReplaySnapshot, shouldSkipReplayTest } from '../../../../utils/replayHelpers'; + +sentryTest( + '[error-mode] should handle errors that result in API error response', + async ({ getLocalTestPath, page, forceFlushReplay }) => { + if (shouldSkipReplayTest()) { + sentryTest.skip(); + } + + let callsToSentry = 0; + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + callsToSentry++; + + return route.fulfill({ + status: 422, + contentType: 'application/json', + }); + }); + + const url = await getLocalTestPath({ testDir: __dirname }); + + await page.goto(url); + await forceFlushReplay(); + expect(callsToSentry).toEqual(0); + + await page.click('#error'); + + await page.click('#log'); + await forceFlushReplay(); + + // Only sent once, but since API failed we do not go into session mode + expect(callsToSentry).toEqual(1); + + const replay = await getReplaySnapshot(page); + expect(replay.recordingMode).toBe('error'); + }, +); diff --git a/packages/browser-integration-tests/utils/replayHelpers.ts b/packages/browser-integration-tests/utils/replayHelpers.ts index 8722fe245a23..cf21ce7b9c7b 100644 --- a/packages/browser-integration-tests/utils/replayHelpers.ts +++ b/packages/browser-integration-tests/utils/replayHelpers.ts @@ -7,7 +7,7 @@ import type { Session, } from '@sentry/replay/build/npm/types/types'; import type { eventWithTime } from '@sentry/replay/build/npm/types/types/rrweb'; -import type { Breadcrumb, Event, ReplayEvent } from '@sentry/types'; +import type { Breadcrumb, Event, ReplayEvent, ReplayRecordingMode } from '@sentry/types'; import pako from 'pako'; import type { Page, Request, Response } from 'playwright'; @@ -104,9 +104,13 @@ function isCustomSnapshot(event: RecordingEvent): event is RecordingEvent & { da * Note that due to how this works with playwright, this is a POJO copy of replay. * This means that we cannot access any methods on it, and also not mutate it in any way. */ -export async function getReplaySnapshot( - page: Page, -): Promise<{ _isPaused: boolean; _isEnabled: boolean; _context: InternalEventContext; session: Session | undefined }> { +export async function getReplaySnapshot(page: Page): Promise<{ + _isPaused: boolean; + _isEnabled: boolean; + _context: InternalEventContext; + session: Session | undefined; + recordingMode: ReplayRecordingMode; +}> { return await page.evaluate(() => { const replayIntegration = (window as unknown as Window & { Replay: { _replay: ReplayContainer } }).Replay; const replay = replayIntegration._replay; @@ -116,6 +120,7 @@ export async function getReplaySnapshot( _isEnabled: replay.isEnabled(), _context: replay.getContext(), session: replay.session, + recordingMode: replay.recordingMode, }; return replaySnapshot; diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index e603cba661dd..8cafc77a68c3 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -146,7 +146,7 @@ export class BrowserClient extends BaseClient { } else { // If beacon is not supported or if they are using the tunnel option // use our regular transport to send client reports to Sentry. - this._sendEnvelope(envelope); + void this._sendEnvelope(envelope); } } catch (e) { __DEBUG_BUILD__ && logger.error(e); diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index bf1ae616c4d6..80ed36450ef5 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -20,6 +20,7 @@ import type { Transaction, TransactionEvent, Transport, + TransportMakeRequestResponse, } from '@sentry/types'; import { addItemToEnvelope, @@ -320,7 +321,10 @@ export abstract class BaseClient implements Client { ); } - this._sendEnvelope(env); + const promise = this._sendEnvelope(env); + if (promise) { + promise.then(sendResponse => this.emit('afterSendEvent', event, sendResponse), null); + } } } @@ -330,7 +334,7 @@ export abstract class BaseClient implements Client { public sendSession(session: Session | SessionAggregates): void { if (this._dsn) { const env = createSessionEnvelope(session, this._dsn, this._options._metadata, this._options.tunnel); - this._sendEnvelope(env); + void this._sendEnvelope(env); } } @@ -363,6 +367,12 @@ export abstract class BaseClient implements Client { /** @inheritdoc */ public on(hook: 'beforeEnvelope', callback: (envelope: Envelope) => void): void; + /** @inheritdoc */ + public on( + hook: 'afterSendEvent', + callback: (event: Event, sendResponse: TransportMakeRequestResponse | void) => void, + ): void; + /** @inheritdoc */ public on(hook: string, callback: unknown): void { if (!this._hooks[hook]) { @@ -379,6 +389,9 @@ export abstract class BaseClient implements Client { /** @inheritdoc */ public emit(hook: 'beforeEnvelope', envelope: Envelope): void; + /** @inheritdoc */ + public emit(hook: 'afterSendEvent', event: Event, sendResponse: TransportMakeRequestResponse | void): void; + /** @inheritdoc */ public emit(hook: string, ...rest: unknown[]): void { if (this._hooks[hook]) { @@ -624,9 +637,9 @@ export abstract class BaseClient implements Client { /** * @inheritdoc */ - protected _sendEnvelope(envelope: Envelope): void { + protected _sendEnvelope(envelope: Envelope): PromiseLike | void { if (this._transport && this._dsn) { - this._transport.send(envelope).then(null, reason => { + return this._transport.send(envelope).then(null, reason => { __DEBUG_BUILD__ && logger.error('Error while sending event:', reason); }); } else { diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index 19a92ebc8599..5b11db7e9c17 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -109,6 +109,10 @@ export function createTransport( ); } + // We use this to identifify if the transport is the base transport + // TODO (v8): Remove this again as we'll no longer need it + send.__sentry__baseTransport__ = true; + return { send, flush, diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 4ee65114fe79..9705f7a6622f 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -1634,6 +1634,136 @@ describe('BaseClient', () => { }); }); + describe('sendEvent', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('emits `afterSendEvent` when sending an error', async () => { + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + enableSend: true, + }), + ); + + // @ts-ignore + const mockSend = jest.spyOn(client._transport, 'send'); + + const errorEvent: Event = { message: 'error' }; + + const callback = jest.fn(); + client.on('afterSendEvent', callback); + + client.sendEvent(errorEvent); + jest.runAllTimers(); + // Wait for two ticks + // note that for whatever reason, await new Promise(resolve => setTimeout(resolve, 0)) causes the test to hang + await undefined; + await undefined; + + expect(mockSend).toBeCalledTimes(1); + expect(callback).toBeCalledTimes(1); + expect(callback).toBeCalledWith(errorEvent, {}); + }); + + it('emits `afterSendEvent` when sending a transaction', async () => { + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + enableSend: true, + }), + ); + + // @ts-ignore + const mockSend = jest.spyOn(client._transport, 'send'); + + const transactionEvent: Event = { type: 'transaction', event_id: 'tr1' }; + + const callback = jest.fn(); + client.on('afterSendEvent', callback); + + client.sendEvent(transactionEvent); + jest.runAllTimers(); + // Wait for two ticks + // note that for whatever reason, await new Promise(resolve => setTimeout(resolve, 0)) causes the test to hang + await undefined; + await undefined; + + expect(mockSend).toBeCalledTimes(1); + expect(callback).toBeCalledTimes(1); + expect(callback).toBeCalledWith(transactionEvent, {}); + }); + + it('still triggers `afterSendEvent` when transport.send rejects', async () => { + expect.assertions(3); + + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + enableSend: true, + }), + ); + + // @ts-ignore + const mockSend = jest.spyOn(client._transport, 'send').mockImplementation(() => { + return Promise.reject('send error'); + }); + + const errorEvent: Event = { message: 'error' }; + + const callback = jest.fn(); + client.on('afterSendEvent', callback); + + client.sendEvent(errorEvent); + jest.runAllTimers(); + // Wait for two ticks + // note that for whatever reason, await new Promise(resolve => setTimeout(resolve, 0)) causes the test to hang + await undefined; + await undefined; + + expect(mockSend).toBeCalledTimes(1); + expect(callback).toBeCalledTimes(1); + expect(callback).toBeCalledWith(errorEvent, undefined); + }); + + it('passes the response to the hook', async () => { + expect.assertions(3); + + const client = new TestClient( + getDefaultTestClientOptions({ + dsn: PUBLIC_DSN, + enableSend: true, + }), + ); + + // @ts-ignore + const mockSend = jest.spyOn(client._transport, 'send').mockImplementation(() => { + return Promise.resolve({ statusCode: 200 }); + }); + + const errorEvent: Event = { message: 'error' }; + + const callback = jest.fn(); + client.on('afterSendEvent', callback); + + client.sendEvent(errorEvent); + jest.runAllTimers(); + // Wait for two ticks + // note that for whatever reason, await new Promise(resolve => setTimeout(resolve, 0)) causes the test to hang + await undefined; + await undefined; + + expect(mockSend).toBeCalledTimes(1); + expect(callback).toBeCalledTimes(1); + expect(callback).toBeCalledWith(errorEvent, { statusCode: 200 }); + }); + }); + describe('captureSession()', () => { test('sends sessions to the client', () => { expect.assertions(1); diff --git a/packages/replay/src/coreHandlers/handleAfterSendEvent.ts b/packages/replay/src/coreHandlers/handleAfterSendEvent.ts new file mode 100644 index 000000000000..f3a531f3ffdc --- /dev/null +++ b/packages/replay/src/coreHandlers/handleAfterSendEvent.ts @@ -0,0 +1,88 @@ +import { getCurrentHub } from '@sentry/core'; +import type { Event, Transport, TransportMakeRequestResponse } from '@sentry/types'; + +import { UNABLE_TO_SEND_REPLAY } from '../constants'; +import type { ReplayContainer } from '../types'; +import { isErrorEvent, isTransactionEvent } from '../util/eventUtils'; + +type AfterSendEventCallback = (event: Event, sendResponse: TransportMakeRequestResponse | void) => void; + +/** + * Returns a listener to be added to `client.on('afterSendErrorEvent, listener)`. + */ +export function handleAfterSendEvent(replay: ReplayContainer): AfterSendEventCallback { + // Custom transports may still be returning `Promise`, which means we cannot expect the status code to be available there + // TODO (v8): remove this check as it will no longer be necessary + const enforceStatusCode = isBaseTransportSend(); + + return (event: Event, sendResponse: TransportMakeRequestResponse | void) => { + if (!isErrorEvent(event) && !isTransactionEvent(event)) { + return; + } + + const statusCode = sendResponse && sendResponse.statusCode; + + // We only want to do stuff on successful error sending, otherwise you get error replays without errors attached + // If not using the base transport, we allow `undefined` response (as a custom transport may not implement this correctly yet) + // If we do use the base transport, we skip if we encountered an non-OK status code + if (enforceStatusCode && (!statusCode || statusCode < 200 || statusCode >= 300)) { + return; + } + + // Collect traceIds in _context regardless of `recordingMode` + // In error mode, _context gets cleared on every checkout + if (isTransactionEvent(event) && event.contexts && event.contexts.trace && event.contexts.trace.trace_id) { + replay.getContext().traceIds.add(event.contexts.trace.trace_id as string); + return; + } + + // Everything below is just for error events + if (!isErrorEvent(event)) { + return; + } + + // Add error to list of errorIds of replay + if (event.event_id) { + replay.getContext().errorIds.add(event.event_id); + } + + // Trigger error recording + // Need to be very careful that this does not cause an infinite loop + if ( + replay.recordingMode === 'error' && + event.exception && + event.message !== UNABLE_TO_SEND_REPLAY // ignore this error because otherwise we could loop indefinitely with trying to capture replay and failing + ) { + setTimeout(async () => { + // Allow flush to complete before resuming as a session recording, otherwise + // the checkout from `startRecording` may be included in the payload. + // Prefer to keep the error replay as a separate (and smaller) segment + // than the session replay. + await replay.flushImmediate(); + + if (replay.stopRecording()) { + // Reset all "capture on error" configuration before + // starting a new recording + replay.recordingMode = 'session'; + replay.startRecording(); + } + }); + } + }; +} + +function isBaseTransportSend(): boolean { + const client = getCurrentHub().getClient(); + if (!client) { + return false; + } + + const transport = client.getTransport(); + if (!transport) { + return false; + } + + return ( + (transport.send as Transport['send'] & { __sentry__baseTransport__?: true }).__sentry__baseTransport__ || false + ); +} diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts index 35ecba8c43e3..ba539b661387 100644 --- a/packages/replay/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay/src/coreHandlers/handleGlobalEvent.ts @@ -2,23 +2,33 @@ import { addBreadcrumb } from '@sentry/core'; import type { Event, EventHint } from '@sentry/types'; import { logger } from '@sentry/utils'; -import { REPLAY_EVENT_NAME, UNABLE_TO_SEND_REPLAY } from '../constants'; import type { ReplayContainer } from '../types'; +import { isErrorEvent, isReplayEvent, isTransactionEvent } from '../util/eventUtils'; import { isRrwebError } from '../util/isRrwebError'; +import { handleAfterSendEvent } from './handleAfterSendEvent'; /** * Returns a listener to be added to `addGlobalEventProcessor(listener)`. */ -export function handleGlobalEventListener(replay: ReplayContainer): (event: Event, hint: EventHint) => Event | null { +export function handleGlobalEventListener( + replay: ReplayContainer, + includeAfterSendEventHandling = false, +): (event: Event, hint: EventHint) => Event | null { + const afterSendHandler = includeAfterSendEventHandling ? handleAfterSendEvent(replay) : undefined; + return (event: Event, hint: EventHint) => { - // Do not apply replayId to the root event - if (event.type === REPLAY_EVENT_NAME) { + if (isReplayEvent(event)) { // Replays have separate set of breadcrumbs, do not include breadcrumbs // from core SDK delete event.breadcrumbs; return event; } + // We only want to handle errors & transactions, nothing else + if (!isErrorEvent(event) && !isTransactionEvent(event)) { + return event; + } + // Unless `captureExceptions` is enabled, we want to ignore errors coming from rrweb // As there can be a bunch of stuff going wrong in internals there, that we don't want to bubble up to users if (isRrwebError(event, hint) && !replay.getOptions()._experiments.captureExceptions) { @@ -27,50 +37,22 @@ export function handleGlobalEventListener(replay: ReplayContainer): (event: Even } // Only tag transactions with replayId if not waiting for an error - // @ts-ignore private - if (!event.type || replay.recordingMode === 'session') { + if (isErrorEvent(event) || (isTransactionEvent(event) && replay.recordingMode === 'session')) { event.tags = { ...event.tags, replayId: replay.getSessionId() }; } - // Collect traceIds in _context regardless of `recordingMode` - if it's true, - // _context gets cleared on every checkout - if (event.type === 'transaction' && event.contexts && event.contexts.trace && event.contexts.trace.trace_id) { - replay.getContext().traceIds.add(event.contexts.trace.trace_id as string); - return event; - } - - // no event type means error - if (!event.type) { - replay.getContext().errorIds.add(event.event_id as string); - } - - if (__DEBUG_BUILD__ && replay.getOptions()._experiments.traceInternals) { + if (__DEBUG_BUILD__ && replay.getOptions()._experiments.traceInternals && isErrorEvent(event)) { const exc = getEventExceptionValues(event); addInternalBreadcrumb({ message: `Tagging event (${event.event_id}) - ${event.message} - ${exc.type}: ${exc.value}`, }); } - // Need to be very careful that this does not cause an infinite loop - if ( - replay.recordingMode === 'error' && - event.exception && - event.message !== UNABLE_TO_SEND_REPLAY // ignore this error because otherwise we could loop indefinitely with trying to capture replay and failing - ) { - setTimeout(async () => { - // Allow flush to complete before resuming as a session recording, otherwise - // the checkout from `startRecording` may be included in the payload. - // Prefer to keep the error replay as a separate (and smaller) segment - // than the session replay. - await replay.flushImmediate(); - - if (replay.stopRecording()) { - // Reset all "capture on error" configuration before - // starting a new recording - replay.recordingMode = 'session'; - replay.startRecording(); - } - }); + // In cases where a custom client is used that does not support the new hooks (yet), + // we manually call this hook method here + if (afterSendHandler) { + // Pretend the error had a 200 response so we always capture it + afterSendHandler(event, { statusCode: 200 }); } return event; diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 7f397855d17f..c42a5cc5b9b6 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -32,7 +32,6 @@ import { debounce } from './util/debounce'; import { getHandleRecordingEmit } from './util/handleRecordingEmit'; import { isExpired } from './util/isExpired'; import { isSessionExpired } from './util/isSessionExpired'; -import { overwriteRecordDroppedEvent, restoreRecordDroppedEvent } from './util/monkeyPatchRecordDroppedEvent'; import { sendReplay } from './util/sendReplay'; /** @@ -499,9 +498,6 @@ export class ReplayContainer implements ReplayContainerInterface { WINDOW.addEventListener('blur', this._handleWindowBlur); WINDOW.addEventListener('focus', this._handleWindowFocus); - // We need to filter out dropped events captured by `addGlobalEventProcessor(this.handleGlobalEvent)` below - overwriteRecordDroppedEvent(this._context.errorIds); - // There is no way to remove these listeners, so ensure they are only added once if (!this._hasInitializedCoreListeners) { addGlobalListeners(this); @@ -530,8 +526,6 @@ export class ReplayContainer implements ReplayContainerInterface { WINDOW.removeEventListener('blur', this._handleWindowBlur); WINDOW.removeEventListener('focus', this._handleWindowFocus); - restoreRecordDroppedEvent(); - if (this._performanceObserver) { this._performanceObserver.disconnect(); this._performanceObserver = null; diff --git a/packages/replay/src/util/addGlobalListeners.ts b/packages/replay/src/util/addGlobalListeners.ts index dfef03b094d4..94146962911f 100644 --- a/packages/replay/src/util/addGlobalListeners.ts +++ b/packages/replay/src/util/addGlobalListeners.ts @@ -1,6 +1,8 @@ +import type { BaseClient } from '@sentry/core'; import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core'; import { addInstrumentationHandler } from '@sentry/utils'; +import { handleAfterSendEvent } from '../coreHandlers/handleAfterSendEvent'; import { handleDomListener } from '../coreHandlers/handleDom'; import { handleFetchSpanListener } from '../coreHandlers/handleFetch'; import { handleGlobalEventListener } from '../coreHandlers/handleGlobalEvent'; @@ -15,6 +17,8 @@ import type { ReplayContainer } from '../types'; export function addGlobalListeners(replay: ReplayContainer): void { // Listeners from core SDK // const scope = getCurrentHub().getScope(); + const client = getCurrentHub().getClient(); + if (scope) { scope.addScopeListener(handleScopeListener(replay)); } @@ -23,7 +27,15 @@ export function addGlobalListeners(replay: ReplayContainer): void { addInstrumentationHandler('xhr', handleXhrSpanListener(replay)); addInstrumentationHandler('history', handleHistorySpanListener(replay)); + // If a custom client has no hooks yet, we continue to use the "old" implementation + const hasHooks = !!(client && client.on); + // Tag all (non replay) events that get sent to Sentry with the current // replay ID so that we can reference them later in the UI - addGlobalEventProcessor(handleGlobalEventListener(replay)); + addGlobalEventProcessor(handleGlobalEventListener(replay, !hasHooks)); + + if (hasHooks) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (client as BaseClient).on('afterSendEvent', handleAfterSendEvent(replay)); + } } diff --git a/packages/replay/src/util/eventUtils.ts b/packages/replay/src/util/eventUtils.ts new file mode 100644 index 000000000000..5cd4989a8267 --- /dev/null +++ b/packages/replay/src/util/eventUtils.ts @@ -0,0 +1,16 @@ +import type { ErrorEvent, Event, ReplayEvent, TransactionEvent } from '@sentry/types'; + +/** If the event is an error event */ +export function isErrorEvent(event: Event): event is ErrorEvent { + return !event.type; +} + +/** If the event is a transaction event */ +export function isTransactionEvent(event: Event): event is TransactionEvent { + return event.type === 'transaction'; +} + +/** If the event is an replay event */ +export function isReplayEvent(event: Event): event is ReplayEvent { + return event.type === 'replay_event'; +} diff --git a/packages/replay/src/util/monkeyPatchRecordDroppedEvent.ts b/packages/replay/src/util/monkeyPatchRecordDroppedEvent.ts deleted file mode 100644 index 6a819ef917b2..000000000000 --- a/packages/replay/src/util/monkeyPatchRecordDroppedEvent.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { getCurrentHub } from '@sentry/core'; -import type { Client, DataCategory, Event, EventDropReason } from '@sentry/types'; - -let _originalRecordDroppedEvent: Client['recordDroppedEvent'] | undefined; - -/** - * Overwrite the `recordDroppedEvent` method on the client, so we can find out which events were dropped. - * */ -export function overwriteRecordDroppedEvent(errorIds: Set): void { - const client = getCurrentHub().getClient(); - - if (!client) { - return; - } - - const _originalCallback = client.recordDroppedEvent.bind(client); - - const recordDroppedEvent: Client['recordDroppedEvent'] = ( - reason: EventDropReason, - category: DataCategory, - event?: Event, - ): void => { - if (event && !event.type && event.event_id) { - errorIds.delete(event.event_id); - } - - return _originalCallback(reason, category, event); - }; - - client.recordDroppedEvent = recordDroppedEvent; - _originalRecordDroppedEvent = _originalCallback; -} - -/** - * Restore the original method. - * */ -export function restoreRecordDroppedEvent(): void { - const client = getCurrentHub().getClient(); - - if (!client || !_originalRecordDroppedEvent) { - return; - } - - client.recordDroppedEvent = _originalRecordDroppedEvent; -} diff --git a/packages/replay/test/fixtures/transaction.ts b/packages/replay/test/fixtures/transaction.ts index 2899740c5e52..1e8ec7a3272a 100644 --- a/packages/replay/test/fixtures/transaction.ts +++ b/packages/replay/test/fixtures/transaction.ts @@ -1,6 +1,6 @@ import type { Event, SeverityLevel } from '@sentry/types'; -export function Transaction(obj?: Partial): any { +export function Transaction(traceId?: string, obj?: Partial): any { const timestamp = new Date().getTime() / 1000; return { @@ -22,7 +22,7 @@ export function Transaction(obj?: Partial): any { hardwareConcurrency: '10', sentry_reportAllChanges: false, }, - trace_id: 'trace_id', + trace_id: traceId || 'trace_id', }, }, // }}} spans: [ diff --git a/packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts b/packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts new file mode 100644 index 000000000000..d46bacf4aede --- /dev/null +++ b/packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts @@ -0,0 +1,331 @@ +import { getCurrentHub } from '@sentry/core'; +import type { ErrorEvent, Event } from '@sentry/types'; + +import { UNABLE_TO_SEND_REPLAY } from '../../../src/constants'; +import { handleAfterSendEvent } from '../../../src/coreHandlers/handleAfterSendEvent'; +import type { ReplayContainer } from '../../../src/replay'; +import { Error } from '../../fixtures/error'; +import { Transaction } from '../../fixtures/transaction'; +import { resetSdkMock } from '../../mocks/resetSdkMock'; +import { useFakeTimers } from '../../utils/use-fake-timers'; + +useFakeTimers(); +let replay: ReplayContainer; + +describe('Integration | coreHandlers | handleAfterSendEvent', () => { + afterEach(() => { + replay.stop(); + }); + + it('records errorIds from sent error events', async () => { + ({ replay } = await resetSdkMock({ + replayOptions: { + stickySession: false, + }, + sentryOptions: { + replaysSessionSampleRate: 1.0, + replaysOnErrorSampleRate: 0.0, + }, + })); + + const error1 = Error({ event_id: 'err1' }); + const error2 = Error({ event_id: 'err2' }); + const error3 = Error({ event_id: 'err3' }); + const error4 = Error({ event_id: 'err4' }); + + const handler = handleAfterSendEvent(replay); + + // With undefined response: Don't capture + handler(error1, undefined); + // With "successful" response: Capture + handler(error2, { statusCode: 200 }); + // With "unsuccessful" response: Don't capture + handler(error3, { statusCode: 0 }); + // With no statusCode response: Don't Capture + handler(error4, { statusCode: undefined }); + + expect(Array.from(replay.getContext().errorIds)).toEqual(['err2']); + expect(Array.from(replay.getContext().traceIds)).toEqual([]); + }); + + it('records traceIds from sent transaction events', async () => { + ({ replay } = await resetSdkMock({ + replayOptions: { + stickySession: false, + }, + sentryOptions: { + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 1.0, + }, + })); + + const transaction1 = Transaction('tr1'); + const transaction2 = Transaction('tr2'); + const transaction3 = Transaction('tr3'); + const transaction4 = Transaction('tr4'); + + const handler = handleAfterSendEvent(replay); + + // With undefined response: Don't capture + handler(transaction1, undefined); + // With "successful" response: Capture + handler(transaction2, { statusCode: 200 }); + // With "unsuccessful" response: Don't capture + handler(transaction3, { statusCode: 0 }); + // With no statusCode response: Don't Capture + handler(transaction4, { statusCode: undefined }); + + expect(Array.from(replay.getContext().errorIds)).toEqual([]); + expect(Array.from(replay.getContext().traceIds)).toEqual(['tr2']); + + // Does not affect error session + jest.runAllTimers(); + await new Promise(process.nextTick); + + expect(Array.from(replay.getContext().errorIds)).toEqual([]); + expect(Array.from(replay.getContext().traceIds)).toEqual(['tr2']); + expect(replay.isEnabled()).toBe(true); + expect(replay.isPaused()).toBe(false); + expect(replay.recordingMode).toBe('error'); + }); + + it('allows undefined send response when using custom transport', async () => { + ({ replay } = await resetSdkMock({ + replayOptions: { + stickySession: false, + }, + sentryOptions: { + replaysSessionSampleRate: 1.0, + replaysOnErrorSampleRate: 0.0, + }, + })); + + const client = getCurrentHub().getClient()!; + // @ts-ignore make sure to remove this + delete client.getTransport()!.send.__sentry__baseTransport__; + + const error1 = Error({ event_id: 'err1' }); + const error2 = Error({ event_id: 'err2' }); + const error3 = Error({ event_id: 'err3' }); + const error4 = Error({ event_id: 'err4' }); + + const handler = handleAfterSendEvent(replay); + + // With undefined response: Capture + handler(error1, undefined); + // With "successful" response: Capture + handler(error2, { statusCode: 200 }); + // With "unsuccessful" response: Capture + handler(error3, { statusCode: 0 }); + // With no statusCode response: Capture + handler(error4, { statusCode: undefined }); + + expect(Array.from(replay.getContext().errorIds)).toEqual(['err1', 'err2', 'err3', 'err4']); + }); + + it('flushes when in error mode', async () => { + ({ replay } = await resetSdkMock({ + replayOptions: { + stickySession: false, + }, + sentryOptions: { + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 1.0, + }, + })); + + const mockSend = getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance; + + const error1 = Error({ event_id: 'err1' }); + + const handler = handleAfterSendEvent(replay); + + expect(replay.recordingMode).toBe('error'); + + handler(error1, { statusCode: 200 }); + + expect(Array.from(replay.getContext().errorIds)).toEqual(['err1']); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + // Send twice, one for the error & one right after for the session conversion + expect(mockSend).toHaveBeenCalledTimes(2); + // This is removed now, because it has been converted to a "session" session + expect(Array.from(replay.getContext().errorIds)).toEqual([]); + expect(replay.isEnabled()).toBe(true); + expect(replay.isPaused()).toBe(false); + expect(replay.recordingMode).toBe('session'); + }); + + it('does not flush when in session mode', async () => { + ({ replay } = await resetSdkMock({ + replayOptions: { + stickySession: false, + }, + sentryOptions: { + replaysSessionSampleRate: 1.0, + replaysOnErrorSampleRate: 0.0, + }, + })); + + const mockSend = getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance; + + const error1 = Error({ event_id: 'err1' }); + + const handler = handleAfterSendEvent(replay); + + expect(replay.recordingMode).toBe('session'); + + handler(error1, { statusCode: 200 }); + + expect(Array.from(replay.getContext().errorIds)).toEqual(['err1']); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + // Send once for the regular session sending + expect(mockSend).toHaveBeenCalledTimes(1); + expect(Array.from(replay.getContext().errorIds)).toEqual(['err1']); + expect(replay.isEnabled()).toBe(true); + expect(replay.isPaused()).toBe(false); + expect(replay.recordingMode).toBe('session'); + }); + + it('ignores profile & replay events', async () => { + ({ replay } = await resetSdkMock({ + replayOptions: { + stickySession: false, + }, + sentryOptions: { + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 1.0, + }, + })); + + const mockSend = getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance; + + const profileEvent: Event = { type: 'profile' }; + const replayEvent: Event = { type: 'replay_event' }; + + const handler = handleAfterSendEvent(replay); + + expect(replay.recordingMode).toBe('error'); + + handler(profileEvent, { statusCode: 200 }); + handler(replayEvent, { statusCode: 200 }); + + expect(Array.from(replay.getContext().errorIds)).toEqual([]); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + expect(mockSend).toHaveBeenCalledTimes(0); + expect(Array.from(replay.getContext().errorIds)).toEqual([]); + expect(replay.isEnabled()).toBe(true); + expect(replay.isPaused()).toBe(false); + expect(replay.recordingMode).toBe('error'); + }); + + it('does not flush in error mode when failing to send the error', async () => { + ({ replay } = await resetSdkMock({ + replayOptions: { + stickySession: false, + }, + sentryOptions: { + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 1.0, + }, + })); + + const mockSend = getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance; + + const error1 = Error({ event_id: 'err1' }); + + const handler = handleAfterSendEvent(replay); + + expect(replay.recordingMode).toBe('error'); + + handler(error1, undefined); + + expect(Array.from(replay.getContext().errorIds)).toEqual([]); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + // Remains in error mode & without flushing + expect(mockSend).toHaveBeenCalledTimes(0); + expect(Array.from(replay.getContext().errorIds)).toEqual([]); + expect(replay.isEnabled()).toBe(true); + expect(replay.isPaused()).toBe(false); + expect(replay.recordingMode).toBe('error'); + }); + + it('does not flush if error event has no exception', async () => { + ({ replay } = await resetSdkMock({ + replayOptions: { + stickySession: false, + }, + sentryOptions: { + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 1.0, + }, + })); + + const mockSend = getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance; + + const error1: ErrorEvent = { event_id: 'err1', type: undefined }; + + const handler = handleAfterSendEvent(replay); + + expect(replay.recordingMode).toBe('error'); + + handler(error1, { statusCode: 200 }); + + expect(Array.from(replay.getContext().errorIds)).toEqual(['err1']); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + // Remains in error mode & without flushing + expect(mockSend).toHaveBeenCalledTimes(0); + expect(Array.from(replay.getContext().errorIds)).toEqual(['err1']); + expect(replay.isEnabled()).toBe(true); + expect(replay.isPaused()).toBe(false); + expect(replay.recordingMode).toBe('error'); + }); + + it('does not flush if error is replay send error', async () => { + ({ replay } = await resetSdkMock({ + replayOptions: { + stickySession: false, + }, + sentryOptions: { + replaysSessionSampleRate: 0.0, + replaysOnErrorSampleRate: 1.0, + }, + })); + + const mockSend = getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance; + + const error1 = Error({ event_id: 'err1', message: UNABLE_TO_SEND_REPLAY }); + + const handler = handleAfterSendEvent(replay); + + expect(replay.recordingMode).toBe('error'); + + handler(error1, { statusCode: 200 }); + + expect(Array.from(replay.getContext().errorIds)).toEqual(['err1']); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + // Remains in error mode & without flushing + expect(mockSend).toHaveBeenCalledTimes(0); + expect(Array.from(replay.getContext().errorIds)).toEqual(['err1']); + expect(replay.isEnabled()).toBe(true); + expect(replay.isPaused()).toBe(false); + expect(replay.recordingMode).toBe('error'); + }); +}); diff --git a/packages/replay/test/integration/coreHandlers/handleGlobalEvent.test.ts b/packages/replay/test/integration/coreHandlers/handleGlobalEvent.test.ts index 1bce8c11572a..24e709707033 100644 --- a/packages/replay/test/integration/coreHandlers/handleGlobalEvent.test.ts +++ b/packages/replay/test/integration/coreHandlers/handleGlobalEvent.test.ts @@ -1,13 +1,8 @@ -import { getCurrentHub } from '@sentry/core'; import type { Event } from '@sentry/types'; import { REPLAY_EVENT_NAME } from '../../../src/constants'; import { handleGlobalEventListener } from '../../../src/coreHandlers/handleGlobalEvent'; import type { ReplayContainer } from '../../../src/replay'; -import { - overwriteRecordDroppedEvent, - restoreRecordDroppedEvent, -} from '../../../src/util/monkeyPatchRecordDroppedEvent'; import { Error } from '../../fixtures/error'; import { Transaction } from '../../fixtures/transaction'; import { resetSdkMock } from '../../mocks/resetSdkMock'; @@ -73,11 +68,10 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => { ); }); - it('only tags errors with replay id, adds trace and error id to context for error samples', async () => { + it('does not add replayId for transactions in error mode', async () => { const transaction = Transaction(); const error = Error(); - // @ts-ignore idc - expect(handleGlobalEventListener(replay)(transaction)).toEqual( + expect(handleGlobalEventListener(replay)(transaction, {})).toEqual( expect.objectContaining({ tags: expect.not.objectContaining({ replayId: expect.anything() }), }), @@ -87,37 +81,6 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => { tags: expect.objectContaining({ replayId: expect.any(String) }), }), ); - - expect(replay.getContext().traceIds).toContain('trace_id'); - expect(replay.getContext().errorIds).toContain('event_id'); - - jest.runAllTimers(); - await new Promise(process.nextTick); // wait for flush - - // Rerverts `recordingMode` to session - expect(replay.recordingMode).toBe('session'); - }); - - it('strips out dropped events from errorIds', async () => { - const error1 = Error({ event_id: 'err1' }); - const error2 = Error({ event_id: 'err2' }); - const error3 = Error({ event_id: 'err3' }); - - // @ts-ignore private - overwriteRecordDroppedEvent(replay.getContext().errorIds); - - const client = getCurrentHub().getClient()!; - - handleGlobalEventListener(replay)(error1, {}); - handleGlobalEventListener(replay)(error2, {}); - handleGlobalEventListener(replay)(error3, {}); - - client.recordDroppedEvent('before_send', 'error', { event_id: 'err2' }); - - // @ts-ignore private - expect(Array.from(replay.getContext().errorIds)).toEqual(['err1', 'err3']); - - restoreRecordDroppedEvent(); }); it('tags errors and transactions with replay id for session samples', async () => { @@ -125,8 +88,7 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => { replay.start(); const transaction = Transaction(); const error = Error(); - // @ts-ignore idc - expect(handleGlobalEventListener(replay)(transaction)).toEqual( + expect(handleGlobalEventListener(replay)(transaction, {})).toEqual( expect.objectContaining({ tags: expect.objectContaining({ replayId: expect.any(String) }), }), @@ -138,6 +100,88 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => { ); }); + it('does not collect errorIds when hooks are available', async () => { + const error1 = Error({ event_id: 'err1' }); + const error2 = Error({ event_id: 'err2' }); + const error3 = Error({ event_id: 'err3' }); + + const handler = handleGlobalEventListener(replay); + + handler(error1, {}); + handler(error2, {}); + handler(error3, {}); + + expect(Array.from(replay.getContext().errorIds)).toEqual([]); + }); + + it('collects errorIds when hooks are not available', async () => { + const error1 = Error({ event_id: 'err1' }); + const error2 = Error({ event_id: 'err2' }); + const error3 = Error({ event_id: 'err3' }); + + const handler = handleGlobalEventListener(replay, true); + + handler(error1, {}); + handler(error2, {}); + handler(error3, {}); + + expect(Array.from(replay.getContext().errorIds)).toEqual(['err1', 'err2', 'err3']); + }); + + it('does not collect traceIds when hooks are available', async () => { + const transaction1 = Transaction('tr1'); + const transaction2 = Transaction('tr2'); + const transaction3 = Transaction('tr3'); + + const handler = handleGlobalEventListener(replay); + + handler(transaction1, {}); + handler(transaction2, {}); + handler(transaction3, {}); + + expect(Array.from(replay.getContext().traceIds)).toEqual([]); + }); + + it('collects traceIds when hooks are not available', async () => { + const transaction1 = Transaction('tr1'); + const transaction2 = Transaction('tr2'); + const transaction3 = Transaction('tr3'); + + const handler = handleGlobalEventListener(replay, true); + + handler(transaction1, {}); + handler(transaction2, {}); + handler(transaction3, {}); + + expect(Array.from(replay.getContext().traceIds)).toEqual(['tr1', 'tr2', 'tr3']); + }); + + it('ignores profile & replay events', async () => { + const profileEvent: Event = { type: 'profile' }; + const replayEvent: Event = { type: 'replay_event' }; + + const handler = handleGlobalEventListener(replay); + const handler2 = handleGlobalEventListener(replay, true); + + expect(replay.recordingMode).toBe('error'); + + handler(profileEvent, {}); + handler(replayEvent, {}); + handler2(profileEvent, {}); + handler2(replayEvent, {}); + + expect(Array.from(replay.getContext().traceIds)).toEqual([]); + expect(Array.from(replay.getContext().errorIds)).toEqual([]); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + expect(Array.from(replay.getContext().errorIds)).toEqual([]); + expect(replay.isEnabled()).toBe(true); + expect(replay.isPaused()).toBe(false); + expect(replay.recordingMode).toBe('error'); + }); + it('does not skip non-rrweb errors', () => { const errorEvent: Event = { exception: { diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts index d4bfa7279a95..85f154e523f4 100644 --- a/packages/replay/test/integration/errorSampleRate.test.ts +++ b/packages/replay/test/integration/errorSampleRate.test.ts @@ -63,6 +63,8 @@ describe('Integration | errorSampleRate', () => { expect(replay).not.toHaveLastSentReplay(); captureException(new Error('testing')); + + await new Promise(process.nextTick); jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); await new Promise(process.nextTick); @@ -109,7 +111,9 @@ describe('Integration | errorSampleRate', () => { }, }, }), - recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 5020, type: 2 }]), + recordingData: JSON.stringify([ + { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + 40, type: 2 }, + ]), }); jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); @@ -120,7 +124,7 @@ describe('Integration | errorSampleRate', () => { recordingData: JSON.stringify([ { data: { isCheckout: true }, - timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + 20, + timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + 40, type: 2, }, ]), @@ -138,11 +142,11 @@ describe('Integration | errorSampleRate', () => { recordingData: JSON.stringify([ { type: 5, - timestamp: BASE_TIMESTAMP + 10000 + 40, + timestamp: BASE_TIMESTAMP + 10000 + 60, data: { tag: 'breadcrumb', payload: { - timestamp: (BASE_TIMESTAMP + 10000 + 40) / 1000, + timestamp: (BASE_TIMESTAMP + 10000 + 60) / 1000, type: 'default', category: 'ui.click', message: '', @@ -347,6 +351,7 @@ describe('Integration | errorSampleRate', () => { captureException(new Error('testing')); + await new Promise(process.nextTick); jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); await new Promise(process.nextTick); @@ -358,7 +363,7 @@ describe('Integration | errorSampleRate', () => { // (advance timers + waiting for flush after the checkout) and // extra time is likely due to async of `addMemoryEntry()` - timestamp: (BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + DEFAULT_FLUSH_MIN_DELAY + 20) / 1000, + timestamp: (BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + DEFAULT_FLUSH_MIN_DELAY + 40) / 1000, error_ids: [expect.any(String)], trace_ids: [], urls: ['http://localhost/'], @@ -394,6 +399,7 @@ describe('Integration | errorSampleRate', () => { captureException(new Error('testing')); + await new Promise(process.nextTick); jest.runAllTimers(); jest.advanceTimersByTime(20); await new Promise(process.nextTick); @@ -431,6 +437,7 @@ describe('Integration | errorSampleRate', () => { captureException(new Error('testing')); + await new Promise(process.nextTick); jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); await new Promise(process.nextTick); @@ -440,6 +447,8 @@ describe('Integration | errorSampleRate', () => { mockRecord.takeFullSnapshot.mockClear(); (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance).mockClear(); + expect(replay).not.toHaveLastSentReplay(); + // Go idle jest.advanceTimersByTime(SESSION_IDLE_DURATION + 1); await new Promise(process.nextTick); @@ -534,6 +543,7 @@ it('sends a replay after loading the session multiple times', async () => { captureException(new Error('testing')); + await new Promise(process.nextTick); jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); await new Promise(process.nextTick); @@ -544,6 +554,6 @@ it('sends a replay after loading the session multiple times', async () => { // Latest checkout when we call `startRecording` again after uploading segment // after an error occurs (e.g. when we switch to session replay recording) expect(replay).toHaveLastSentReplay({ - recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 5020, type: 2 }]), + recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 5040, type: 2 }]), }); }); diff --git a/packages/replay/test/mocks/mockSdk.ts b/packages/replay/test/mocks/mockSdk.ts index 9da56061071f..af23a47cc5bc 100644 --- a/packages/replay/test/mocks/mockSdk.ts +++ b/packages/replay/test/mocks/mockSdk.ts @@ -1,4 +1,4 @@ -import type { Envelope, Transport } from '@sentry/types'; +import type { Envelope, Transport, TransportMakeRequestResponse } from '@sentry/types'; import type { Replay as ReplayIntegration } from '../../src'; import type { ReplayContainer } from '../../src/replay'; @@ -13,9 +13,21 @@ export interface MockSdkParams { } class MockTransport implements Transport { - send: (request: Envelope) => PromiseLike = jest.fn(async () => { - return; - }); + send: (request: Envelope) => PromiseLike; + + constructor() { + const send: ((request: Envelope) => PromiseLike) & { + __sentry__baseTransport__?: boolean; + } = jest.fn(async () => { + return { + statusCode: 200, + }; + }); + + send.__sentry__baseTransport__ = true; + this.send = send; + } + async flush() { return true; } diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts index 71e2ac5a96d6..0ddc6a9615f2 100644 --- a/packages/types/src/client.ts +++ b/packages/types/src/client.ts @@ -10,7 +10,7 @@ import type { SdkMetadata } from './sdkmetadata'; import type { Session, SessionAggregates } from './session'; import type { Severity, SeverityLevel } from './severity'; import type { Transaction } from './transaction'; -import type { Transport } from './transport'; +import type { Transport, TransportMakeRequestResponse } from './transport'; /** * User-Facing Sentry SDK Client. @@ -163,6 +163,14 @@ export interface Client { */ on?(hook: 'beforeEnvelope', callback: (envelope: Envelope) => void): void; + /** + * Register a callback for when an event has been sent. + */ + on?( + hook: 'afterSendEvent', + callback: (event: Event, sendResponse: TransportMakeRequestResponse | void) => void, + ): void; + /** * Fire a hook event for transaction start and finish. Expects to be given a transaction as the * second argument. @@ -174,4 +182,10 @@ export interface Client { * second argument. */ emit?(hook: 'beforeEnvelope', envelope: Envelope): void; + + /* + * Fire a hook event after sending an event. Expects to be given an Event as the + * second argument. + */ + emit?(hook: 'afterSendEvent', event: Event, sendResponse: TransportMakeRequestResponse | void): void; }