Skip to content

Commit 523e5a4

Browse files
session recording: measure record_min_ms from first user event
The record_min_ms gate was previously anchored to the timestamp of the first emitted rrweb event of any type. On a typical page load the first event is a Meta or FullSnapshot fired before the user interacts, so a session like "load page -> idle for 10s -> click once" would immediately cross the threshold and start flushing a near-empty replay, producing a sub-record_min_ms recording on the server. Anchor _recordMinMsCheckStart to the first user-activity event instead, which is the same notion of "recording duration" the player surfaces and the customer is configuring against. Updates an existing unit test whose timings encoded the old behavior and adds a regression test for the idle-then-click case.
1 parent 3f8ce87 commit 523e5a4

2 files changed

Lines changed: 54 additions & 6 deletions

File tree

src/recorder/session-recording.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -281,10 +281,14 @@ SessionRecording.prototype.startRecording = function (shouldStopBatcher) {
281281
this._onIdleTimeout();
282282
return;
283283
}
284-
if (this._recordMinMsCheckStart === null) {
285-
this._recordMinMsCheckStart = ev.timestamp;
286-
}
287284
if (isUserEvent(ev)) {
285+
// Measure record_min_ms from the first user event, not the first rrweb event.
286+
// Initial Meta/FullSnapshot/Mutation events fire at page load before any
287+
// interaction, so anchoring on them would let an idle-then-click flush a
288+
// near-empty replay (see MULTI-436).
289+
if (this._recordMinMsCheckStart === null) {
290+
this._recordMinMsCheckStart = ev.timestamp;
291+
}
288292
if (this.batcher.stopped && ev.timestamp - this._recordMinMsCheckStart >= this.recordMinMs) {
289293
this.batcher.start();
290294
}

tests/unit/session-recording.js

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -685,12 +685,56 @@ describe(`SessionRecording`, function() {
685685
clock.tick(2000);
686686
expect(rec.batcher.stopped).to.be.true;
687687

688-
// user event before record_min_ms should not start the batcher
688+
// user event before record_min_ms (measured from first user event) should not start the batcher
689689
mockRrweb.emit(EventType.IncrementalSnapshot, IncrementalSource.MouseInteraction);
690690
expect(rec.batcher.stopped).to.be.true;
691691

692-
// after record_min_ms, a user event should start the batcher
693-
clock.tick(4000);
692+
// after record_min_ms has elapsed since the first user event, the next user event starts the batcher
693+
clock.tick(recordMinMs);
694+
mockRrweb.emit(EventType.IncrementalSnapshot, IncrementalSource.MouseMove);
695+
expect(rec.batcher.stopped).to.be.false;
696+
697+
rec.stopRecording();
698+
});
699+
700+
it(`measures record_min_ms from first user event, not first rrweb event`, async function() {
701+
// Regression test for MULTI-436: page loads, rrweb emits initial Meta/FullSnapshot,
702+
// user is idle past record_min_ms, then clicks once. Previously this immediately
703+
// started the batcher and flushed a near-empty replay because the threshold was
704+
// measured from the initial snapshot rather than from the first user activity.
705+
const recordMinMs = 8000;
706+
const customMixpanel = new MockMixpanelLib({'record_min_ms': recordMinMs});
707+
708+
const rec = new SessionRecording({
709+
mixpanelInstance: customMixpanel,
710+
replayId: `min-ms-idle-load-replay`,
711+
rrwebRecord: mockRrweb.recordStub,
712+
sharedLockStorage: localStorage,
713+
});
714+
715+
rec.startRecording();
716+
expect(rec.batcher.stopped).to.be.true;
717+
718+
// initial non-user events at page load (t=0)
719+
mockRrweb.emit(EventType.Meta);
720+
mockRrweb.emit(EventType.FullSnapshot);
721+
722+
// user is idle for longer than record_min_ms, then interacts once
723+
clock.tick(recordMinMs + 1000);
724+
mockRrweb.emit(EventType.IncrementalSnapshot, IncrementalSource.MouseInteraction);
725+
726+
// batcher must NOT start: there has only been one user event so far,
727+
// so the recorded user-activity window is 0ms, well below record_min_ms.
728+
expect(rec.batcher.stopped).to.be.true;
729+
730+
// another user event still inside record_min_ms (measured from the first
731+
// user event) must also keep the batcher stopped.
732+
clock.tick(recordMinMs - 1);
733+
mockRrweb.emit(EventType.IncrementalSnapshot, IncrementalSource.MouseMove);
734+
expect(rec.batcher.stopped).to.be.true;
735+
736+
// one more tick crosses record_min_ms from the first user event; batcher starts.
737+
clock.tick(2);
694738
mockRrweb.emit(EventType.IncrementalSnapshot, IncrementalSource.MouseMove);
695739
expect(rec.batcher.stopped).to.be.false;
696740

0 commit comments

Comments
 (0)