Skip to content

Commit 46ba348

Browse files
fix(call): prevent stale-data VideoCall mount and guard startRecording (#1230)
* fix(call): prevent stale-data VideoCall mount and guard startRecording (#1226) Key Stage by stage.id to eliminate the one-render window where player.stage updates before useStage(), causing VideoCall to mount with stale discussion config during stage transitions. Also guard startRecording() return value to prevent TypeError crash if the Daily SDK returns undefined in a transitional state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(call): add deferred Sentry alert for non-Promise startRecording and test Address Copilot review: add deferred Sentry capture in the non-Promise fallback path so silently-missed recordings are surfaced. Add REC-005b test verifying the component doesn't crash when startRecording returns undefined and that Sentry fires the deferred alert. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 850993d commit 46ba348

4 files changed

Lines changed: 88 additions & 22 deletions

File tree

client/src/Game.jsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ export function Game() {
136136
<Profile />
137137
</div>
138138
<div className="absolute top-12 left-0 right-0 bottom-0 m-2">
139-
<Stage />
139+
<Stage key={stage.id} />
140140
</div>
141141
</>
142142
);

client/src/call/hooks/useCallStartSignaling.js

Lines changed: 39 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -34,28 +34,46 @@ export function useCallStartSignaling(callObject, recordingEnabled, stageId) {
3434
const startRecordingIfNeeded = () => {
3535
if (recordingEnabled && !recordingStartedRef.current) {
3636
recordingStartedRef.current = true;
37-
callObject.startRecording({ type: "raw-tracks" }).then(
38-
() => console.log("[Recording] Started raw-tracks recording from client"),
39-
(err) => {
40-
console.warn("[Recording] Failed to start recording:", err.message);
41-
recordingStartedRef.current = false;
37+
const result = callObject.startRecording({ type: "raw-tracks" });
38+
if (result && typeof result.then === "function") {
39+
result.then(
40+
() => console.log("[Recording] Started raw-tracks recording from client"),
41+
(err) => {
42+
console.warn("[Recording] Failed to start recording:", err.message);
43+
recordingStartedRef.current = false;
4244

43-
// Defer Sentry alert: wait 5s and check if another participant
44-
// successfully started recording (indicated by recording-started
45-
// event setting recordingConfirmedRef). This avoids false alarms
46-
// when one client fails but another succeeds — Daily broadcasts
47-
// recording-started to all participants regardless of who initiated.
48-
const timer = setTimeout(() => {
49-
if (!recordingConfirmedRef.current) {
50-
Sentry.captureMessage("Recording not started for stage", {
51-
level: "error",
52-
extra: { triggeringError: err.message, stageId },
53-
});
54-
}
55-
}, 5000);
56-
pendingTimers.push(timer);
57-
}
58-
);
45+
// Defer Sentry alert: wait 5s and check if another participant
46+
// successfully started recording (indicated by recording-started
47+
// event setting recordingConfirmedRef). This avoids false alarms
48+
// when one client fails but another succeeds — Daily broadcasts
49+
// recording-started to all participants regardless of who initiated.
50+
const timer = setTimeout(() => {
51+
if (!recordingConfirmedRef.current) {
52+
Sentry.captureMessage("Recording not started for stage", {
53+
level: "error",
54+
extra: { triggeringError: err.message, stageId },
55+
});
56+
}
57+
}, 5000);
58+
pendingTimers.push(timer);
59+
}
60+
);
61+
} else {
62+
console.warn("[Recording] startRecording() returned non-Promise; call may be in transitional state");
63+
recordingStartedRef.current = false;
64+
65+
// Defer Sentry alert: if no participant confirms recording within 5s,
66+
// surface the issue so we don't silently miss a whole stage of recording.
67+
const timer = setTimeout(() => {
68+
if (!recordingConfirmedRef.current) {
69+
Sentry.captureMessage("Recording not started for stage", {
70+
level: "error",
71+
extra: { triggeringError: "non-promise return", stageId },
72+
});
73+
}
74+
}, 5000);
75+
pendingTimers.push(timer);
76+
}
5977
}
6078
};
6179

playwright/component-tests/video-call/mocked/Recording.ct.jsx

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,51 @@ test.describe('Client-Side Recording (useCallStartSignaling)', () => {
207207
expect(recordingErrors[0].hint.extra.triggeringError).toBeTruthy();
208208
});
209209

210+
/**
211+
* REC-005b: No crash when startRecording returns undefined (non-Promise)
212+
*
213+
* Validates:
214+
* - callObject.startRecording() returns undefined (transitional state)
215+
* - Component does NOT crash (no TypeError on .then())
216+
* - Sentry fires deferred alert when no recording confirmed
217+
*
218+
* Regression test for #1226
219+
*/
220+
test('REC-005b: no crash when startRecording returns undefined', async ({ mount, page }) => {
221+
test.slow();
222+
223+
// Configure startRecording to return undefined BEFORE mount
224+
await page.evaluate(() => {
225+
window.__mockStartRecordingBehavior = 'return-undefined';
226+
});
227+
228+
const component = await mount(<VideoCall showSelfView />, {
229+
hooksConfig: recordingEnabledConfig,
230+
});
231+
await expect(component).toBeVisible({ timeout: 15000 });
232+
233+
// Verify startRecording was called (but returned undefined)
234+
await expect(async () => {
235+
const calls = await page.evaluate(() => window.mockCallObject._startRecordingCalls);
236+
expect(calls.length).toBeGreaterThanOrEqual(1);
237+
}).toPass({ timeout: 5000 });
238+
239+
// Reset Sentry after mount so we start clean
240+
await page.evaluate(() => window.mockSentryCaptures.reset());
241+
242+
// Wait past the 5s deferred Sentry timer (no recording-started event)
243+
await page.waitForTimeout(6000);
244+
245+
// Sentry should fire with "non-promise return" as the triggering error
246+
const captures = await page.evaluate(() => window.mockSentryCaptures);
247+
const recordingErrors = captures.messages.filter(
248+
m => m.message === 'Recording not started for stage'
249+
);
250+
expect(recordingErrors.length).toBeGreaterThanOrEqual(1);
251+
expect(recordingErrors[0].hint.level).toBe('error');
252+
expect(recordingErrors[0].hint.extra.triggeringError).toBe('non-promise return');
253+
});
254+
210255
/**
211256
* REC-006: Sentry fires on recording-error when no recording confirmed
212257
*

playwright/mocks/daily/MockDailyProvider.jsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,9 @@ class MockCallObject extends MockEventEmitter {
105105
if (this._startRecordingBehavior === 'reject') {
106106
return Promise.reject(new Error('Recording failed (mock)'));
107107
}
108+
if (this._startRecordingBehavior === 'return-undefined') {
109+
return undefined;
110+
}
108111
return Promise.resolve();
109112
}
110113

0 commit comments

Comments
 (0)