Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/detect-rrweb-init-failure.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"posthog-js": patch
---

Detect and report when rrweb fails to initialize. rrweb's `record()` silently swallows startup errors and returns `undefined`, which previously left the SDK reporting an active recording status while capturing zero data. The SDK now checks the return value and reports a new `rrweb_error` status, making the failure visible in debug properties.
Original file line number Diff line number Diff line change
Expand Up @@ -3071,6 +3071,80 @@ describe('Lazy SessionRecording', () => {
})
})

describe('when rrweb record() returns undefined', () => {
it('does not report recording as started', () => {
loadScriptMock.mockImplementation((_ph: any, _path: any, callback: any) => {
assignableWindow.__PosthogExtensions__.rrweb = {
record: jest.fn(() => undefined),
version: 'fake',
wasMaxDepthReached: jest.fn(() => false),
resetMaxDepthState: jest.fn(),
}
assignableWindow.__PosthogExtensions__.rrweb.record.takeFullSnapshot = jest.fn()
assignableWindow.__PosthogExtensions__.rrweb.record.addCustomEvent = jest.fn()
assignableWindow.__PosthogExtensions__.initSessionRecording = () => {
return new LazyLoadedSessionRecording(posthog)
}
callback()
})

sessionRecording.onRemoteConfig(
makeFlagsResponse({
sessionRecording: {
endpoint: '/s/',
},
})
)

expect(sessionRecording.started).toEqual(false)
expect(sessionRecording.status).toEqual('rrweb_error')
})

it('recovers when rrweb starts successfully on retry', () => {
let recordCallCount = 0
loadScriptMock.mockImplementation((_ph: any, _path: any, callback: any) => {
assignableWindow.__PosthogExtensions__.rrweb = {
record: jest.fn(({ emit }) => {
recordCallCount++
if (recordCallCount === 1) {
return undefined
}
_emit = emit
return () => {}
}),
version: 'fake',
wasMaxDepthReached: jest.fn(() => false),
resetMaxDepthState: jest.fn(),
}
assignableWindow.__PosthogExtensions__.rrweb.record.takeFullSnapshot = jest.fn(() => {
_emit(createFullSnapshot())
})
assignableWindow.__PosthogExtensions__.rrweb.record.addCustomEvent = jest.fn()
assignableWindow.__PosthogExtensions__.initSessionRecording = () => {
return new LazyLoadedSessionRecording(posthog)
}
callback()
})

sessionRecording.onRemoteConfig(
makeFlagsResponse({
sessionRecording: {
endpoint: '/s/',
},
})
)

expect(sessionRecording.started).toEqual(false)
expect(sessionRecording.status).toEqual('rrweb_error')

// simulate session rotation triggering a restart
sessionRecording['_lazyLoadedSessionRecording']!.start()

expect(sessionRecording.started).toEqual(true)
expect(sessionRecording.status).not.toEqual('rrweb_error')
})
})

