Skip to content

fix(dispatcher): lift progress controller creation before instrumentation#41643

Open
Skn0tt wants to merge 1 commit into
mainfrom
fix-abort-signal-race
Open

fix(dispatcher): lift progress controller creation before instrumentation#41643
Skn0tt wants to merge 1 commit into
mainfrom
fix-abort-signal-race

Conversation

@Skn0tt

@Skn0tt Skn0tt commented Jul 6, 2026

Copy link
Copy Markdown
Member

Lift ProgressController creation and registration to happen before onBeforeCall instrumentation, so that __abort__ messages can find the controller even when they arrive very early.

ProgressController now also stores an abort that arrives while it is still in the 'before' state and applies it once run() starts.

This fixes the flaky "should throw an Error when aborted in-flight with a string reason" test on tracing bots, where the extra instrumentation overhead caused the abort message to arrive before the controller was registered.

…tion

ProgressController creation and registration now happens before the
onBeforeCall instrumentation, so __abort__ messages can find the
controller even when they arrive very early.

ProgressController also stores an abort that arrives while it is still
in the 'before' state and applies it once run() starts.

This fixes the flaky "should throw an Error when aborted in-flight
with a string reason" test on tracing bots, where the extra
instrumentation overhead caused the abort message to arrive before the
controller was registered.
@Skn0tt Skn0tt force-pushed the fix-abort-signal-race branch from 9f96567 to 0f6ac87 Compare July 6, 2026 14:36
@Skn0tt Skn0tt changed the title fix(dispatcher): register ProgressController early and honor pre-run aborts fix(dispatcher): lift progress controller creation before instrumentation Jul 6, 2026
@Skn0tt Skn0tt requested a review from dgozman July 6, 2026 14:36
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

