-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(replay): Add replayStart/replayEnd client lifecycle hooks #20369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -25,3 +25,37 @@ export type ReplayRecordingData = string | Uint8Array; | |||||
| * @hidden | ||||||
| */ | ||||||
| export type ReplayRecordingMode = 'session' | 'buffer'; | ||||||
|
|
||||||
| /** | ||||||
| * Reason a replay recording stopped, passed to the `replayEnd` client hook. | ||||||
| * | ||||||
| * - `manual`: user called `replay.stop()`. | ||||||
| * - `sessionExpired`: session hit `maxReplayDuration` or the idle-expiry threshold. | ||||||
| * - `sendError`: a replay segment failed to send after retries. | ||||||
| * - `mutationLimit`: DOM mutation budget for the session was exhausted. | ||||||
| * - `eventBufferError`: the event buffer threw an unexpected error. | ||||||
| * - `eventBufferOverflow`: the event buffer ran out of space. | ||||||
| */ | ||||||
| export type ReplayStopReason = | ||||||
| | 'manual' | ||||||
| | 'sessionExpired' | ||||||
| | 'sendError' | ||||||
| | 'mutationLimit' | ||||||
| | 'eventBufferError' | ||||||
| | 'eventBufferOverflow'; | ||||||
|
|
||||||
| /** | ||||||
| * Payload emitted on the `replayStart` client hook when a replay begins recording. | ||||||
| */ | ||||||
| export interface ReplayStartEvent { | ||||||
| sessionId: string; | ||||||
| recordingMode: ReplayRecordingMode; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Payload emitted on the `replayEnd` client hook when a replay stops recording. | ||||||
| */ | ||||||
| export interface ReplayEndEvent { | ||||||
| sessionId?: string; | ||||||
|
||||||
| sessionId?: string; | |
| sessionId: string; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,5 @@ | ||||||||||||||||||||||||||||||||||
| /* eslint-disable max-lines */ // TODO: We might want to split this file up | ||||||||||||||||||||||||||||||||||
| import type { ReplayRecordingMode, Span } from '@sentry/core'; | ||||||||||||||||||||||||||||||||||
| import type { ReplayRecordingMode, ReplayStopReason, Span } from '@sentry/core'; | ||||||||||||||||||||||||||||||||||
| import { getActiveSpan, getClient, getRootSpan, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, spanToJSON } from '@sentry/core'; | ||||||||||||||||||||||||||||||||||
| import { EventType, record } from '@sentry-internal/rrweb'; | ||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||
|
|
@@ -495,7 +495,10 @@ export class ReplayContainer implements ReplayContainerInterface { | |||||||||||||||||||||||||||||||||
| * Currently, this needs to be manually called (e.g. for tests). Sentry SDK | ||||||||||||||||||||||||||||||||||
| * does not support a teardown | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| public async stop({ forceFlush = false, reason }: { forceFlush?: boolean; reason?: string } = {}): Promise<void> { | ||||||||||||||||||||||||||||||||||
| public async stop({ | ||||||||||||||||||||||||||||||||||
| forceFlush = false, | ||||||||||||||||||||||||||||||||||
| reason, | ||||||||||||||||||||||||||||||||||
| }: { forceFlush?: boolean; reason?: ReplayStopReason } = {}): Promise<void> { | ||||||||||||||||||||||||||||||||||
| if (!this._isEnabled) { | ||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
@@ -508,8 +511,11 @@ export class ReplayContainer implements ReplayContainerInterface { | |||||||||||||||||||||||||||||||||
| // breadcrumbs to trigger a flush (e.g. in `addUpdate()`) | ||||||||||||||||||||||||||||||||||
| this.recordingMode = 'buffer'; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const stopReason: ReplayStopReason = reason ?? 'manual'; | ||||||||||||||||||||||||||||||||||
| getClient()?.emit('replayEnd', { sessionId: this.session?.id, reason: stopReason }); | ||||||||||||||||||||||||||||||||||
|
logaretm marked this conversation as resolved.
Dismissed
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| DEBUG_BUILD && debug.log(`Stopping Replay${reason ? ` triggered by ${reason}` : ''}`); | ||||||||||||||||||||||||||||||||||
| DEBUG_BUILD && debug.log(`Stopping Replay triggered by ${stopReason}`); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| resetReplayIdOnDynamicSamplingContext(); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
@@ -862,6 +868,13 @@ export class ReplayContainer implements ReplayContainerInterface { | |||||||||||||||||||||||||||||||||
| this._isEnabled = true; | ||||||||||||||||||||||||||||||||||
| this._isPaused = false; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (this.session) { | ||||||||||||||||||||||||||||||||||
| getClient()?.emit('replayStart', { | ||||||||||||||||||||||||||||||||||
| sessionId: this.session.id, | ||||||||||||||||||||||||||||||||||
|
logaretm marked this conversation as resolved.
Dismissed
|
||||||||||||||||||||||||||||||||||
| recordingMode: this.recordingMode, | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| this.startRecording(); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+871
to
878
|
||||||||||||||||||||||||||||||||||
| if (this.session) { | |
| getClient()?.emit('replayStart', { | |
| sessionId: this.session.id, | |
| recordingMode: this.recordingMode, | |
| }); | |
| } | |
| this.startRecording(); | |
| this.startRecording(); | |
| if (this.session && this._stopRecording) { | |
| getClient()?.emit('replayStart', { | |
| sessionId: this.session.id, | |
| recordingMode: this.recordingMode, | |
| }); | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong stop reason emitted for ReplayDurationLimitError
Medium Severity
The catch block in _sendReplay calls this.stop({ reason: 'sendError' }) for all errors, including ReplayDurationLimitError. That error is thrown when the session exceeds maxReplayDuration (line 1199–1200), which the ReplayStopReason documentation explicitly maps to 'sessionExpired' ("session hit maxReplayDuration or the idle-expiry threshold"). Consumers listening on replayEnd will receive reason: 'sendError' when the actual cause is duration expiry, making it impossible to distinguish network failures from session timeouts — which is the stated purpose of the typed reason union.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 47e2074. Configure here.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| /** | ||
| * @vitest-environment jsdom | ||
| */ | ||
|
|
||
| import '../utils/mock-internal-setTimeout'; | ||
| import type { ReplayEndEvent, ReplayStartEvent } from '@sentry/core'; | ||
| import { getClient } from '@sentry/core'; | ||
| import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import type { Replay } from '../../src/integration'; | ||
| import type { ReplayContainer } from '../../src/replay'; | ||
| import { BASE_TIMESTAMP } from '../index'; | ||
| import { resetSdkMock } from '../mocks/resetSdkMock'; | ||
|
|
||
| describe('Integration | lifecycle hooks', () => { | ||
| let replay: ReplayContainer; | ||
| let integration: Replay; | ||
| let startEvents: ReplayStartEvent[]; | ||
| let endEvents: ReplayEndEvent[]; | ||
| let unsubscribes: Array<() => void>; | ||
|
|
||
| beforeAll(() => { | ||
| vi.useFakeTimers(); | ||
| }); | ||
|
|
||
| beforeEach(async () => { | ||
| ({ replay, integration } = await resetSdkMock({ | ||
| replayOptions: { stickySession: false }, | ||
| sentryOptions: { replaysSessionSampleRate: 0.0 }, | ||
| autoStart: false, | ||
| })); | ||
|
|
||
| startEvents = []; | ||
| endEvents = []; | ||
| const client = getClient()!; | ||
| unsubscribes = [ | ||
| client.on('replayStart', event => startEvents.push(event)), | ||
| client.on('replayEnd', event => endEvents.push(event)), | ||
| ]; | ||
|
|
||
| await vi.runAllTimersAsync(); | ||
| }); | ||
|
|
||
| afterEach(async () => { | ||
| unsubscribes.forEach(off => off()); | ||
| await integration.stop(); | ||
| await vi.runAllTimersAsync(); | ||
| vi.setSystemTime(new Date(BASE_TIMESTAMP)); | ||
| }); | ||
|
|
||
| it('fires replayStart with session mode when start() is called', () => { | ||
| integration.start(); | ||
|
|
||
| expect(startEvents).toHaveLength(1); | ||
| expect(startEvents[0]).toEqual({ | ||
| sessionId: expect.any(String), | ||
| recordingMode: 'session', | ||
| }); | ||
| expect(startEvents[0]!.sessionId).toBe(replay.session!.id); | ||
| }); | ||
|
|
||
| it('fires replayStart with buffer mode when startBuffering() is called', () => { | ||
| integration.startBuffering(); | ||
|
|
||
| expect(startEvents).toHaveLength(1); | ||
| expect(startEvents[0]).toEqual({ | ||
| sessionId: expect.any(String), | ||
| recordingMode: 'buffer', | ||
| }); | ||
| }); | ||
|
|
||
| it('fires replayEnd with reason "manual" when integration.stop() is called', async () => { | ||
| integration.start(); | ||
| const sessionId = replay.session!.id; | ||
|
|
||
| await integration.stop(); | ||
|
|
||
| expect(endEvents).toHaveLength(1); | ||
| expect(endEvents[0]).toEqual({ sessionId, reason: 'manual' }); | ||
| }); | ||
|
|
||
| it('forwards the internal stop reason to replayEnd subscribers', async () => { | ||
| integration.start(); | ||
| const sessionId = replay.session!.id; | ||
|
|
||
| await replay.stop({ reason: 'mutationLimit' }); | ||
|
|
||
| expect(endEvents).toHaveLength(1); | ||
| expect(endEvents[0]).toEqual({ sessionId, reason: 'mutationLimit' }); | ||
| }); | ||
|
|
||
| it('does not fire replayEnd twice when stop() is called while already stopped', async () => { | ||
| integration.start(); | ||
|
|
||
| await replay.stop({ reason: 'sendError' }); | ||
| await replay.stop({ reason: 'sendError' }); | ||
|
|
||
| expect(endEvents).toHaveLength(1); | ||
| expect(endEvents[0]!.reason).toBe('sendError'); | ||
| }); | ||
|
|
||
| it('stops invoking callbacks after the returned unsubscribe is called', () => { | ||
| const [offStart] = unsubscribes; | ||
| offStart!(); | ||
|
|
||
| integration.start(); | ||
|
|
||
| expect(startEvents).toHaveLength(0); | ||
| }); | ||
| }); |


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring for
sendErrorsays it only covers failures after retries, but the replay code also stops withreason: 'sendError'for non-retryable conditions like rate limiting andReplayDurationLimitError(duration too long). Either broaden thesendErrordescription to match actual behavior, or introduce more granular stop reasons (e.g. rate-limited / invalid / durationLimit) so consumers can distinguish why replay ended.