⬆️ Bump nock from 13.0.0 to 13.0.3#10
Merged
Merged
Conversation
Bumps [nock](https://github.com/nock/nock) from 13.0.0 to 13.0.3. - [Release notes](https://github.com/nock/nock/releases) - [Changelog](https://github.com/nock/nock/blob/main/CHANGELOG.md) - [Commits](nock/nock@v13.0.0...v13.0.3) Signed-off-by: dependabot[bot] <support@github.com>
dependabot
Bot
added
the
⬆️⬇️ dependencies
wwilsman
changed the title
Manoj-Katta
added a commit
that referenced
this pull request
May 13, 2026
…rectly (PPLT-4214) Bundle of ce:review + PR review fixes for the v143 PlzDedicatedWorker direct-fetch fallback path in network.js. Closes the L755 coverage gap that's been blocking CI since the unstable worker-based Test D was reverted in 2d48f20. Renames: - captureScriptDirectly → captureResourceDirectly. The function captures any allowlisted resource that fell through to direct fetch, not just worker scripts; the old name was misleading. Module-private; no external callers affected. Must-fix (P1/P2 production-risk items): - Cookies: read from network.page.session (full Network domain) instead of the triggering session, since worker sessions throw "Internal error" on Network.getCookies. Defensive try/catch retained. - DIRECT_FETCH_TIMEOUT (5s) caps captureResourceDirectly via Promise.race; prevents idle() from hanging the full networkIdleWaitTimeout (~30s) when a worker host accepts TCP and stalls. (P1 #1) - makeDirectRequest now returns { body, status }; captureResourceDirectly enforces the 25MB MAX_RESOURCE_SIZE guard before saveResource and records the real HTTP status instead of hardcoded 200. Font path call site updated to destructure. (P1 #2) - Direct-fetch gate at _handleLoadingFinished mirrors saveResponseResource's disallowedHostnames-then-allowedHostnames precedence; emits a debug log when fallback is skipped due to hostname gating. (P2 #10 + C12) - Authorization header in makeDirectRequest now requires target origin to match the page's snapshot origin; prevents Basic-auth credential leak to redirected third-party origins. (P2 #11) Reviewer polish: - _handleResponsePaused malformed/oversized branch: Fetch.failRequest runs BEFORE _forgetRequest so Chrome's Fetch state can't leak paused if failRequest throws. Unknown errors trigger a last-resort Fetch.continueResponse to un-pause. Known races (ABORTED_MESSAGE / Invalid InterceptionId) remain silent. (P2 #4) - Unknown errors in _handleResponsePaused failRequest catch and _continueResponse catch now log at warn (was debug) for production observability. (P2 #5 + C8) - _handleResponsePaused: inline comment explaining when the untracked-request branch fires (service-worker-fulfilled responses or cleanup races). (C2) - _handleResponsePaused: url normalization consistent between tracked and untracked paths via normalizeURL on the untracked fallback. (C3) - parseInt now uses explicit radix 10 at both call sites. (C7) - RESPONSE_RECEIVED_TIMEOUT comment notes the cumulative N*2s worst case. (C9) Test: - New deterministic spec "logs gracefully when the direct-fetch fallback fails" exercises captureResourceDirectly's catch path by dropping Network.responseReceived for a CSS asset (no JS execution, no real worker). Closes the L755 coverage gap. (C1 / P1 #3) Intentionally deferred (reviewer comments replied separately): - P2 #15 — no-response branch flip-flop predates this PR (commit ae1d388, 2022-09-21); out of scope. - P2 #6 — sec-ch-ua hardcoded version is one of several stale literals in the makeDirectRequest header block; deferring full audit. - C4 + S4 — DISABLED_FEATURES extraction; existing block-level comment adequate. - C5 — percy.test.js timing fix; reviewer pre-approved deferring. - S2 / S3 — example / reference already present in existing comments. - Favicon Task A — pending separate investigation of snapshot.test.js timing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Manoj-Katta
added a commit
that referenced
this pull request
May 18, 2026
* chore: upgrade Chromium from v126 to v143 for asset discovery Upgrade bundled Chromium from v126.0.6478.184 to v143.0.7499.169 to match the renderer browser version. Update sec-ch-ua header from v123 to v143 in direct font request headers. PPLT-4214 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(core): ignore favicon.ico in test server request log Chrome >=128 new headless auto-requests /favicon.ico on page load, breaking discovery tests that asserted exact request counts. Filter at the helper level so all tests benefit without per-assertion churn. * fix(core): disable site isolation for Chrome 143 new headless Chrome >=128 new headless enforces site isolation via IsolateOrigins and site-per-process, putting cross-origin sub-resources and worker fetches into separate renderer processes. The existing --disable-site-isolation-trials flag only covers opt-in trials and no longer keeps everything in one renderer. Extend the existing --disable-features list with IsolateOrigins and site-per-process so the main page's Fetch.enable / Network.enable listeners continue to see cross-origin and worker traffic, matching the v126 old-headless behavior Percy relies on. Also add HttpsFirstBalancedModeAutoEnable to prevent new headless from blocking HTTP asset discovery with ERR_BLOCKED_BY_CLIENT. * chore(ci): bump cache-key for Chromium v143 upgrade Invalidates actions/cache entries that still contain the old v126 Chromium binary so CI downloads the new v143 revisions fresh. * test(core): align test helpers with Chrome new-headless behavior (PPLT-5285) Two adjustments for Chrome >=128 (new headless) without changing production code: - test server now replies 204 to the auto-fetched /favicon.ico so it doesn't leak into a snapshot's resource list and shift downstream request indices (the previous helper only filtered favicon from the request log; the browser still received the default reply and the resource still got captured). - iframe-srcdoc test regex accepts both serializations: pre-Chrome-128 left "<" / ">" literal inside attribute values, while >=128 entity-escapes them per HTML5 spec. Both are valid HTML. Fixes 4 of the v143 failures: takes additional snapshots after running each execute, runs the execute callback in the correct frame, can execute scripts at various states, can execute scripts that wait for specific states. Cluster A (cross-origin / domain-validation, 14 specs) is being tracked separately — root cause is Chrome's stricter CORS / Opaque Response Blocking; chasing a narrower fix than --disable-web-security before landing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): disable Local Network Access checks for Chrome 143 (PPLT-5284) Chrome 143 enables Local Network Access (LNA) checks by default. When asset discovery serves the root document via Fetch.fulfillRequest (the domSnapshot path), Chrome treats sub-resource requests to local network addresses (*.localhost, 127.0.0.1, RFC 1918) as needing explicit user permission. Headless can't grant the prompt, so the request fails with corsError = LocalNetworkAccessPermissionDenied and the resource is never delivered to Percy's CDP listeners. Add LocalNetworkAccessChecks to the existing --disable-features list so the fulfilled-root flow keeps loading cross-origin local sub-resources, matching v126 behavior. This is narrower than --disable-web-security: it preserves CORS / CORB / ORB enforcement on real cross-origin internet hosts and only relaxes Chrome's loopback / private-network gating, which is exactly what asset discovery needs. Resolves the 12 cross-origin and auto-domain-validation discovery test failures from PR #2187 CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(core): document Chrome 143 upgrade decisions per PR review Address inline review comments on PR #2187: - src/browser.js: split the --disable-features comment into one bullet per feature. Each bullet documents the specific Chrome >=128/143 behavior that broke and why disabling is the right fix - src/install.js: add the procedure for picking per-platform revision numbers (chromiumdash + snapshot index) so future upgrades don't have to re-derive it - test/helpers/server.js: only short-circuit /favicon.ico to 204 when the test hasn't supplied an explicit reply for it. Tests that want favicon-as-asset (none today, but future-proof) keep working via `server.reply('/favicon.ico', ...)` - test/snapshot.test.js: expand the iframe srcdoc-serialization comment with a literal example of each form so the regex's "either form" intent is obvious No behavioral change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): handle Chrome 143 split request lifecycle and malformed Content-Length (PPLT-4214) Two v143 regressions surfaced after the launch-flag fixes landed: 1. `does not capture remote files with content-length NAN greater than 25MB` — Chrome 143 won't terminate the body of a malformed `Content-Length` response, so Network.loadingFinished never fires, the page's <link href="large.css"> blocks the load event, and the navigation times out (3 retries × 30s = 93s). The existing size guard in saveResponseResource is unreachable. 2. `captures requests from workers` — for page-fetched worker scripts, Chrome 143 splits the lifecycle across two CDP sessions: the page session sees Fetch.requestPaused under requestId X, but Network.responseReceived / loadingFinished for the same script fire on the worker session under a fresh requestId Y once the worker target attaches. Percy's #requests entry under X is never cleaned up, idle() blocks indefinitely, retries fail. Confirmed via raw CDP probe (/tmp/worker-probe.mjs) — Chrome itself behaves correctly; the bug is in Percy's requestId-keyed bookkeeping. Fix: enable Fetch response-stage interception (Fetch.continueRequest with interceptResponse:true) and add _handleResponsePaused with four branches: - responseErrorReason set: confirm the failure with Fetch.failRequest so _handleLoadingFailed logs the network error as before. - Content-Length oversized or malformed: forget the request, Fetch.failRequest with 'Aborted' before the body streams. This unsticks the page load for the NaN test. - 3xx redirect: skip — _handleRequest already builds the redirect chain off the next request-stage event with redirectResponse. - Otherwise: schedule a 1s backstop that forgets the request only if Network.loadingFinished / loadingFailed haven't already done so. For normal requests this is a no-op (Network events fire within tens of ms). For worker scripts where loadingFinished never fires on the page session, this is what unblocks idle(). Trade-off: every Fetch-intercepted request now does one extra CDP roundtrip (response-stage pause -> continueResponse). Sub-millisecond per request on localhost websocket. Puppeteer and Playwright run this same dual-stage pattern. Helpers added: inspectContentLength (returns tooLarge / malformed / rawValue) and headersArrayToObject (Fetch event headers come as [{name,value}]). Constant RESPONSE_STAGE_BACKSTOP_MS = 1000. Both target tests pass in 2-3s in isolation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): preserve original errorText for response-stage Fetch errors (PPLT-4214) CI on 56306df surfaced a regression in `logs instrumentation for network errors`: when a server destroys the socket mid-response, Network.loadingFailed used to fire with errorText='net::ERR_EMPTY_RESPONSE' (or similar concrete reason), and _handleLoadingFailed logged asset_load_missing/network_error. After enabling response-stage Fetch interception, the error now surfaces first as Fetch.requestPaused with responseErrorReason set. The previous handler called Fetch.failRequest({errorReason: responseErrorReason}), which forces Chrome to fire Network.loadingFailed with errorText synthesized from that reason. For socket-destroy, responseErrorReason is the generic 'Failed', which collapses to net::ERR_FAILED — the exact errorText the asset_load_missing branch explicitly excludes, so the instrumentation was lost. Switch to Fetch.continueResponse for errored responses. This lets Chrome propagate the original errorText through Network.loadingFailed naturally, restoring the asset_load_missing log. Verified locally: - logs instrumentation for network errors: passes (3s) - captures requests from workers: still passes (4s) - does not capture remote files with content-length NAN: still passes (2s) - captures redirected resources, does not capture event-stream requests, logs failed request errors with a debug loglevel, logs unhandled response errors gracefully: all still pass Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): cover favicon capture path explicitly The other discovery specs opt out of /favicon.ico via the test server's 204 short-circuit so favicon noise doesn't shift their captured[] indices. This spec opts back in by registering an explicit /favicon.ico reply, asserting that Percy correctly captures the favicon when the server actually serves one — the production scenario where a customer's site has a real favicon. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(core): capture body at Fetch response stage instead of using a timer (PPLT-4214) Replaces the 1-second backstop timer with a principled fix: read the response body via Fetch.getResponseBody at response stage, save the resource, and forget the request — so the entire lifecycle is complete before Chrome would dispatch Network.loadingFinished. The backstop approach was a workaround that silently dropped requests where Network.loadingFinished never fired on the page session (Chrome 143's split-session worker scripts being the motivating case). The worker test passed because the orphan was deleted, but the worker script itself was never captured as a Percy resource — a real fidelity gap, even if invisible to visual diffs. Approach 2 captures the resource using the data Chrome gives us at response stage: - Errored response: continue, let Network.loadingFailed propagate the original errorText to _handleLoadingFailed (unchanged from before). - Oversized / malformed Content-Length: log skip, forget, Fetch.failRequest before body streams (unchanged from before). - Redirect (3xx): continue; _handleRequest builds the chain on the next request stage event. - Streaming (text/event-stream): continue without reading body — SSE bodies never complete and Fetch.getResponseBody would hang. - Normal: read body via Fetch.getResponseBody, build a synthetic response object, run saveResponseResource, forget the request. Network.loadingFinished still fires later for normal page-session requests but finds nothing in #requests and exits early (the existing handler already guards for this case). Other changes in this commit: - Drop RESPONSE_STAGE_BACKSTOP_MS constant and the setTimeout block. - Trim the multi-paragraph block comments in _handleResponsePaused, the helpers, and the dispatch in _handleRequestPaused down to one-liners. - Update `logs unhandled response errors gracefully` test to mock Fetch.getResponseBody (the body-read path is now Fetch-based). - Make `captures favicon when the server provides one` deterministic by adding <link rel="icon"> to the test DOM — the previous version relied on Chrome's auto-fetch which has non-deterministic timing on Windows CI. Net diff vs. backstop approach: -27 lines in network.js, +5 lines in discovery.test.js. All affected tests pass locally: - captures favicon when the server provides one (2s) - does not capture remote files with content-length NAN greater than 25MB (2s) - captures requests from workers (2s) - captures redirected resources (3s) - does not capture event-stream requests (3s) - logs failed request errors with a debug loglevel (2s) - logs instrumentation for network errors (2s) - logs unhandled response errors gracefully (2s) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): cover saveResponseResource catch and auto-fetched favicon path (PPLT-4214) Drop the dead Content-Length size check from saveResponseResource — the oversized/malformed header path is now rejected earlier in _handleResponsePaused via Fetch.failRequest, so this guard never runs. The body.length check below still covers cached responses whose headers lie about size. Add two discovery specs to keep coverage at 100%: - "logs gracefully when direct font request fails" exercises the catch in saveResponseResource by serving the font as application/octet-stream (forces makeDirectRequest) and returning 400 on the direct fetch (makes makeRequest throw without retry). - "captures auto-fetched favicon when the page does not declare one" validates that Percy captures Chrome's implicit /favicon.ico fetch. Bumps networkIdleTimeout to 1000ms so the auto-fetch lands before idle declares the network done — Chrome's auto-fetch timing varies by platform and was previously flaky on Windows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): guard Network constructor against null session.browser (PPLT-4214) The Network constructor reads page.session.browser.version.userAgent. If the session crashes mid-construction (close() sets browser to null), the read throws a TypeError that masks the real "Session crashed!" error. Add optional chaining so userAgent stays unset on the race; the next CDP call in Network.watch surfaces the real session-closed error as expected. Fixes Snapshot > handles page crashes on Linux CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): cover untracked-request branch and simplify error message read (PPLT-4214) - Add discovery spec that emits a response-stage Fetch.requestPaused for a request that was never tracked at the request stage. Exercises the defensive null-request branches in _handleResponsePaused (line 236, 254, 278-282). - Simplify error?.message ?? '' to error && error.message ? error.message : '' in browser.js handleError so all branches are reachable from existing launch-failure tests. Behaviorally identical for any realistic input. - Update install.js comment to point to the browseable index.html for Chromium snapshots. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): stabilize two timing-fragile tests under suite load (PPLT-4214) - Replace the 500ms polling loop in 'returns the same promise for concurrent validation requests to the same hostname' with a deterministic Promise that the validation mock resolves on its first call. The hardcoded ceiling was too tight for a loaded CI suite and the v143 response-stage refactor's per-request overhead pushed the test over the edge consistently. - Attach a no-op catch on the sync-job promise in 'handleSyncJob should handle promise reject' so the rejection isn't reported as unhandled in the race window between snapshot returning and handleSyncJob attaching its own handler. expectAsync still observes the rejection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): attach catch handler synchronously on sync-snapshot promise (PPLT-4214) The previous fix attached .catch() after percy.snapshot returned, but percy.js:495 creates and assigns the upload promise synchronously inside the snapshot call, and the upload can reject before the test's next line runs. Use a setter on the promise property so the no-op catch is attached the instant percy assigns the promise — eliminating the race window. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): bump waitForTimeout to give failing upload reliable headroom (PPLT-4214) The 'stops accepting snapshots when an in-progress build fails' spec relied on a 100ms waitForTimeout to keep snapshot-2 in discovery while snapshot-1's upload failed. The v143 response-stage refactor makes the suite ~50% slower under CI load, so 100ms is no longer enough — both uploads were getting through. Bump to 1000ms. The race window the test exercises is the same; we just give the failing upload more time to be observed before the second snapshot leaves discovery. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): cover oversized/malformed branch for untracked requests (PPLT-4214) Adds a discovery spec that emits a response-stage Fetch.requestPaused with a malformed Content-Length header for a request that was never tracked at the request stage. This exercises the if (request) false branch inside _handleResponsePaused's oversized/malformed handler — line 254 was flagged as a coverage gap on CI after the response-stage refactor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): timeout responseReceived wait so worker scripts don't leak (PPLT-4214) Root cause: Chrome 143 doesn't fire Network.responseReceived for worker scripts — only requestWillBeSent and loadingFinished. Percy's _handleLoadingFinished awaited responseReceived unconditionally, blocking forever and leaking the request in #requests so idle() never returned. Fix: - Cap the responseReceived wait in _handleLoadingFinished at 2s. When responseReceived never fires (worker scripts), forget the request without saving rather than blocking. - Simplify _handleResponsePaused: keep response-stage interception only for the malformed/oversized Content-Length abort. Stop reading the body at response stage — let it be captured later via the v126 path (Network.getResponseBody after loadingFinished). The body-read at response stage was added in bd44f96 as a workaround for the leak above; the proper fix is the timeout. Test fix: 'logs unhandled response errors gracefully' now mocks Network.getResponseBody (the actual call site after this change), not Fetch.getResponseBody. Verified locally: - captures requests from workers: 5s (was 90s timeout) - does not capture remote files with content-length NAN: 4s - 3 content-length casing variants: pass - logs instrumentation for network errors / 5xx / empty: pass - captures redirected resources: pass - does not capture event-stream requests: pass - captures stylesheet initiated fonts: pass - infers mime type when CDP returns text/plain: pass - captures responsive assets / responsive srcset+mediaquery: pass - handles page crashes: pass - captures with valid auth credentials + 3 font-auth variants: pass - handleSyncJob / in-progress build fails / concurrent validation: pass Auth test 'does not capture without auth credentials' still slow at 1m34s — that's a fundamental Chrome 143 behavior (load event doesn't fire after auth cancel), not fixable in Percy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): drop trailing blank line that broke yarn lint (PPLT-4214) CI flagged no-multiple-empty-lines at discovery.test.js:3817 — leftover from removing the failing browser.js coverage test attempt earlier. Verified yarn lint clean locally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): make browser-launch failure path testable across Chrome versions (PPLT-4214) Pass a real Error from handleExitClose so the rejection has one shape on every code path; switch the failing-launch test to --version, which makes Chrome exit deterministically without depending on its arg-parsing quirks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): make worker-script responseReceived timeout observable (PPLT-4214) Lift the 2s cap to a named constant, clear the timer on the responseReceived-wins path, and log a debug line when the cap fires so a regression of the v143 worker-script hang would surface in CI rather than silently dropping the resource. Asserts the log in the existing worker test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): use platform-specific early-exit arg in launch-failure test (PPLT-4214) Chrome 143 with --version exits cleanly on Linux/macOS but keeps running on Windows. Use --remote-debugging-port=null on Windows (still exits there, per original v126 behavior) and --version elsewhere so each runner exercises the handleExitClose path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): capture v143 worker scripts via direct HTTP fallback (PPLT-4214) Chrome 143+ fetches dedicated worker scripts in the browser process (PlzDedicatedWorker) and never surfaces the response on any CDP session, so the bytes can't be retrieved via Network.getResponseBody. For enableJavaScript:true snapshots the cloud renderer needs the script to construct the worker. Fall back to a direct HTTP fetch (same pattern makeDirectRequest already uses for fonts) when the responseReceived timeout fires for a Script-type request, and assert worker.js is captured in the existing worker test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): widen v143 worker-script fallback for PlzDedicatedWorker (PPLT-4214) Two issues left worker.js missing from the snapshot resource list on Linux + Windows CI even after the direct-HTTP fallback in 9adeecd: 1. Chrome 143 PlzDedicatedWorker reports the worker script's `resourceType` as 'Other', not 'Script'. The strict `request.type === 'Script'` gate skipped the fallback. Replace it with an `allowedHostnames` filter so the gate matches what the snapshot would have captured anyway, regardless of how Chrome classifies the request type. 2. `Network.loadingFinished` for the worker script fires on the *worker* CDP session in v143. `Network.getCookies` on the worker session throws "Internal error", which made `makeDirectRequest` throw silently inside the fallback. Wrap the cookies call in try/catch and proceed with an empty cookies array. Page-session callers (fonts) are unaffected. Also derive the resource mimetype from the URL via `mime.lookup()` instead of hardcoding `application/javascript`, so any timed-out allowed-hostname resource is saved with the correct mimetype. Verified locally: focused worker test passes in 5s. All discovery and network specs pass on macOS Node 25 (the 27 unrelated failures in the local full run are Node 25-only mock issues in install/doctor tests; CI runs Node 14 where they pass). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(core): remove unreachable no-response branch in saveResponseResource (PPLT-4214) The `if (!response)` block was dead code: saveResponseResource is only called from _handleLoadingFinished after the `if (!request.response)` early-return guards against a null response, so the inner check could never fire. The pre-existing `/* istanbul ignore if */` was masking the unreachability; deleting it surfaces the cleaner control flow. CI flagged this line as the only remaining coverage gap on Linux after the worker-test fix in 37f5176. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert "refactor(core): remove unreachable no-response branch in saveResponseResource (PPLT-4214)" This reverts commit ec13879. * test(core): cover network.js race-catch branches; drop istanbul annotations (PPLT-4214) Drops the two `/* istanbul ignore next: race with abort/close */` comments in _handleResponsePaused (Fetch.failRequest catch) and _continueResponse (Fetch.continueResponse catch), and backs each branch with a real test. New specs under "Discovery / with resource errors": - logs gracefully when Fetch.failRequest fails during malformed-CL abort - silently swallows Fetch.continueResponse benign races (ABORTED_MESSAGE) - logs gracefully when Fetch.continueResponse fails with unexpected error - logs gracefully when direct worker-script fetch fails Together with the existing untracked-request specs, every branch in the two catch blocks plus the captureScriptDirectly catch is now exercised without `istanbul ignore`, addressing the post-merge coverage gap on network.js (previously L317, L755). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): drop CI-unstable direct worker-script direct-fetch spec (PPLT-4214) The companion to c20c581's network.js cleanup, "logs gracefully when direct worker-script fetch fails", passed locally on macOS Node 25 but failed on CI Linux Node 14 because its worker scaffolding (real `new Worker()` + `waitForSelector: '.done'`) interacted badly with PlzDedicatedWorker when /worker.js had to return 200 on the first hit and 400 on the second. The worker initialization never completed, captureScriptDirectly was never reached, the expected debug log never appeared. The other three new specs from c20c581 (failRequest unexpected error, continueResponse ABORTED_MESSAGE silent return, continueResponse unexpected error) all pass on CI Linux and are kept. Network.js:755 returns to "uncovered" status; a deterministic test for that catch (no JS execution, no real worker) will land in a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): harden v143 direct-fetch fallback + rename captureScriptDirectly (PPLT-4214) Bundle of ce:review + PR review fixes for the v143 PlzDedicatedWorker direct-fetch fallback path in network.js. Closes the L755 coverage gap that's been blocking CI since the unstable worker-based Test D was reverted in 2d48f20. Renames: - captureScriptDirectly → captureResourceDirectly. The function captures any allowlisted resource that fell through to direct fetch, not just worker scripts; the old name was misleading. Module-private; no external callers affected. Must-fix (P1/P2 production-risk items): - Cookies: read from network.page.session (full Network domain) instead of the triggering session, since worker sessions throw "Internal error" on Network.getCookies. Defensive try/catch retained. - DIRECT_FETCH_TIMEOUT (5s) caps captureResourceDirectly via Promise.race; prevents idle() from hanging the full networkIdleWaitTimeout (~30s) when a worker host accepts TCP and stalls. (P1 #1) - makeDirectRequest now returns { body, status }; captureResourceDirectly enforces the 25MB MAX_RESOURCE_SIZE guard before saveResource and records the real HTTP status instead of hardcoded 200. Font path call site updated to destructure. (P1 #2) - Direct-fetch gate at _handleLoadingFinished mirrors saveResponseResource's disallowedHostnames-then-allowedHostnames precedence; emits a debug log when fallback is skipped due to hostname gating. (P2 #10 + C12) - Authorization header in makeDirectRequest now requires target origin to match the page's snapshot origin; prevents Basic-auth credential leak to redirected third-party origins. (P2 #11) Reviewer polish: - _handleResponsePaused malformed/oversized branch: Fetch.failRequest runs BEFORE _forgetRequest so Chrome's Fetch state can't leak paused if failRequest throws. Unknown errors trigger a last-resort Fetch.continueResponse to un-pause. Known races (ABORTED_MESSAGE / Invalid InterceptionId) remain silent. (P2 #4) - Unknown errors in _handleResponsePaused failRequest catch and _continueResponse catch now log at warn (was debug) for production observability. (P2 #5 + C8) - _handleResponsePaused: inline comment explaining when the untracked-request branch fires (service-worker-fulfilled responses or cleanup races). (C2) - _handleResponsePaused: url normalization consistent between tracked and untracked paths via normalizeURL on the untracked fallback. (C3) - parseInt now uses explicit radix 10 at both call sites. (C7) - RESPONSE_RECEIVED_TIMEOUT comment notes the cumulative N*2s worst case. (C9) Test: - New deterministic spec "logs gracefully when the direct-fetch fallback fails" exercises captureResourceDirectly's catch path by dropping Network.responseReceived for a CSS asset (no JS execution, no real worker). Closes the L755 coverage gap. (C1 / P1 #3) Intentionally deferred (reviewer comments replied separately): - P2 #15 — no-response branch flip-flop predates this PR (commit ae1d388, 2022-09-21); out of scope. - P2 #6 — sec-ch-ua hardcoded version is one of several stale literals in the makeDirectRequest header block; deferring full audit. - C4 + S4 — DISABLED_FEATURES extraction; existing block-level comment adequate. - C5 — percy.test.js timing fix; reviewer pre-approved deferring. - S2 / S3 — example / reference already present in existing comments. - Favicon Task A — pending separate investigation of snapshot.test.js timing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): revert log.warn promotion in CDP-race catches; satisfy lint Two related CI fixes for the previous commit: 1. Revert log.warn -> log.debug in _handleResponsePaused's failRequest catch and _continueResponse's catch. The promotion was too aggressive: it treated Session closed / Target closed (lifecycle teardown races) as unknown errors and fired a warn line, breaking exact-stderr-equality assertions in tests like snapshot.test.js's `handles the page closing early` (which intentionally tears down the browser mid-fetch) and producing noise on routine percy.stop() teardown for customers. The other parts of the earlier reorder commit are kept: - Fetch.failRequest BEFORE _forgetRequest (lifecycle correctness) - Last-resort Fetch.continueResponse on unknown failRequest errors - Distinction between known-race silent return vs unknown-error log Production observability for stuck-snapshot incidents will be addressed in a follow-up via instrumentation telemetry rather than user-facing logs. 2. Convert the targetDOM template literal in `logs gracefully when the direct-fetch fallback fails` to a single-quoted string. The template literal had no interpolation and tripped ESLint's quotes rule (single-quote preference). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): close network.js coverage gap; mark unreachable else (PPLT-4214) Covers the two integration-testable uncovered lines in `network.js`: - `Network.getCookies` failure catch in `makeDirectRequest` - 25 MB direct-fetch body-size guard in `captureResourceDirectly` The third uncovered branch (the hostname-not-allowed skip in `_handleLoadingFinished`) only fires for PlzDedicatedWorker requests whose worker-script fetch bypasses `Fetch.requestPaused`. Cross-origin assets loaded via the document session still flow through `sendResponseResource`, so the integration test harness can't reliably exercise that branch. Annotated with `/* istanbul ignore else */` plus a justification comment so the gate stays at 100%. * style(core): drop padded blank line at end of resource-errors describe * refactor(core): extract direct-fetch helpers for clean coverage (PPLT-4214) Extracts two pure helpers from `makeDirectRequest` so the previously uncovered branches become unit-testable without `/* istanbul ignore */` annotations: - `pickCookieSession(network, session)` — picks the page's full Network domain when set, falls back to the request's own session otherwise. - `shouldAttachAuth(authorization, requestUrl, snapshotUrl)` — re-enforces the same-origin rule for Node-side Basic auth (cross-origin or malformed URLs return false defensively). `makeDirectRequest` now reads as straight-line composition: pick session → fetch cookies → assemble headers → `if (shouldAttachAuth(...))` attach. Tests: - 7 unit tests in `test/unit/network.test.js` cover every branch of both helpers (page-set, page-undefined, page-no-session, no-auth, empty-auth, same-origin, cross-origin, malformed URLs). - 1 integration test in `test/discovery.test.js` covers the mime-lookup fallback to `application/javascript` for direct-fetch URLs with no recognizable extension. Behavior is byte-identical to the previous inline implementation. * refactor(core): extract raceWithTimeout helper for clean coverage (PPLT-4214) Same pattern as pickCookieSession / shouldAttachAuth: lifts the Promise.race + setTimeout dance out of captureResourceDirectly into an exported pure helper so its branches become unit-testable. The setTimeout callback (only fires on a 5s+ direct-fetch stall) was the last remaining function-coverage gap in network.js — no realistic integration test stalls a fetch long enough to trip it. With the helper unit-tested via a 10ms timeout, the gap closes without an istanbul-ignore marker. Side benefit: the outer try/finally in captureResourceDirectly no longer needs to clear the timer (the helper does it internally), which slightly simplifies the function. Tests: 3 unit tests in test/unit/network.test.js cover all 3 outcomes of raceWithTimeout (resolves-before-timeout, rejects-on-timeout, inner-rejection-propagates). * fix(core): honor server Content-Type on direct-fetch; safer mime fallback (PPLT-4214) Previously, captureResourceDirectly derived mimetype from URL extension alone, falling back to 'application/javascript' for extensionless paths. That default is the worst wrong guess — extensionless REST endpoints (/api/data, worker registries) often serve JSON/HTML/binary, and treating those as JS produces confusing renderer parse errors. New precedence in captureResourceDirectly: 1. HTTP Content-Type response header from the direct fetch 2. mime.lookup() on the URL extension 3. application/octet-stream (safe binary default, never JS) makeDirectRequest's makeRequest callback now also returns `headers` so captureResourceDirectly can read the server's Content-Type. The font path in saveResponseResource continues to destructure { body } only and is unaffected. Test: the unknown-extension integration spec now asserts the server's declared 'application/octet-stream' is preserved (was previously asserting the JS fallback that this commit removes). * refactor(core): extract DISABLED_FEATURES + resolveDirectFetchMime helpers (PPLT-4214) Two minimal changes: 1. browser.js — pull the inline `--disable-features=` string into a `DISABLED_FEATURES` array with a per-entry comment for each flag. Typos in feature names are now diff-visible; security-boundary flags (IsolateOrigins, site-per-process) are explicitly tagged as headless-only. No behavior change. 2. network.js — extract the direct-fetch mimetype resolution into an exported `resolveDirectFetchMime(responseHeaders, urlForLookup)` helper. Same playbook as pickCookieSession / shouldAttachAuth / raceWithTimeout: pure function, all branches unit-testable without integration test gymnastics. Closes the last `network.js` branch coverage gap (line 822's optional-chain F-branch). 3. network.js — RESPONSE_RECEIVED_TIMEOUT comment updated to mention PERCY_NETWORK_IDLE_WAIT_TIMEOUT as the cumulative safety valve. Tests: 3 unit tests in test/unit/network.test.js cover all branches of resolveDirectFetchMime (server-MIME bare, server-MIME with charset parameter, URL-extension fallback, application/octet-stream fallback, empty content-type). --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Bumps nock from 13.0.0 to 13.0.3.
Release notes
Sourced from nock's releases.
Commits
ac6ebbbfix: data comparison with undefined object values (#2049)a0a7fdfrefactor(test): Mocha DSL for default reply headers (#2015)bd1bd71test(delay): ease assertions on delayed time. (#2047)76bcff8chore(deps): bump lodash from 4.17.14 to 4.17.19 (#2044)749ec7fchore(deps): bump npm from 6.14.5 to 6.14.6 (#2040)3b24821fix(intercept): mark Interceptors consumed immediately (#2033)9be36cfdocs: fix wrong typing (#2036)cf3f858chore(deps-dev): bump eslint-plugin-mocha from 6.3.0 to 7.0.1 (#2034)e432ac5fix: allow Content-Type request introspection when header is an array (#2010)0a03796chore(deps-dev): bump mocha from 7.2.0 to 8.0.1 (#2032)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 commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot ignore this major versionwill 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 versionwill 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 dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)