Skip to content

fix: bound CdpPage.CloseAsync's wait for target close confirmation - #3530

Merged
kblok merged 1 commit into
masterfrom
fix-page-close-hang-navigation-race
Aug 3, 2026
Merged

fix: bound CdpPage.CloseAsync's wait for target close confirmation#3530
kblok merged 1 commit into
masterfrom
fix-page-close-hang-navigation-race

Conversation

@kblok

@kblok kblok commented Aug 2, 2026

Copy link
Copy Markdown
Member

Summary

ShouldNotThrowAnErrorWhenEvaluationDoesANavigation has been intermittently hanging the whole test host on the CHROME-headful-ubuntu-latest-cdp CI job (Blame collector killing it after 5 minutes of inactivity, repeatedly, always mid-test).

Turns out the hang isn't in the evaluate call at all — it's in the test's teardown, in Page.CloseAsync(). Reproduced it locally (headful Chrome CDP) by capturing raw CDP traffic: when a navigation is still in flight and Target.closeTarget races it, Chrome accepts the close (success: true) but finishes loading the new document first, and in that window can simply go silent on the target — no Target.detachedFromTarget, no Target.targetDestroyed, ever. CloseAsync was waiting on that event with zero timeout, so it just hung forever.

Every other protocol call in this codebase already has a bounded wait via ProtocolTimeout (180s by default). This one didn't. Now it does — same timeout, and since Chrome already accepted the close request, a timeout here just logs a warning and returns instead of throwing.

Locally this turned a reliably-forever hang into a bounded ~3 minute wait, test still passes. Ran the full EvaluationTests, PageTests/CloseTests, PageEventsCloseTests, and TargetTests suites afterward with no regressions.

Test plan

  • dotnet build clean (net8.0/net10.0/netstandard2.0), no StyleCop warnings
  • dotnet format --verify-no-changes clean
  • Reproduced the hang locally with headful Chrome CDP, confirmed root cause via raw CDP message capture
  • Verified the fix resolves the hang (bounded wait, test still passes) across multiple repro attempts
  • Ran EvaluationTests, PageTests/CloseTests, PageEventsCloseTests, TargetTests — all green

🤖 Generated with Claude Code

https://claude.ai/code/session_01K92eapPm8e7mX7puT4cF4s

Target.closeTarget can race an in-flight client-initiated navigation
(e.g. window.location assigned from inside an evaluate call): Chrome
acknowledges the close with success:true but finishes loading the new
document first, and was observed (headful Chrome for Testing) to then
never send the follow-up Target.detachedFromTarget/targetDestroyed
events at all. Since that wait had no timeout, CloseAsync (and any
caller awaiting it, including test teardown) hung forever.

Every other protocol round-trip in this codebase is already bounded by
ProtocolTimeout, so CloseAsync now is too. The close request was
already accepted by Chrome, so on timeout we log a warning and return
instead of throwing - IsClosed still flips true later if the events
eventually arrive.

Reproduced locally: PageEvaluateTests.ShouldNotThrowAnErrorWhenEvaluationDoesANavigation
hung indefinitely (confirmed via raw CDP traffic capture showing Chrome
going silent on the target right after the close request) roughly half
the time in headful Chrome CDP, matching repeated CI test-host timeouts
on the CHROME-headful-ubuntu-latest-cdp job. With this change the same
race now resolves after ProtocolTimeout instead of hanging.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K92eapPm8e7mX7puT4cF4s
@kblok
kblok merged commit 8e1dbd2 into master Aug 3, 2026
36 of 39 checks passed
@kblok
kblok deleted the fix-page-close-hang-navigation-race branch August 3, 2026 12:58
sondresjolyst pushed a commit to sondresjolyst/garge-api that referenced this pull request Aug 17, 2026
Updated [PuppeteerSharp](https://github.com/hardkoded/puppeteer-sharp)
from 25.4.0 to 25.5.0.

<details>
<summary>Release notes</summary>

_Sourced from [PuppeteerSharp's
releases](https://github.com/hardkoded/puppeteer-sharp/releases)._

## 25.5.0

## What's Changed
* fix: PWA launch returning a page with an empty URL on Windows by
@​kblok in hardkoded/puppeteer-sharp#3529
* fix: bound CdpPage.CloseAsync's wait for target close confirmation by
@​kblok in hardkoded/puppeteer-sharp#3530
* fix: close remaining race in PWA launch where page.Url could still be
empty by @​kblok in
hardkoded/puppeteer-sharp#3531
* fix: reject PWA access when network restrictions are configured
(#​15271) by @​kblok in
hardkoded/puppeteer-sharp#3540
* feat: track dialog status (#​15266) by @​kblok in
hardkoded/puppeteer-sharp#3542
* feat: roll to Firefox 153.0 (#​15258) by @​kblok in
hardkoded/puppeteer-sharp#3537
* fix: roll to Chrome 151.0.7922.71 (#​15272) by @​kblok in
hardkoded/puppeteer-sharp#3535
* fix: disable WebUIOmniboxPopup and WebUIOmniboxAimPopup (#​15278) by
@​kblok in hardkoded/puppeteer-sharp#3538
* fix: roll to Firefox 153.0.1 (#​15269) by @​kblok in
hardkoded/puppeteer-sharp#3536
* fix: do not override user agent when nothing is emulated (#​15274) by
@​kblok in hardkoded/puppeteer-sharp#3539
* fix: forward headers to browserURL discovery and WebSocket connection
(#​15238) by @​kblok in
hardkoded/puppeteer-sharp#3541
* Bump version to 25.5.0 by @​kblok in
hardkoded/puppeteer-sharp#3543


**Full Changelog**:
hardkoded/puppeteer-sharp@v25.4.0...v25.5.0

Commits viewable in [compare
view](hardkoded/puppeteer-sharp@v25.4.0...v25.5.0).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=PuppeteerSharp&package-manager=nuget&previous-version=25.4.0&new-version=25.5.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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