Await the start_url navigation and surface its failures - #338
Closed
yummybomb wants to merge 1 commit into
Closed
Conversation
Page.navigate only resolves once the navigation commits, so the 3s budget on /configure's start_url step expired against origins that were slow to answer. The handler then logged a warning, returned 200, and handed the session over still rendering the pre-navigation page while the navigation committed seconds later. Chrome also reports an unreachable URL by returning an errorText from Page.navigate and committing an error page. DispatchStartURL discarded the result, so that read as a successful navigation. Raise the navigate budget to 15s, which covers a slow origin's DNS, connect and first byte; commit semantics mean it does not have to cover subresource loading. Return Chrome's errorText from DispatchStartURL so callers can tell a committed navigation from a committed error page, and log both failure modes with the elapsed time. A navigation that still fails keeps returning 200 rather than discarding an otherwise healthy browser over a start_url the caller may not control.
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
/configure'sstart_urlstep gave the navigation a 3s budget and treatedanything else as best-effort. Two failure modes fell out of that, both of which
end with the session being handed over on the page it was already showing while
the endpoint reports success:
Page.navigateonly resolvesonce the navigation commits, so the budget has to cover the origin's DNS,
connect and first byte. Against an origin slower than 3s the context expired,
the handler logged a warning and returned 200, and the navigation committed a
few seconds later — after the caller already had the browser.
Page.navigatewith anerrorTextand commits an error page when the URL isunreachable.
DispatchStartURLdiscarded the result, so an unreachablestart_urlreturned no error at all.What changed
startURLDispatchTimeout(3s) becomesstartURLNavigateTimeout(15s). Commitsemantics mean this only has to cover time to first byte, not subresource
loading, so it does not slow down heavy pages.
cdpclient.DispatchStartURLreturns Chrome's navigationerrorTextalongsideits error, letting callers distinguish a committed navigation from a committed
error page.
DispatchStartURLAndWaitignores it deliberately — its pollingloop already re-navigates through
chrome-error://— and the wrapper'sabout:blankreset is unchanged in behavior.start_url dispatch failedwith the elapsed time. Themessage is unchanged so existing log queries keep matching.
treat the instance as tainted and discard an otherwise healthy browser over a
start_urlthe caller may not control.that a failure is logged and still returns 200. Descriptions do not reach
generated code, so
lib/oapi/oapi.gois unchanged;lib/events/category_gen.gowas regenerated and is unchanged.
Verification
Local reproduction driving Chromium 151 under Xvfb with the headful image's flag
set, calling this package's
DispatchStartURLbehind a replica ofstartChromiumAndWait's readiness gate:Page.navigate: read: failed to get reader: context deadline exceeded), handover still renders the previous page, navigation commits at ~6.2sstart_urlnet::ERR_CONNECTION_REFUSEDAlso measured that restored tab targets all exist before the readiness gate
returns (checked up to 40 tabs), so the single
Target.getTargetssnapshot isnot racing target creation.
New unit tests cover a committed navigation, a reported
errorText, and anavigation that does not commit within the budget.
go vetand the full unitsuite pass (22 packages); the e2e suite needs Docker images and was not run.
Notes for review
hung origin cannot stall a configure indefinitely. It sits well inside the
calling activity's timeout.
field, which requires the npm down-converter for codegen. Left out here, so the
failure remains observable only in instance logs.
there before it ships.