Skip to content

fix(browser): drain route handlers before closing context - #157

Open
nichenqin wants to merge 2 commits into
mainfrom
fix/drain-browser-routes
Open

nichenqin wants to merge 2 commits into
mainfrom
fix/drain-browser-routes

Conversation

@nichenqin

Copy link
Copy Markdown
Contributor

Changes

Wait for in-flight Playwright page route handlers before disposing their browser context. Context cleanup still runs if draining fails. This fixes the link-picker runner teardown error observed during teable-ee #3518 without suppressing route failures.

Verification

  • Real Chromium smoke: immediate context disposal reproduces route.fetch: Request context disposed; draining routes allows the same request and delayed fulfillment to finish.
  • pnpm check:types: passed, 361 files.
  • Prettier check on the changed file: passed.

No production deployment or automatic merge.

@nichenqin

Copy link
Copy Markdown
Contributor Author

The real browser case link/y554-picker-keeps-selection-across-tabs is running against repaired teable-ee head 43929b7aac using this lab branch: https://github.com/teableio/teable-e2e-lab/actions/runs/35041620365 . The local deterministic Chromium lifecycle smoke and PR pnpm check already passed. No merge performed.

@nichenqin

Copy link
Copy Markdown
Contributor Author

Follow-up: the first real-browser run was workflow-green but contained route.fulfill: Route is already handled!; it is not clean acceptance.

Root cause reproduced with the pinned Playwright 1.57 and two concurrent delayed routes: unrouteAll({ behavior: "wait" }) clears the client route list before draining. The first completed handler can then unregister server interception while a second handler still intends to fulfill.

The runner now drains its delayed response promises before unregistering interception and aborts newly arriving requests during shutdown. Rejected in-flight operations still propagate after cleanup.

Deterministic Chromium smoke: old lifecycle reproduced the exact Route is already handled! error; the new lifecycle completed both requests with zero errors. Typecheck passed (361 files). A new real-browser CI run will verify the committed follow-up.

@nichenqin

Copy link
Copy Markdown
Contributor Author

Clean acceptance on lab head 659fd80 against teable-ee 43929b7aac: https://github.com/teableio/teable-e2e-lab/actions/runs/35042727730

  • Real link-picker browser regression: 1 passed.
  • Downloaded and inspected the complete job log: no Unhandled Errors, route.fetch disposal failure, or route.fulfill already-handled failure.
  • Run acceptance/comparison job: passed.
  • Latest PR pnpm check and CLA checks: passed.
  • Local two-concurrent-route reproduction on Playwright 1.57: old lifecycle reproduced the exact already-handled error; new lifecycle had zero errors.

This supersedes the earlier workflow-green run that still contained an unhandled rejection. No merge performed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant