Repository navigation
Conversation
Two related fixes so the headless-Chrome QA scripts run on Windows: scripts/chrome-port.mjs: replace rmSync with async rm (maxRetries 12, retryDelay 150). When a killed Chrome's file handles outlive the kill, Windows reports a sharing violation (EPERM); a single synchronous unlink then throws and masks the real startup error. If cleanup still fails, both errors are wrapped in AggregateError so the original cause is never lost. scripts/chrome-startup.test.mjs: the fake-chrome shim is a /bin/sh script, which cannot run on Windows; the test never reached the cleanup path it verifies. On win32, use process.execPath as an early-exit Chrome substitute and assert profile directories disappear via tmpdir snapshots. POSIX path unchanged. Tested on Windows 11 (10.0.26100), Node v24.19.0: node --test scripts/chrome-startup.test.mjs passes 3/3.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two small, related fixes so the headless-Chrome QA toolchain (
capture,drive,turn-check) runs on Windows. Both patches are no-ops on POSIX behavior-wise, and the full startup test suite passes 3/3 on Windows 11 (10.0.26100), Node v24.19.0.Patch 1 —
scripts/chrome-port.mjs: retry profile cleanup so a transientEPERMdoes not mask the real errorFirst principles. On Windows, a file cannot be deleted while any process holds an open handle to it. When Chrome fails to start and is killed with
SIGKILL, kernel handle teardown is asynchronous: the dying child's handles to its--user-data-dirprofile files can briefly outlive the kill. The current cleanup does a single synchronous unlink:A sharing violation (
EPERM) on any profile file at that instant makesrmSyncthrow inside thecatchblock — the block whose entire job is to surface the real diagnostic (Chrome exited before startup: ...). The caller then sees a misleadingEPERMinstead of the actual cause, breaking the error contract thatchrome-startup.test.mjsitself asserts on (/Chrome exited before startup/).Why the fix is principled rather than a workaround. Node's
fs.rmalready implements exactly this semantics —maxRetries/retryDelayexist primarily because Windows sharing violations are a documented, transient filesystem behavior — so the patch simply opts into the platform-appropriate API:~1.8 s worst case, only spent when an
EPERM-class error actually occurs; the POSIX happy path is unaffected. If cleanup still fails, both errors are wrapped in anAggregateError("Chrome startup and profile cleanup both failed") so the original startup error is never lost — the diagnostic contract is preserved even in the worst case.Patch 2 —
scripts/chrome-startup.test.mjs: make the fake-Chrome shim work on WindowsFirst principles. The test fabricates a
chromebinary as a#!/bin/shshell script withchmod 0o755. On Windows there is no/bin/sh, so spawning the shim fails before any launcher logic runs — the test never reaches the cleanup path it exists to verify. It's not that the test fails loudly; it fails in a way that tells you nothing about profile cleanup.Fix. On
win32, useprocess.execPath(Node itself) as the early-exit Chrome substitute: Node is always present, requires no new dependency, and invoking it with Chrome's arguments guarantees an immediate non-listening exit, exercising the same "Chrome exited before startup" cleanup path. Cleanup is then verified directly — snapshot theiso-capture-/iso-drive-/turn-check-prefixed temp profile directories before and after, and assert none leak. The POSIX path (shim script + receipt file) is kept byte-for-byte and remains the code path used on macOS/Linux.Testing
node --test scripts/chrome-startup.test.mjs→ 3/3 pass (~1.3 s).1482e84(currentmain).EPERM-class errors that essentially do not occur here, and the shim path is untouched.