Skip to content

fix(abort): do not lose in-flight AbortSignal under tracing#41658

Open
yury-s wants to merge 1 commit into
microsoft:mainfrom
yury-s:fix-abort-signal-tracing-race
Open

fix(abort): do not lose in-flight AbortSignal under tracing#41658
yury-s wants to merge 1 commit into
microsoft:mainfrom
yury-s:fix-abort-signal-tracing-race

Conversation

@yury-s

@yury-s yury-s commented Jul 7, 2026

Copy link
Copy Markdown
Member

Summary

  • An in-flight action aborted via AbortSignal could be silently dropped when tracing is enabled, causing the action to hang until the test timed out (surfaced as flaky failures in the Tracing CI config, e.g. page-click.spec.ts "should throw an Error when aborted in-flight with a string reason").
  • Root cause: the client __abort__ message aborts the server-side ProgressController, but the controller was only registered after onBeforeCall recorded the "before" trace snapshot. The abort raced into that window, found no controller, and was dropped.
  • Fix: register the progress controller before running instrumentation, and let ProgressController buffer an abort that arrives before run() starts so it is applied as soon as the command begins.

Failing test: https://github.com/microsoft/playwright/actions/runs/28827888598/job/85495105077?pr=41653

When an action is aborted via an AbortSignal, the client sends an
`__abort__` message that aborts the server-side ProgressController for
the call. With tracing enabled, `onBeforeCall` records a "before"
snapshot, which delayed registering the progress controller until after
the snapshot completed. The `__abort__` message could arrive during that
window, find no registered controller, and be silently dropped — leaving
the action to run until the test timed out.

Register the progress controller before running instrumentation, and let
ProgressController buffer an abort that arrives before run() starts so it
is applied as soon as the command begins.
@pavelfeldman

Copy link
Copy Markdown
Member

Check out #41643, it might be a bit better. Let alone earlier!

@yury-s yury-s added the CQ1 label Jul 7, 2026
@yury-s yury-s requested a review from Skn0tt July 7, 2026 01:06
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Test results for "MCP"

1 failed
❌ [webkit] › mcp/annotate.spec.ts:173 › user-initiated annotate downloads zip with feedback.md @mcp-macos-latest-webkit

7642 passed, 1132 skipped


Merge workflow run.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

5 flaky ⚠️ [chromium-library] › library/video.spec.ts:717 › screencast › should work with video+trace `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/har-websocket.spec.ts:235 › should attach websocket messages `@chromium-ubuntu-22.04-node24`
⚠️ [chromium-library] › library/chromium/oopif.spec.ts:282 › should click `@chromium-ubuntu-22.04-node22`
⚠️ [chromium-library] › library/inspector/cli-codegen-3.spec.ts:224 › cli codegen › should generate frame locators (4) `@chromium-ubuntu-22.04-node22`
⚠️ [chromium-library] › library/video.spec.ts:717 › screencast › should work with video+trace `@chromium-ubuntu-22.04-node22`

49493 passed, 1164 skipped


Merge workflow run.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Test results for "tests 2"

3 failed
❌ [electron-page] › page/page-autowaiting-no-hang.spec.ts:41 › assigning location to about:blank @electron-macos-latest
❌ [electron-page] › page/page-autowaiting-no-hang.spec.ts:46 › assigning location to about:blank after non-about:blank @electron-macos-latest
❌ [firefox-library] › library/heap.spec.ts:223 › should not leak workers @firefox-macos-15-xlarge

36 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:564 › should not throw an error when evaluation does a synchronous navigation and returns an object `@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/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/video.spec.ts:717 › screencast › should work with video+trace `@chromium-windows-latest`
⚠️ [chromium-library] › library/chromium/chromium.spec.ts:371 › should produce network events, routing, and annotations for Service Worker `@chromium-macos-15-large`
⚠️ [chromium-library] › library/heap.spec.ts:304 › cycle handles `@chromium-macos-15-large`
⚠️ [chromium-library] › library/screenshot.spec.ts:213 › page screenshot › should not hang when event loop is blocked `@chromium-macos-14-xlarge`
⚠️ [chromium-library] › library/chromium/oopif.spec.ts:282 › should click `@chromium-macos-15-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/browsertype-connect.spec.ts:141 › launchServer › should be able to reconnect to a browser `@chrome-macos-latest`
⚠️ [chromium-library] › library/browsertype-connect.spec.ts:1132 › launchServer only › should be able to reconnect to a browser 12 times without warnings `@chrome-macos-latest`
⚠️ [chromium-library] › library/chromium/chromium.spec.ts:211 › should intercept service worker requests (main and within) `@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`
⚠️ [firefox-page] › page/page-emulate-media.spec.ts:144 › should keep reduced motion and color emulation after reload `@tracing-firefox`
⚠️ [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:209 › should fire close event for a persistent context `@firefox-macos-15-large`
⚠️ [firefox-library] › library/inspector/cli-codegen-test.spec.ts:20 › should print the correct imports and context options `@firefox-macos-15-large`
⚠️ [firefox-library] › library/tracing.spec.ts:209 › should respect tracesDir and name `@firefox-macos-15-large`
⚠️ [firefox-library] › library/inspector/cli-codegen-3.spec.ts:224 › cli codegen › should generate frame locators (4) `@firefox-windows-latest`
⚠️ [webkit-library] › library/playwright-client.spec.ts:26 › should connect from a page and drive the same browser `@tracing-webkit`
⚠️ [webkit-library] › library/video.spec.ts:371 › screencast › should scale frames down to the requested size `@webkit-macos-15-large`
⚠️ [webkit-page] › page/page-request-continue.spec.ts:194 › post data › should compute content-length from post data `@webkit-macos-15-large`
⚠️ [webkit-page] › page/page-set-input-files.spec.ts:38 › should upload a folder `@webkit-macos-15-large`
⚠️ [android-page] › page/page-request-fulfill.spec.ts:81 › should fulfill with unuassigned status codes

105904 passed, 4476 skipped, 3 did not run


Merge workflow run.

@yury-s yury-s requested a review from dgozman July 7, 2026 04:07
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