Skip to content

Commit 4388b87

Browse files
fix(replay): address review — stopReplay leak, stale session-end job, throttle, a11y
- web SDK: stopReplay() now detaches the session-rotation listener so a later rotation can't restart the recorder after a manual stop. Extract a shared startForSession() helper (was duplicated between initial start + restart). - worker: createSessionEndJob replaces any existing delayed job for the device before enqueueing. Previously a client session_id rotation left the stale (deviceId-keyed) job with the old payload, so every later event saw isNewSession=true and spawned duplicate session_start events. - recorder: throttle onUserActivity to <=1/sec — MouseMove/Scroll fired a localStorage write on every event. lastActivityMs still updates on all. - dashboard: aria-pressed on the Skip-idle toggle for screen readers.
1 parent cb291eb commit 4388b87

4 files changed

Lines changed: 57 additions & 30 deletions

File tree

apps/start/src/components/sessions/replay/index.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,7 @@ function ReplayContent({
324324
{hasReplay && (
325325
<button
326326
type="button"
327+
aria-pressed={skipInactive}
327328
onClick={() => setSkipInactive((v) => !v)}
328329
title={
329330
skipInactive

apps/worker/src/utils/session-handler.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,20 @@ export async function createSessionEndJob({
2929
}: {
3030
payload: IServiceCreateEventPayload;
3131
}) {
32+
const jobId = getSessionEndJobId(payload.projectId, payload.deviceId);
33+
// The job is keyed by deviceId, and BullMQ's add() is a no-op when a job
34+
// with the same id already exists. When a client rotates its session_id
35+
// (Phase 5) a stale delayed job still carries the OLD session's payload —
36+
// without replacing it, later events keep matching the old session and
37+
// spawn duplicate session_start events. Remove any existing delayed job so
38+
// the new one carries the current session's payload.
39+
const existing = await sessionsQueue.getJob(jobId);
40+
if (existing) {
41+
await existing.remove().catch(() => {
42+
// Job may be active/locked — remove can throw. Fall back to the add
43+
// below (a no-op if the id is still present); non-fatal.
44+
});
45+
}
3246
return sessionsQueue.add(
3347
'session',
3448
{
@@ -37,7 +51,7 @@ export async function createSessionEndJob({
3751
},
3852
{
3953
delay: SESSION_TIMEOUT,
40-
jobId: getSessionEndJobId(payload.projectId, payload.deviceId),
54+
jobId,
4155
attempts: 3,
4256
backoff: {
4357
type: 'exponential',

packages/sdks/web/src/index.ts

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,13 @@ export class OpenPanel extends OpenPanelBase {
6969
*/
7070
private windowId?: string;
7171

72+
/**
73+
* Unsubscribe for the session-rotation listener registered in
74+
* maybeStartReplay(). Captured so stopReplay() can detach it — otherwise a
75+
* later rotation would restart the recorder after a manual stop.
76+
*/
77+
private replaySessionUnsub?: () => void;
78+
7279
constructor(public options: OpenPanelOptions) {
7380
super({
7481
sdk: 'web',
@@ -218,21 +225,28 @@ export class OpenPanel extends OpenPanelBase {
218225
// start a new one with the fresh id (chunk_index=0 + FullSnapshot).
219226
let activeSessionId = this.sessionId;
220227
const bumpActivity = () => this.sessionManager?.bumpActivity();
221-
startReplayRecorder(
222-
opts,
223-
(chunk) => {
224-
this.send({
225-
type: 'replay',
226-
payload: {
227-
...chunk,
228-
session_id: activeSessionId,
229-
},
230-
});
231-
},
232-
bumpActivity,
233-
);
228+
// Shared recorder-start so the initial start and the post-rotation restart
229+
// stay in sync (single source of truth for the chunk-send closure).
230+
const startForSession = (sessionId: string) => {
231+
activeSessionId = sessionId;
232+
startReplayRecorder(
233+
opts,
234+
(chunk) => {
235+
this.send({
236+
type: 'replay',
237+
payload: {
238+
...chunk,
239+
session_id: activeSessionId,
240+
},
241+
});
242+
},
243+
bumpActivity,
244+
);
245+
};
234246

235-
this.sessionManager.onSessionIdChanged((newId) => {
247+
startForSession(this.sessionId);
248+
249+
this.replaySessionUnsub = this.sessionManager.onSessionIdChanged((newId) => {
236250
// Match PostHog: a session rotation also mints a fresh window_id. The
237251
// recorder restarts with a new FullSnapshot at chunk_index=0, so the
238252
// post-rotation recording is fully self-describing — the dashboard
@@ -246,24 +260,15 @@ export class OpenPanel extends OpenPanelBase {
246260
});
247261
this.sessionId = newId;
248262
stopReplayRecorder();
249-
activeSessionId = newId;
250-
startReplayRecorder(
251-
opts,
252-
(chunk) => {
253-
this.send({
254-
type: 'replay',
255-
payload: {
256-
...chunk,
257-
session_id: activeSessionId,
258-
},
259-
});
260-
},
261-
bumpActivity,
262-
);
263+
startForSession(newId);
263264
});
264265
}
265266

266267
public stopReplay() {
268+
// Detach the rotation listener first, so a later session rotation can't
269+
// restart the recorder after a manual stop.
270+
this.replaySessionUnsub?.();
271+
this.replaySessionUnsub = undefined;
267272
stopReplayRecorder();
268273
}
269274

packages/sdks/web/src/replay/recorder.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ export function startReplayRecorder(
8383
// always captured. lastActivityMs tracks the last real user interaction.
8484
let isIdle = false;
8585
let lastActivityMs = Date.now();
86+
// MouseMove/Scroll fire many times a second and each onUserActivity() bump
87+
// writes localStorage. Throttle the notify to at most once/sec — lastActivityMs
88+
// still updates on every interactive event (in-memory, cheap).
89+
let lastActivityNotifyMs = 0;
8690

8791
function flush(isFullSnapshot: boolean): void {
8892
if (buffer.length === 0) return;
@@ -148,7 +152,10 @@ export function startReplayRecorder(
148152

149153
if (interactive) {
150154
lastActivityMs = event.timestamp;
151-
onUserActivity?.();
155+
if (event.timestamp - lastActivityNotifyMs >= 1000) {
156+
lastActivityNotifyMs = event.timestamp;
157+
onUserActivity?.();
158+
}
152159
if (isIdle) {
153160
// Returning from idle. The player's DOM mirror is stale (we dropped
154161
// the mutations that happened while idle), so force a fresh

0 commit comments

Comments
 (0)