fix(screencast): auto-save video when page closes without stop#41744
fix(screencast): auto-save video when page closes without stop#41744sarang-code2 wants to merge 2 commits into
Conversation
When using page.screencast.start({ path }), if the page closes before
stop() is called, the video file was never saved. The server already
stops the recording gracefully, but the client never called
artifact.saveAs().
Listen for Events.Page.Close in the client Screencast constructor and
auto-save the artifact when the page closes while a recording is active.
Fixes microsoft#41608
|
@microsoft-github-policy-service agree |
|
The Why this looks racyThe new test does: await page.close();
expect(fs.existsSync(videoPath)).toBeTruthy();That passes only because the save happens to finish before the assertion runs, not because of any ordering guarantee - could flake on a slower disk or a loaded CI runner. Separately, swallowing the save error with Should |
|
Opened sarang-code2#1 against this branch with a fix for the race condition above, in case it's useful - happy to have it merged in, adjusted, or ignored, your call. Summary: Test results - screencast.spec.ts, 195 runs (13 tests × 15 repeats), all passingTest results - full page-close.spec.ts suite (18 tests), no regressions |
|
For visibility: also opened #41746 directly against |
Follow-up to review feedback on PR microsoft#41744. The auto-save triggered from the once(Events.Page.Close, ...) listener was fire-and-forget — artifact.saveAs(savePath).catch(() => {}) with nothing awaiting it. That meant page.close() could resolve before the video was actually on disk, and a failed save produced no signal at all. Screencast now tracks the in-flight save as _pendingAutoSave, and Page.close() awaits it in a finally block before resolving. Co-authored-by: Git-Raini <216259955+Git-Raini@users.noreply.github.com>
|
Applied the fix from @Git-Raini - Screencast now tracks the in-flight auto-save as _pendingAutoSave, and Page.close() awaits it in a finally block. Thanks for the review and fix! |
Follow-up to review feedback on PR microsoft#41744. The auto-save triggered from the once(Events.Page.Close, ...) listener was fire-and-forget — artifact.saveAs(savePath).catch(() => {}) with nothing awaiting it. That meant page.close() could resolve before the video was actually on disk, and a failed save produced no signal at all. Screencast now tracks the in-flight save as _pendingAutoSave, and Page.close() awaits it in a finally block before resolving. Co-authored-by: Git-Raini <216259955+Git-Raini@users.noreply.github.com>
sarang-code2
force-pushed
the
fix/screencast-auto-save-on-page-close
branch
from
60bf317 to
25133ae
Compare
Follow-up to review feedback on PR microsoft#41744. The auto-save triggered from the once(Events.Page.Close, ...) listener was fire-and-forget — artifact.saveAs(savePath).catch(() => {}) with nothing awaiting it. That meant page.close() could resolve before the video was actually on disk, and a failed save produced no signal at all. Screencast now tracks the in-flight save as _pendingAutoSave, and Page.close() awaits it in a finally block before resolving. Co-authored-by: Git-Raini <216259955+Git-Raini@users.noreply.github.com>
sarang-code2
force-pushed
the
fix/screencast-auto-save-on-page-close
branch
from
25133ae to
a5c1fb4
Compare
2 participants
Closes #41608
Problem
When using
page.screencast.start({ path: 'video.webm' }), if the page closes beforepage.screencast.stop()is called, the video file is never saved to disk. The server already stops the recording gracefully, but the client never callsartifact.saveAs()to write the file.This makes it impossible to record a video showing "the window closing when a button is pressed."
Solution
Listen for
Events.Page.Closein the client-sideScreencastconstructor. When the page closes while a recording is active (with apathoption), automatically save the artifact to the specified path.Changes:
packages/playwright-core/src/client/screencast.ts— Register a one-timePage.Closelistener that auto-saves the artifacttests/library/screencast.spec.ts— Add test verifying auto-save on page closeUsage