7 flaky ⚠️ [chromium-library] › library/video.spec.ts:717 › screencast › should work with video+trace `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/popup.spec.ts:260 › should not throw when click closes popup `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/video.spec.ts:645 › screencast › should capture full viewport `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/chromium/chromium.spec.ts:211 › should intercept service worker requests (main and within) `@chromium-ubuntu-22.04-node24`
⚠️ [chromium-library] › library/video.spec.ts:275 › screencast › should capture navigation `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/video.spec.ts:717 › screencast › should work with video+trace `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/video.spec.ts:717 › screencast › should work with video+trace `@chromium-ubuntu-22.04-node22`

49491 passed, 1164 skipped


Merge workflow run.

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Test results for "MCP"

4 failed
❌ [chrome] › mcp/annotate.spec.ts:57 › should capture multiple screenshots in one annotation @mcp-windows-latest-chrome
❌ [firefox] › mcp/annotate.spec.ts:173 › user-initiated annotate downloads zip with feedback.md @mcp-macos-latest-firefox
❌ [firefox] › mcp/annotate.spec.ts:446 › should switch screencast to -s session on show --annotate @mcp-macos-latest-firefox
❌ [webkit] › mcp/http.spec.ts:104 › http transport browser lifecycle (isolated) @mcp-ubuntu-latest-webkit

7639 passed, 1132 skipped


Merge workflow run.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Test results for "tests 2"

4 fatal errors, not part of any test
9 failed
❌ [electron-page] › page/page-autowaiting-no-hang.spec.ts:41 › assigning location to about:blank @electron-macos-latest
❌ [chromium-page] › page/page-network-response.spec.ts:454 › should give a readable error when response.body() races with navigation @tracing-chromium
❌ [chromium-page] › page/page-evaluate.spec.ts:374 › should properly serialize PerformanceMeasure object @chrome-beta-ubuntu-22.04
❌ [firefox-library] › library/defaultbrowsercontext-2.spec.ts:139 › should create userDataDir if it does not exist @firefox-macos-15-large
❌ [firefox-library] › library/defaultbrowsercontext-2.spec.ts:180 › should have passed URL when launching with ignoreDefaultArgs: true @firefox-macos-15-large
❌ [firefox-library] › library/defaultbrowsercontext-2.spec.ts:202 › should handle exception @firefox-macos-15-large
❌ [firefox-library] › library/har.spec.ts:517 › should record failed request overrides @firefox-macos-15-large
❌ [firefox-library] › library/inspector/cli-codegen-3.spec.ts:146 › cli codegen › should generate frame locators (2) @firefox-macos-15-large
❌ [webkit-page] › page/page-network-response.spec.ts:454 › should give a readable error when response.body() races with navigation @tracing-webkit

41 flaky ⚠️ [electron-page] › page/page-autowaiting-no-hang.spec.ts:41 › assigning location to about:blank `@electron-ubuntu-latest`
⚠️ [electron-page] › page/page-autowaiting-no-hang.spec.ts:46 › assigning location to about:blank after non-about:blank `@electron-ubuntu-latest`
⚠️ [electron-page] › page/page-evaluate.spec.ts:573 › should not throw an error when evaluation does a synchronous navigation and returns undefined `@electron-ubuntu-latest`
⚠️ [electron-page] › page/page-route.spec.ts:1027 › route.fulfill should throw if called twice `@electron-ubuntu-latest`
⚠️ [electron-page] › page/page-evaluate.spec.ts:564 › should not throw an error when evaluation does a synchronous navigation and returns an object `@electron-macos-latest`
⚠️ [electron-page] › page/page-evaluate.spec.ts:573 › should not throw an error when evaluation does a synchronous navigation and returns undefined `@electron-macos-latest`
⚠️ [electron-page] › page/page-route.spec.ts:1027 › route.fulfill should throw if called twice `@electron-macos-latest`
⚠️ [playwright-test] › ui-mode-test-output.spec.ts:118 › should collapse repeated console messages for test `@realtime-time-runner-chromium-linux`
⚠️ [chromium-library] › library/video.spec.ts:456 › screencast › should be 800x600 with null viewport `@msedge-dev-windows-latest`
⚠️ [chromium-library] › library/chromium/chromium.spec.ts:177 › serviceWorker(), and fromServiceWorker() work `@chrome-ubuntu-22.04`
⚠️ [chromium-library] › library/video.spec.ts:456 › screencast › should be 800x600 with null viewport `@chrome-ubuntu-22.04`
⚠️ [chromium-library] › library/video.spec.ts:456 › screencast › should be 800x600 with null viewport `@msedge-windows-latest`
⚠️ [chromium-library] › library/chromium/css-coverage.spec.ts:112 › should report stylesheets across navigations `@chromium-macos-15-large`
⚠️ [chromium-library] › library/heap.spec.ts:304 › cycle handles `@chromium-macos-15-large`
⚠️ [chromium-page] › page/page-set-input-files.spec.ts:202 › should upload large file with relative path `@chromium-macos-15-large`
⚠️ [chromium-library] › library/chromium/oopif.spec.ts:282 › should click `@chromium-macos-14-xlarge`
⚠️ [chromium-library] › library/screenshot.spec.ts:213 › page screenshot › should not hang when event loop is blocked `@chromium-macos-14-xlarge`
⚠️ [chromium-library] › library/video.spec.ts:645 › screencast › should capture full viewport `@chromium-macos-15-xlarge`
⚠️ [chromium-library] › library/video.spec.ts:456 › screencast › should be 800x600 with null viewport `@chrome-windows-latest`
⚠️ [chromium-library] › library/video.spec.ts:456 › screencast › should be 800x600 with null viewport `@chrome-beta-ubuntu-22.04`
⚠️ [chromium-library] › library/browsercontext-proxy.spec.ts:256 › should throw for socks4 authentication `@chrome-macos-latest`
⚠️ [chromium-library] › library/browsertype-connect.spec.ts:141 › launchServer › should be able to reconnect to a browser `@chrome-macos-latest`
⚠️ [chromium-library] › library/browsertype-connect.spec.ts:189 › launchServer › should ignore page.pause when headed `@chrome-macos-latest`
⚠️ [chromium-library] › library/browsertype-connect.spec.ts:647 › launchServer › should properly disconnect when connection closes from the client side `@chrome-macos-latest`
⚠️ [chromium-library] › library/browsertype-connect.spec.ts:701 › launchServer › should filter launch options `@chrome-macos-latest`
⚠️ [chromium-library] › library/chromium/chromium.spec.ts:177 › serviceWorker(), and fromServiceWorker() work `@chrome-macos-latest`
⚠️ [chromium-library] › library/video.spec.ts:456 › screencast › should be 800x600 with null viewport `@chrome-macos-latest`
⚠️ [electron-page] › page/page-autowaiting-no-hang.spec.ts:41 › assigning location to about:blank `@electron-windows-latest`
⚠️ [electron-page] › page/page-evaluate.spec.ts:564 › should not throw an error when evaluation does a synchronous navigation and returns an object `@electron-windows-latest`
⚠️ [firefox-library] › library/inspector/cli-codegen-3.spec.ts:224 › cli codegen › should generate frame locators (4) `@tracing-firefox`
⚠️ [firefox-page] › page/page-wait-for-function.spec.ts:43 › should poll on interval `@tracing-firefox`
⚠️ [firefox-library] › library/browsertype-connect.spec.ts:525 › launchServer › should saveAs videos from remote browser `@firefox-macos-15-large`
⚠️ [firefox-library] › library/defaultbrowsercontext-2.spec.ts:166 › should have default URL when launching browser `@firefox-macos-15-large`
⚠️ [firefox-library] › library/defaultbrowsercontext-2.spec.ts:209 › should fire close event for a persistent context `@firefox-macos-15-large`
⚠️ [firefox-library] › library/defaultbrowsercontext-2.spec.ts:286 › exposes browser `@firefox-macos-15-large`
⚠️ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:872 › cli codegen › should attribute navigation to press/fill `@firefox-macos-15-large`
⚠️ [firefox-library] › library/inspector/cli-codegen-2.spec.ts:339 › cli codegen › should record open in a new tab with url `@firefox-macos-15-large`
⚠️ [firefox-library] › library/inspector/cli-codegen-javascript.spec.ts:84 › should save the codegen output to a file if specified `@firefox-macos-15-large`
⚠️ [firefox-library] › library/screenshot.spec.ts:213 › page screenshot › should not hang when event loop is blocked `@firefox-macos-15-large`
⚠️ [firefox-library] › library/inspector/cli-codegen-3.spec.ts:224 › cli codegen › should generate frame locators (4) `@firefox-windows-latest`
⚠️ [firefox-library] › library/screenshot.spec.ts:213 › page screenshot › should not hang when event loop is blocked `@firefox-macos-15-xlarge`

105898 passed, 4474 skipped


Merge workflow run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants