Repository navigation
fix(engine): look a host up again after a failed lookup instead of refusing it for the whole page - #7751
Conversation
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 0 important · 0 nits · head 4da32b17
The fix itself is right: only a failed lookup is forgotten, and requests already waiting share it. The one blocker is the red Comments run (inline).
Scope
Reviewed the delta on #7748 (a6037e14..4da32b17, 2 files). This PR is stacked, so the CI workflow (lint, typecheck, tests) has not run on it. Locally against chrome-headless-shell 157, both test files pass 10/10, three runs in a row. With the cache-drop line removed, the new test fails (1 of 1), which matches the fails-without-the-fix line.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
9d5edce to
b1d3b1d
Compare
Edit accuracy: accurate 267 (base branch 267), smooth 98 of thoseThe gate passes. Quarantined, measured but not gated (8)
Unstable (1)
|
There was a problem hiding this comment.
Verdict: APPROVE — 1 resolved, 0 open, 0 new · 4da32b1 → b1d3b1d
Receipt
The rebase onto main after #7748 is byte-identical to 9d5edce2 in both files. forgetFailedLookup keeps the same behaviour. All CI on b1d3b1dc is green: CI, Comments, CodeQL, regression and Windows render verification. Locally, both test files pass 10/10, and tsc, oxlint and oxfmt --check are clean.
What
With
PRODUCER_RESTRICT_PAGE_NETWORK=true, a host whose name lookup fails is refused for that request and lookedup again on the next one. Before, the failure was cached as a refusal for the rest of the page.
Why
Stacked on #7748, from its review. One transient DNS failure on a public host (a web font, a remote video, a CDN
script) would otherwise block every later request to that host for the rest of a long render, while Chrome on its
own would have retried. The asset would go missing from the output without the render failing.
Related work
Refs #7748 (base of this PR; GitHub retargets it to
mainwhen #7748 merges).How
pageNetworkPolicy.ts: the host check returns"public" | "non-public" | "unresolved"instead of a boolean.Answers about resolved addresses stay cached for the page. An
"unresolved"answer is dropped from the cacheonce it settles, so the next request to that host asks again. A request already waiting on the same lookup
shares its answer.
Test plan
New test in
pageNetworkPolicy.browser.test.ts(real headless Chrome): a name whose first lookup fails isrefused once, then allowed when the next lookup answers with a public address. The second request reaches the
target server; the first does not.
Fails without the fix: with the cache-drop line removed, the new test fails (1 of 1). With
restrictPageNetworkreplaced by a no-op, the file fails 6 of 8.
Both test files pass 10 of 10, three runs in a row on Linux;
tsc,oxlintandoxfmt --checkpass.Not exercised: Windows and macOS hosts; a real DNS failure (the test injects one through the resolver).