describe('buffering minimum duration', () => {
it('can report no duration when no data', () => {
sessionRecording.onRemoteConfig(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
LinkedFlagMatching,
PAUSED,
RecordingTriggersStatus,
RRWEB_ERROR,
SAMPLED,
SessionRecordingStatus,
TRIGGER_ACTIVATED,
Expand All @@ -30,6 +31,7 @@ const defaultTriggersStatus: RecordingTriggersStatus = {
receivedFlags: true,
isRecordingEnabled: true,
isSampled: undefined,
rrwebError: false,
urlTriggerMatching: {
onRemoteConfig: () => {},
_instance: fakePostHog,
Expand Down Expand Up @@ -58,6 +60,31 @@ const makeLinkedFlagMatcher = (linkedFlag: string | null, linkedFlagSeen: boolea

const testCases: TestConfig[] = [
// Basic states
{
name: 'rrweb error',
config: { rrwebError: true },
anyMatchExpected: RRWEB_ERROR,
allMatchExpected: RRWEB_ERROR,
},
{
name: 'rrweb error overrides sampling true',
config: { rrwebError: true, isSampled: true },
anyMatchExpected: RRWEB_ERROR,
allMatchExpected: RRWEB_ERROR,
},
{
name: 'rrweb error overrides active trigger',
config: {
rrwebError: true,
urlTriggerMatching: {
...defaultTriggersStatus.urlTriggerMatching,
_instance: fakePostHog,
triggerStatus: () => TRIGGER_ACTIVATED,
} as unknown as URLTriggerMatching,
},
anyMatchExpected: RRWEB_ERROR,
allMatchExpected: RRWEB_ERROR,
},
{
name: 'flags not received',
config: { receivedFlags: false },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ export class LazyLoadedSessionRecording implements LazyLoadedSessionRecordingInt
*/
private _queuedRRWebEvents: QueuedRRWebEvent[] = []
private _isIdle: boolean | 'unknown' = 'unknown'
private _rrwebError = false
private _maxDepthExceeded = false

private _linkedFlagMatching: LinkedFlagMatching
Expand Down Expand Up @@ -813,6 +814,11 @@ export class LazyLoadedSessionRecording implements LazyLoadedSessionRecordingInt
this._makeSamplingDecision(this.sessionId)
this._startRecorder()

if (!this.isStarted) {
this._rrwebError = true
return
}

// calling addEventListener multiple times is safe and will not add duplicates
addEventListener(window, 'beforeunload', this._onBeforeUnload)
addEventListener(window, 'offline', this._onOffline)
Expand Down Expand Up @@ -1124,6 +1130,7 @@ export class LazyLoadedSessionRecording implements LazyLoadedSessionRecordingInt
isRecordingEnabled: true,
// things that do still vary
isSampled: this._isSampled,
rrwebError: this._rrwebError,
urlTriggerMatching: this._urlTriggerMatching,
eventTriggerMatching: this._eventTriggerMatching,
linkedFlagMatching: this._linkedFlagMatching,
Expand Down Expand Up @@ -1636,6 +1643,16 @@ export class LazyLoadedSessionRecording implements LazyLoadedSessionRecordingInt
...sessionRecordingOptions,
})

if (!this._stopRrweb) {
this._rrwebError = true
logger.error(
'rrweb failed to start - Loss of recording data is possible. Check the browser console for rrweb errors.'
)
return
}

this._rrwebError = false

// We reset the last activity timestamp, resetting the idle timer
this._lastActivityTimestamp = Date.now()
// stay unknown if we're not sure if we're idle or not
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
export const PAUSED = 'paused'
export const PENDING_CONFIG = 'pending_config'
export const LAZY_LOADING = 'lazy_loading'
export const RRWEB_ERROR = 'rrweb_error'

const TRIGGER = 'trigger'
export const TRIGGER_ACTIVATED = TRIGGER + '_activated'
Expand All @@ -25,6 +26,7 @@
get receivedFlags(): boolean
get isRecordingEnabled(): false | true | undefined
get isSampled(): false | true | null
get rrwebError(): boolean
get urlTriggerMatching(): URLTriggerMatching
get eventTriggerMatching(): EventTriggerMatching
get linkedFlagMatching(): LinkedFlagMatching
Expand All @@ -49,7 +51,7 @@
* the sample rate determined this session should be sent to the server.
*/
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const sessionRecordingStatuses = [DISABLED, SAMPLED, ACTIVE, BUFFERING, PAUSED, PENDING_CONFIG, LAZY_LOADING] as const
const sessionRecordingStatuses = [DISABLED, SAMPLED, ACTIVE, BUFFERING, PAUSED, PENDING_CONFIG, LAZY_LOADING, RRWEB_ERROR] as const

Check failure on line 54 in packages/browser/src/extensions/replay/external/triggerMatching.ts

View workflow job for this annotation

GitHub Actions / Lint

Replace `DISABLED,·SAMPLED,·ACTIVE,·BUFFERING,·PAUSED,·PENDING_CONFIG,·LAZY_LOADING,·RRWEB_ERROR` with `⏎····DISABLED,⏎····SAMPLED,⏎····ACTIVE,⏎····BUFFERING,⏎····PAUSED,⏎····PENDING_CONFIG,⏎····LAZY_LOADING,⏎····RRWEB_ERROR,⏎`
export type SessionRecordingStatus = (typeof sessionRecordingStatuses)[number]

// while we have both lazy and eager loaded replay we might get either type of config
Expand Down Expand Up @@ -395,6 +397,10 @@

// we need a no-op matcher before we can lazy-load the other matches, since all matchers wait on remote config anyway
export function nullMatchSessionRecordingStatus(triggersStatus: RecordingTriggersStatus): SessionRecordingStatus {
if (triggersStatus.rrwebError) {
return RRWEB_ERROR
}

if (!triggersStatus.isRecordingEnabled) {
return DISABLED
}
Expand All @@ -403,6 +409,10 @@
}

export function anyMatchSessionRecordingStatus(triggersStatus: RecordingTriggersStatus): SessionRecordingStatus {
if (triggersStatus.rrwebError) {
return RRWEB_ERROR
}

if (!triggersStatus.receivedFlags) {
return BUFFERING
}
Expand Down Expand Up @@ -446,6 +456,10 @@
}

export function allMatchSessionRecordingStatus(triggersStatus: RecordingTriggersStatus): SessionRecordingStatus {
if (triggersStatus.rrwebError) {
return RRWEB_ERROR
}

if (!triggersStatus.receivedFlags) {
return BUFFERING
}
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/extensions/replay/types/rrweb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export type recordOptions = {

// Replication of `record` from inside `@rrweb/record`
export type rrwebRecord = {
(options: recordOptions): () => void
(options: recordOptions): (() => void) | undefined
Copy link
Member

Choose a reason for hiding this comment

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

i wish i'd finished moving rrweb into this monorepo... we should really make this explicit
undefined to mean error is so vague 🙈

addCustomEvent: (tag: string, payload: any) => void
takeFullSnapshot: () => void
mirror: {
Expand Down
Loading