From 4c9408176712475d0fe20bdd7e31807c3fe97060 Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Fri, 17 Apr 2026 11:19:58 +0530 Subject: [PATCH 01/37] 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) --- packages/core/src/install.js | 12 ++++++------ packages/core/src/network.js | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/core/src/install.js b/packages/core/src/install.js index ac87c89bd..5bb788352 100644 --- a/packages/core/src/install.js +++ b/packages/core/src/install.js @@ -175,13 +175,13 @@ export function chromium({ }); } -// default chromium revisions corresponds to v126.0.6478.184 +// default chromium revisions corresponds to v143.0.7499.169 chromium.revisions = { - linux: '1300309', - win64: '1300297', - win32: '1300295', - darwin: '1300293', - darwinArm: '1300314' + linux: '1536366', + win64: '1536376', + win32: '1536377', + darwin: '1536380', + darwinArm: '1536376' }; // export the namespace by default diff --git a/packages/core/src/network.js b/packages/core/src/network.js index dba250149..e19d6e3b4 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -558,7 +558,7 @@ async function makeDirectRequest(network, request, session) { 'sec-fetch-site': 'same-origin', 'sec-fetch-mode': 'cors', 'sec-fetch-dest': 'font', - 'sec-ch-ua': '"Chromium";v="123", "Google Chrome";v="123", "Not?A_Brand";v="99"', + 'sec-ch-ua': '"Chromium";v="143", "Google Chrome";v="143", "Not?A_Brand";v="99"', 'sec-ch-ua-mobile': '?0', 'sec-ch-ua-platform': '"macOS"', 'sec-fetch-user': '?1', From c3b1f1636c3b323efc25ff47ac0890ab354d0603 Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Sun, 19 Apr 2026 12:46:47 +0530 Subject: [PATCH 02/37] 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. --- packages/core/test/helpers/server.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/core/test/helpers/server.js b/packages/core/test/helpers/server.js index ad8d0bdf0..0a1cfef9f 100644 --- a/packages/core/test/helpers/server.js +++ b/packages/core/test/helpers/server.js @@ -28,7 +28,11 @@ export function createTestServer({ default: defaultReply, ...replies }, port = 8 server.route(async (req, res, next) => { let pathname = req.url.pathname; if (req.url.search) pathname += req.url.search; - server.requests.push(req.body ? [pathname, req.body, req.headers] : [pathname, req.headers]); + // skip favicon.ico — Chrome >=128 new headless auto-requests it, but it's + // not part of any test's asset graph. Filter here to avoid per-test churn. + if (req.url.pathname !== '/favicon.ico') { + server.requests.push(req.body ? [pathname, req.body, req.headers] : [pathname, req.headers]); + } let reply = replies[pathname] || defaultReply; return reply ? await reply(req, res) : next(); }); From 319744ff4a532e533400de16b1fe9b55765e09e0 Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Sun, 19 Apr 2026 13:20:51 +0530 Subject: [PATCH 03/37] 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. --- packages/core/src/browser.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/core/src/browser.js b/packages/core/src/browser.js index 74f3eade3..c84731727 100644 --- a/packages/core/src/browser.js +++ b/packages/core/src/browser.js @@ -20,8 +20,10 @@ export class Browser extends EventEmitter { #lastid = 0; args = [ - // disable the translate popup and optimization downloads - '--disable-features=Translate,OptimizationGuideModelDownloading', + // disable the translate popup, optimization downloads, baseline site + // isolation (so cross-origin sub-resources and worker fetches stay in + // the main renderer for interception), and HTTPS-first navigation blocking + '--disable-features=Translate,OptimizationGuideModelDownloading,IsolateOrigins,site-per-process,HttpsFirstBalancedModeAutoEnable', // disable several subsystems which run network requests in the background '--disable-background-networking', // disable task throttling of timer tasks from background pages From 1b7f936542b7b9745f4bb6a940d99a3daebea89e Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Mon, 20 Apr 2026 14:43:11 +0530 Subject: [PATCH 04/37] 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. --- .github/.cache-key | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/.cache-key b/.github/.cache-key index b25300ecf..d05221f2f 100644 --- a/.github/.cache-key +++ b/.github/.cache-key @@ -6,5 +6,5 @@ ; ; / \ _____________/_ __ \_____________ -Times we have broken CI: 3 +Times we have broken CI: 4 Times Windows has broken CI: 99+ From f64f1f8a7283d3dc8cf1e5e0d8059e5ba42a83c6 Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Mon, 27 Apr 2026 15:38:37 +0530 Subject: [PATCH 05/37] test(core): align test helpers with Chrome new-headless behavior (PPLT-5285) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packages/core/test/helpers/server.js | 10 ++++++---- packages/core/test/snapshot.test.js | 5 ++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/core/test/helpers/server.js b/packages/core/test/helpers/server.js index 0a1cfef9f..311df5ff8 100644 --- a/packages/core/test/helpers/server.js +++ b/packages/core/test/helpers/server.js @@ -28,11 +28,13 @@ export function createTestServer({ default: defaultReply, ...replies }, port = 8 server.route(async (req, res, next) => { let pathname = req.url.pathname; if (req.url.search) pathname += req.url.search; - // skip favicon.ico — Chrome >=128 new headless auto-requests it, but it's - // not part of any test's asset graph. Filter here to avoid per-test churn. - if (req.url.pathname !== '/favicon.ico') { - server.requests.push(req.body ? [pathname, req.body, req.headers] : [pathname, req.headers]); + // Chrome >=128 new headless auto-requests /favicon.ico for every navigation. + // Reply 204 so the browser doesn't capture it as a snapshot resource, and + // skip it from the requests log so per-test request assertions stay stable. + if (req.url.pathname === '/favicon.ico') { + return res.writeHead(204).end(); } + server.requests.push(req.body ? [pathname, req.body, req.headers] : [pathname, req.headers]); let reply = replies[pathname] || defaultReply; return reply ? await reply(req, res) : next(); }); diff --git a/packages/core/test/snapshot.test.js b/packages/core/test/snapshot.test.js index b8dbebad2..138fff4ed 100644 --- a/packages/core/test/snapshot.test.js +++ b/packages/core/test/snapshot.test.js @@ -1535,10 +1535,13 @@ describe('Snapshot', () => { await percy.idle(); + // srcdoc HTML attribute serialization: pre-Chrome-128 left `<` `>` literal + // inside attribute values; Chrome >=128 entity-escapes them per HTML5 spec. + // Accept either form. expect(Buffer.from(( api.requests['/builds/123/resources'][0] .body.data.attributes['base64-content'] - ), 'base64').toString()).toMatch(/Foo<\/p>/); + ), 'base64').toString()).toMatch(/Foo<\/p>|<p>Foo<\/p>)/); }); it('errors if execute cannot be serialized', async () => { From 5ba42dc99e8b81491fcf97a0e064499aff048ce0 Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Mon, 27 Apr 2026 16:09:18 +0530 Subject: [PATCH 06/37] 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) --- packages/core/src/browser.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/core/src/browser.js b/packages/core/src/browser.js index c84731727..20fed215d 100644 --- a/packages/core/src/browser.js +++ b/packages/core/src/browser.js @@ -20,10 +20,15 @@ export class Browser extends EventEmitter { #lastid = 0; args = [ - // disable the translate popup, optimization downloads, baseline site + // disable: translate popup, optimization downloads, baseline site // isolation (so cross-origin sub-resources and worker fetches stay in - // the main renderer for interception), and HTTPS-first navigation blocking - '--disable-features=Translate,OptimizationGuideModelDownloading,IsolateOrigins,site-per-process,HttpsFirstBalancedModeAutoEnable', + // the main renderer for interception), HTTPS-first navigation blocking, + // and Local Network Access permission checks (Chrome 143+ blocks + // sub-resource requests to *.localhost / 127.0.0.1 / RFC1918 with + // `LocalNetworkAccessPermissionDenied` when the document was served + // via Fetch.fulfillRequest from asset discovery — there is no permission + // prompt available in headless to grant the access). + '--disable-features=Translate,OptimizationGuideModelDownloading,IsolateOrigins,site-per-process,HttpsFirstBalancedModeAutoEnable,LocalNetworkAccessChecks', // disable several subsystems which run network requests in the background '--disable-background-networking', // disable task throttling of timer tasks from background pages From 4f02ad63fa48f2b192f2b0a05ecefbf2fb8b2fe1 Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Mon, 27 Apr 2026 17:42:45 +0530 Subject: [PATCH 07/37] 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) --- packages/core/src/browser.js | 12 ++++-------- packages/core/src/install.js | 3 ++- packages/core/test/helpers/server.js | 10 +++++----- packages/core/test/snapshot.test.js | 6 +++--- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/packages/core/src/browser.js b/packages/core/src/browser.js index 20fed215d..cbb110f53 100644 --- a/packages/core/src/browser.js +++ b/packages/core/src/browser.js @@ -20,14 +20,10 @@ export class Browser extends EventEmitter { #lastid = 0; args = [ - // disable: translate popup, optimization downloads, baseline site - // isolation (so cross-origin sub-resources and worker fetches stay in - // the main renderer for interception), HTTPS-first navigation blocking, - // and Local Network Access permission checks (Chrome 143+ blocks - // sub-resource requests to *.localhost / 127.0.0.1 / RFC1918 with - // `LocalNetworkAccessPermissionDenied` when the document was served - // via Fetch.fulfillRequest from asset discovery — there is no permission - // prompt available in headless to grant the access). + // Disable Chrome features that break asset discovery in v143 new-headless: + // site/origin isolation (so cross-origin events stay on the page session), + // HTTPS-first auto-upgrade (would block HTTP discovery), and Local Network + // Access permission checks (would block sub-resources to localhost/RFC1918). '--disable-features=Translate,OptimizationGuideModelDownloading,IsolateOrigins,site-per-process,HttpsFirstBalancedModeAutoEnable,LocalNetworkAccessChecks', // disable several subsystems which run network requests in the background '--disable-background-networking', diff --git a/packages/core/src/install.js b/packages/core/src/install.js index 5bb788352..32cc608b3 100644 --- a/packages/core/src/install.js +++ b/packages/core/src/install.js @@ -175,7 +175,8 @@ export function chromium({ }); } -// default chromium revisions corresponds to v143.0.7499.169 +// Chrome 143.0.7499.169 (base position 1536371) — closest per-platform +// revision from https://commondatastorage.googleapis.com/chromium-browser-snapshots/ chromium.revisions = { linux: '1536366', win64: '1536376', diff --git a/packages/core/test/helpers/server.js b/packages/core/test/helpers/server.js index 311df5ff8..7dc1aac5a 100644 --- a/packages/core/test/helpers/server.js +++ b/packages/core/test/helpers/server.js @@ -28,14 +28,14 @@ export function createTestServer({ default: defaultReply, ...replies }, port = 8 server.route(async (req, res, next) => { let pathname = req.url.pathname; if (req.url.search) pathname += req.url.search; - // Chrome >=128 new headless auto-requests /favicon.ico for every navigation. - // Reply 204 so the browser doesn't capture it as a snapshot resource, and - // skip it from the requests log so per-test request assertions stay stable. - if (req.url.pathname === '/favicon.ico') { + let reply = replies[pathname] || defaultReply; + // Chrome >=128 auto-fetches /favicon.ico on every navigation; reply 204 + // by default so it doesn't pollute snapshot resources. Tests can still + // override via `server.reply('/favicon.ico', ...)`. + if (req.url.pathname === '/favicon.ico' && !replies['/favicon.ico']) { return res.writeHead(204).end(); } server.requests.push(req.body ? [pathname, req.body, req.headers] : [pathname, req.headers]); - let reply = replies[pathname] || defaultReply; return reply ? await reply(req, res) : next(); }); diff --git a/packages/core/test/snapshot.test.js b/packages/core/test/snapshot.test.js index 138fff4ed..270def333 100644 --- a/packages/core/test/snapshot.test.js +++ b/packages/core/test/snapshot.test.js @@ -1535,9 +1535,9 @@ describe('Snapshot', () => { await percy.idle(); - // srcdoc HTML attribute serialization: pre-Chrome-128 left `<` `>` literal - // inside attribute values; Chrome >=128 entity-escapes them per HTML5 spec. - // Accept either form. + // Chrome >=128 entity-escapes `<`/`>` inside attribute values per HTML5 + // spec; pre-128 left them literal. Accept either form (e.g. + // `srcdoc="

Foo

"` or `srcdoc="<p>Foo</p>"`). expect(Buffer.from(( api.requests['/builds/123/resources'][0] .body.data.attributes['base64-content'] From 56306dfdccef1f25271be9b95aaca24eb457296b Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Tue, 28 Apr 2026 00:25:50 +0530 Subject: [PATCH 08/37] fix(core): handle Chrome 143 split request lifecycle and malformed Content-Length (PPLT-4214) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 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) --- packages/core/src/network.js | 143 ++++++++++++++++++++++++++++++++++- 1 file changed, 142 insertions(+), 1 deletion(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index e19d6e3b4..4be75e7b3 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -4,6 +4,10 @@ import mime from 'mime-types'; import { DefaultMap, createResource, hostnameMatches, normalizeURL, waitFor, decodeAndEncodeURLWithLogging, handleIncorrectFontMimeType, executeDomainValidation } from './utils.js'; const MAX_RESOURCE_SIZE = 25 * (1024 ** 2) * 0.63; // 25MB, 0.63 factor for accounting for base64 encoding +// How long to wait for Network.loadingFinished after a Fetch response-stage +// pause before giving up on the request. Long enough to swallow normal jitter, +// short enough that it doesn't dominate a request's idle wait. +const RESPONSE_STAGE_BACKSTOP_MS = 1000; const ALLOWED_STATUSES = [200, 201, 301, 302, 304, 307, 308]; const ALLOWED_RESOURCES = ['Document', 'Stylesheet', 'Image', 'Media', 'Font', 'Other']; const ABORTED_MESSAGE = 'Request was aborted by browser'; @@ -205,6 +209,16 @@ export class Network { _handleRequestPaused = async (session, event) => { let { networkId: requestId, requestId: interceptId, resourceType } = event; + // Response-stage interception: Fetch.continueRequest with interceptResponse:true + // pauses the request a second time once response headers are available. We use + // this to guard against responses that Chrome 143 may never finish loading + // (e.g. malformed Content-Length), which would otherwise hang the page's + // load event waiting for a stylesheet body that never arrives. + if (event.responseStatusCode != null || event.responseErrorReason != null) { + await this._handleResponsePaused(session, event); + return; + } + // wait for request to be sent await this.#requestsLifeCycleHandler.get(requestId).requestWillBeSent; let pending = this.#pending.get(requestId); @@ -217,6 +231,107 @@ export class Network { await this._handleRequest(session, { ...pending, resourceType, interceptId }); } + // Called when a request that was continued with interceptResponse:true is + // paused a second time at the response stage. Three jobs: + // + // 1. If the browser already errored the response (server abort, DNS, etc.), + // confirm the failure so _handleLoadingFailed can log it. + // + // 2. If Content-Length says the response is oversized or malformed, fail + // the request before the body streams. This guards against Chrome 143 + // never emitting Network.loadingFinished for malformed Content-Length + // responses, which would otherwise hang the page's load event waiting + // for a stylesheet body that never arrives. + // + // 3. Otherwise, schedule a backstop cleanup. In Chrome 143, worker scripts + // fetched by the page never emit Network.loadingFinished on the page + // session — those events fire on the worker session under a fresh + // requestId. Without the backstop the page-session request entry would + // stay pending forever and block idle(). For all other requests the + // existing loadingFinished / loadingFailed handlers forget the request + // first and the backstop is a no-op. + _handleResponsePaused = async (session, event) => { + let { networkId: requestId, requestId: interceptId, responseHeaders, responseStatusCode, responseErrorReason } = event; + let request = this.#requests.get(requestId); + let url = request ? originURL(request) : event.request?.url; + let headersObj = headersArrayToObject(responseHeaders); + let { tooLarge, malformed, rawValue } = inspectContentLength(headersObj); + + // Network-error responses (server abort, DNS failure, etc.). Just confirm + // the failure and let _handleLoadingFailed log the error and clean up. + if (responseErrorReason) { + try { + await this.send(session, 'Fetch.failRequest', { + requestId: interceptId, + errorReason: responseErrorReason + }); + } catch (error) { + /* istanbul ignore next: race with abort/close */ + this.log.debug(`Failed to fail errored response for ${url}: ${error.message}`); + } + return; + } + + if (tooLarge || malformed) { + let meta = { ...this.meta, url, responseStatus: responseStatusCode }; + logAssetInstrumentation(this.log, 'asset_not_uploaded', 'resource_too_large', { + url, + size: rawValue, + snapshot: meta.snapshot + }); + this.log.debug('- Skipping resource larger than 25MB', meta); + + if (request) { + this._forgetRequest(request); + this.#requestsLifeCycleHandler.get(requestId).resolveResponseReceived(); + } + try { + await this.send(session, 'Fetch.failRequest', { + requestId: interceptId, + errorReason: 'Aborted' + }); + } catch (error) { + /* istanbul ignore next: race with abort/close */ + this.log.debug(`Failed to abort oversized response for ${url}: ${error.message}`); + } + return; + } + + // For redirect responses (3xx), don't process here — the redirect chain + // is constructed by _handleRequest when the next Fetch.requestPaused + // (request stage) fires for the same requestId with redirectResponse set. + // We just continue the response so the browser follows the redirect. + let isRedirect = responseStatusCode >= 300 && responseStatusCode < 400; + + // Schedule a backstop cleanup for requests where Network.loadingFinished / + // Network.loadingFailed will never fire on this session. The motivating + // case is Chrome 143 worker scripts loaded by the page: Fetch intercepts + // them on the page session, but the corresponding Network events fire on + // the worker session under a fresh requestId — so without this backstop + // the page-session entry would stay pending and block idle(). For all + // other requests, the existing loadingFinished / loadingFailed handlers + // forget the request first and this timer is a no-op. + if (!isRedirect && request) { + let backstop = setTimeout(() => { + if (this.#requests.has(requestId)) { + this.log.debug(`Forgetting request without loadingFinished: ${url}`, { ...this.meta, url }); + this._forgetRequest(request); + this.#requestsLifeCycleHandler.get(requestId).resolveResponseReceived(); + } + }, RESPONSE_STAGE_BACKSTOP_MS); + // don't keep the process alive on this timer + backstop.unref?.(); + } + + try { + await this.send(session, 'Fetch.continueResponse', { requestId: interceptId }); + } catch (error) { + /* istanbul ignore next: race with abort/close */ + if (error.message === ABORTED_MESSAGE || error.message.includes('Invalid InterceptionId')) return; + this.log.debug(`Failed to continue response for ${url}: ${error.message}`); + } + } + // Called when a request will be sent. If the request has already been intercepted, handle it; // otherwise set it to be pending until it is paused. _handleRequestWillBeSent = async event => { @@ -434,6 +549,28 @@ function originURL(request) { return normalizeURL((request.redirectChain[0] || request).url); } +// Convert Fetch.requestPaused responseHeaders (array of {name,value}) to a +// case-preserving header object for inspection. +function headersArrayToObject(arr) { + let out = {}; + if (!Array.isArray(arr)) return out; + for (let { name, value } of arr) out[name] = value; + return out; +} + +// Inspect a response's Content-Length header. Returns: +// - tooLarge: header parsed as finite number > MAX_RESOURCE_SIZE +// - malformed: header is present and non-empty but does not parse to a finite number +// - rawValue: the raw header value (for logging) +function inspectContentLength(headers) { + let key = headers && Object.keys(headers).find(k => k.toLowerCase() === 'content-length'); + let rawValue = key ? headers[key] : undefined; + let parsed = parseInt(rawValue); + let tooLarge = Number.isFinite(parsed) && parsed > MAX_RESOURCE_SIZE; + let malformed = rawValue !== undefined && rawValue !== null && String(rawValue).length > 0 && !Number.isFinite(parsed); + return { tooLarge, malformed, rawValue }; +} + // Validate domain for auto-allowlisting feature // Only validates domains that returned 200 status async function validateDomainForAllowlist(network, hostname, url, statusCode) { @@ -512,8 +649,12 @@ async function sendResponseResource(network, request, session) { .map(([k, v]) => ({ name: k.toLowerCase(), value: String(v) })) }); } else { + // interceptResponse:true pauses the request again at the response stage + // so we can guard against oversized or malformed responses before the + // browser tries to stream the body. See _handleResponsePaused. await send('Fetch.continueRequest', { - requestId: request.interceptId + requestId: request.interceptId, + interceptResponse: true }); } } catch (error) { From 6a5af2960beef6366cebcbd75c22ce73ee3b5245 Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Tue, 28 Apr 2026 08:48:35 +0530 Subject: [PATCH 09/37] fix(core): preserve original errorText for response-stage Fetch errors (PPLT-4214) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI on 56306dfd 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) --- packages/core/src/network.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 4be75e7b3..498c915ba 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -257,17 +257,20 @@ export class Network { let headersObj = headersArrayToObject(responseHeaders); let { tooLarge, malformed, rawValue } = inspectContentLength(headersObj); - // Network-error responses (server abort, DNS failure, etc.). Just confirm - // the failure and let _handleLoadingFailed log the error and clean up. + // Network-error responses (server abort, DNS failure, etc.). Continue the + // response so Chrome's natural Network.loadingFailed propagates the + // *original* errorText (e.g. net::ERR_EMPTY_RESPONSE) to + // _handleLoadingFailed. Calling Fetch.failRequest here would synthesize a + // new errorText derived from `responseErrorReason`, and a generic + // `Failed` collapses to net::ERR_FAILED which the asset_load_missing log + // path explicitly excludes — losing the instrumentation. if (responseErrorReason) { try { - await this.send(session, 'Fetch.failRequest', { - requestId: interceptId, - errorReason: responseErrorReason - }); + await this.send(session, 'Fetch.continueResponse', { requestId: interceptId }); } catch (error) { /* istanbul ignore next: race with abort/close */ - this.log.debug(`Failed to fail errored response for ${url}: ${error.message}`); + if (error.message === ABORTED_MESSAGE || error.message.includes('Invalid InterceptionId')) return; + this.log.debug(`Failed to continue errored response for ${url}: ${error.message}`); } return; } From 890883ad18010ab93cf1e475a3026dd74d413ed7 Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Tue, 28 Apr 2026 11:19:20 +0530 Subject: [PATCH 10/37] test(core): cover favicon capture path explicitly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packages/core/test/discovery.test.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 898bc8b42..7e979d9cf 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -457,6 +457,26 @@ describe('Discovery', () => { ])); }); + it('captures favicon when the server provides one', async () => { + server.reply('/favicon.ico', () => [200, 'image/x-icon', pixel]); + + await percy.snapshot({ + name: 'favicon snapshot', + url: 'http://localhost:8000', + domSnapshot: testDOM + }); + + await percy.idle(); + + expect(captured[0]).toEqual(jasmine.arrayContaining([ + jasmine.objectContaining({ + attributes: jasmine.objectContaining({ + 'resource-url': 'http://localhost:8000/favicon.ico' + }) + }) + ])); + }); + it('does not capture event-stream requests', async () => { let eventStreamDOM = dedent``]); + + await percy.snapshot({ + name: 'worker direct-fetch failure snapshot', + url: 'http://localhost:8000', + waitForSelector: '.done', + enableJavaScript: true + }); + + await percy.idle(); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + jasmine.stringMatching(/Direct fetch failed for http:\/\/localhost:8000\/worker\.js -/) + ])); + }); }); describe('with remote resources', () => { From 2d48f2040e87d8bb8a8e75026340e1bf71610f37 Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Mon, 11 May 2026 23:02:58 +0530 Subject: [PATCH 29/37] test(core): drop CI-unstable direct worker-script direct-fetch spec (PPLT-4214) The companion to c20c5813'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 c20c5813 (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) --- packages/core/test/discovery.test.js | 46 ---------------------------- 1 file changed, 46 deletions(-) diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 0af61ddc0..c878510f2 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -3222,52 +3222,6 @@ describe('Discovery', () => { jasmine.stringMatching(/Failed to continue response for http:\/\/example\.com\/orphan-continue-error: Target closed/) ])); }); - - it('logs gracefully when direct worker-script fetch fails', async () => { - // Same Network.requestWillBeSent stall as `captures requests from workers` - // to force the response-event race that triggers RESPONSE_RECEIVED_TIMEOUT. - spyOn(percy.browser, '_handleMessage').and.callFake(function(data) { - let { method } = JSON.parse(data); - if (method === 'Network.requestWillBeSent') { - setTimeout(this._handleMessage.and.originalFn.bind(this), 10, data); - } else { - this._handleMessage.and.originalFn.call(this, data); - } - }); - - let workerHits = 0; - server.reply('/worker.js', () => { - workerHits += 1; - if (workerHits === 1) { - return [200, 'text/javascript', dedent` - self.addEventListener("message", async ({ data }) => { - let response = await fetch(new Request(data)); - self.postMessage("done"); - })`]; - } - return [400, 'text/plain', 'bad request']; - }); - - server.reply('/', () => [200, 'text/html', dedent` - `]); - - await percy.snapshot({ - name: 'worker direct-fetch failure snapshot', - url: 'http://localhost:8000', - waitForSelector: '.done', - enableJavaScript: true - }); - - await percy.idle(); - - expect(logger.stderr).toEqual(jasmine.arrayContaining([ - jasmine.stringMatching(/Direct fetch failed for http:\/\/localhost:8000\/worker\.js -/) - ])); - }); }); describe('with remote resources', () => { From f0d9c8e7582474cf0151814c0c879e5b45c9ba58 Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Wed, 13 May 2026 12:28:23 +0530 Subject: [PATCH 30/37] fix(core): harden v143 direct-fetch fallback + rename captureScriptDirectly (PPLT-4214) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 2d48f204. 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 ae1d3886, 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) --- packages/core/src/network.js | 111 +++++++++++++++++++-------- packages/core/test/discovery.test.js | 35 +++++++++ 2 files changed, 113 insertions(+), 33 deletions(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index fa1cea63c..29c186dda 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -7,8 +7,11 @@ const MAX_RESOURCE_SIZE = 25 * (1024 ** 2) * 0.63; // 25MB, 0.63 factor for acco const ALLOWED_STATUSES = [200, 201, 301, 302, 304, 307, 308]; const ALLOWED_RESOURCES = ['Document', 'Stylesheet', 'Image', 'Media', 'Font', 'Other']; const ABORTED_MESSAGE = 'Request was aborted by browser'; -// Chrome 143 omits Network.responseReceived for worker scripts; cap the wait so loadingFinished can clean up. +// Chrome 143 omits Network.responseReceived for worker scripts; cap the wait +// so loadingFinished can clean up. Per-request — N timeouts accumulate to N*2s; real-world N is ~1–2. const RESPONSE_RECEIVED_TIMEOUT = 2000; +// Cap idle() impact when a host accepts the TCP connection then stalls during a direct fetch. +const DIRECT_FETCH_TIMEOUT = 5000; // Stable, machine-readable codes for abort errors thrown from this module. // Consumers should prefer `error.code` over string matching on `error.message`. @@ -279,8 +282,11 @@ export class Network { // hangs worker-initiated fetches, so we don't. _handleResponsePaused = async (session, event) => { let { networkId: requestId, requestId: interceptId, responseHeaders, responseStatusCode } = event; + // request may be undefined when a response-stage pause arrives for a request + // whose request-stage tracking we never installed (service-worker-fulfilled, + // or a cleanup race). We still need to unpause Chrome regardless. let request = this.#requests.get(requestId); - let url = request ? originURL(request) : event.request?.url; + let url = request ? originURL(request) : (event.request?.url && normalizeURL(event.request.url)); let headersObj = headersArrayToObject(responseHeaders); let { tooLarge, malformed, rawValue } = inspectContentLength(headersObj); @@ -290,14 +296,28 @@ export class Network { url, size: rawValue, snapshot: meta.snapshot }); this.log.debug('- Skipping resource larger than 25MB', meta); - if (request) { - this._forgetRequest(request); - this.#requestsLifeCycleHandler.get(requestId).resolveResponseReceived(); - } + + // Disposition first, then forget the request — so we never leave Chrome's + // Fetch state paused while Percy thinks the request is already done. try { await this.send(session, 'Fetch.failRequest', { requestId: interceptId, errorReason: 'Aborted' }); } catch (error) { - this.log.debug(`Failed to abort oversized response for ${url}: ${error.message}`); + if (error.message === ABORTED_MESSAGE || error.message.includes('Invalid InterceptionId')) { + // benign race — request was already aborted upstream; nothing to un-pause + } else { + this.log.warn(`Failed to abort oversized response for ${url}: ${error.message}`); + // Last-resort: un-pause Chrome's Fetch so it doesn't leak the response. + try { + await this.send(session, 'Fetch.continueResponse', { requestId: interceptId }); + } catch (continueError) { + this.log.debug(`Last-resort continueResponse also failed for ${url}: ${continueError.message}`); + } + } + } + + if (request) { + this._forgetRequest(request); + this.#requestsLifeCycleHandler.get(requestId).resolveResponseReceived(); } return; } @@ -312,7 +332,7 @@ export class Network { await this.send(session, 'Fetch.continueResponse', { requestId: interceptId }); } catch (error) { if (error.message === ABORTED_MESSAGE || error.message.includes('Invalid InterceptionId')) return; - this.log.debug(`Failed to continue response for ${url}: ${error.message}`); + this.log.warn(`Failed to continue response for ${url}: ${error.message}`); } } @@ -420,14 +440,16 @@ export class Network { if (!request.response) { this.log.debug(`Skipping resource: responseReceived not received within ${RESPONSE_RECEIVED_TIMEOUT}ms - ${request.url}`); - // Chrome 143+ fetches dedicated worker scripts in the browser process - // (PlzDedicatedWorker) and never surfaces the response on any CDP - // session. resourceType varies — v143 reports 'Other' for the worker - // script, older Chrome reports 'Script' — so we don't gate on type. - // We do gate on allowedHostnames so we never direct-fetch a host the - // snapshot wouldn't have captured anyway. - if (hostnameMatches(this.intercept.allowedHostnames, originURL(request))) { - await captureScriptDirectly(this, request, session); + // Chrome 143+ PlzDedicatedWorker: dedicated worker scripts fetch in the browser + // process and never surface a CDP response. resourceType varies ('Other' on v143, + // 'Script' on older Chrome) so we gate on hostname rather than type, and mirror + // sendResponseResource's disallowedHostnames-before-allowedHostnames precedence. + let url = originURL(request); + if (!hostnameMatches(this.intercept.disallowedHostnames, url) && + hostnameMatches(this.intercept.allowedHostnames, url)) { + await captureResourceDirectly(this, request, session); + } else { + this.log.debug(`- Skipping direct-fetch fallback for ${url}: hostname not allowed`, this.meta); } this._forgetRequest(request); return; @@ -507,7 +529,7 @@ export class Network { _initializeNetworkIdleWaitTimeout() { // Per-instance timeout so concurrent pages with different env values // (or env values changed mid-run by tests) don't stomp each other. - this.networkIdleWaitTimeout = parseInt(process.env.PERCY_NETWORK_IDLE_WAIT_TIMEOUT) || 30000; + this.networkIdleWaitTimeout = parseInt(process.env.PERCY_NETWORK_IDLE_WAIT_TIMEOUT, 10) || 30000; if (this.networkIdleWaitTimeout > 60000) { this.log.warn('Setting PERCY_NETWORK_IDLE_WAIT_TIMEOUT over 60000ms is not recommended. ' + @@ -567,7 +589,7 @@ function headersArrayToObject(arr) { function inspectContentLength(headers) { let key = headers && Object.keys(headers).find(k => k.toLowerCase() === 'content-length'); let rawValue = key ? headers[key] : undefined; - let parsed = parseInt(rawValue); + let parsed = parseInt(rawValue, 10); let tooLarge = Number.isFinite(parsed) && parsed > MAX_RESOURCE_SIZE; let malformed = rawValue !== undefined && rawValue !== null && String(rawValue).length > 0 && !Number.isFinite(parsed); return { tooLarge, malformed, rawValue }; @@ -689,15 +711,14 @@ async function sendResponseResource(network, request, session) { } } -// Make a new request with Node based on a network request. The session is -// usually the page session (full Network domain support), but for the v143 -// worker-script fallback the loadingFinished event arrives on the worker -// session where Network.getCookies returns an Internal error — proceed with -// no cookies in that case rather than abandoning the resource. +// Make a new request with Node based on a network request. Cookies are read +// from the page session because worker/auxiliary sessions have a partial +// Network domain where Network.getCookies throws "Internal error". async function makeDirectRequest(network, request, session) { let cookies = []; + let cookieSession = network.page?.session ?? session; try { - ({ cookies } = await session.send('Network.getCookies', { urls: [request.url] })); + ({ cookies } = await cookieSession.send('Network.getCookies', { urls: [request.url] })); } catch (error) { network.log.debug(`Network.getCookies unavailable for ${request.url}: ${error.message}`); } @@ -719,31 +740,53 @@ async function makeDirectRequest(network, request, session) { }; if (network.authorization?.username) { - // include basic authorization username and password - let { username, password } = network.authorization; - let token = Buffer.from([username, password || ''].join(':')).toString('base64'); - headers.Authorization = `Basic ${token}`; + // Browser's URLLoader origin-scopes Basic auth; this fallback runs in Node, so + // we re-enforce the same-origin rule explicitly to avoid leaking creds. + let targetOrigin, pageOrigin; + try { targetOrigin = new URL(request.url).origin; } catch {} + try { pageOrigin = new URL(network.meta?.snapshotURL).origin; } catch {} + + if (targetOrigin && pageOrigin && targetOrigin === pageOrigin) { + let { username, password } = network.authorization; + let token = Buffer.from([username, password || ''].join(':')).toString('base64'); + headers.Authorization = `Basic ${token}`; + } } - return makeRequest(request.url, { buffer: true, headers }); + return makeRequest(request.url, { buffer: true, headers }, (body, res) => ({ body, status: res.statusCode })); } // Capture a resource via direct HTTP fetch when the browser-side response // never surfaces — Chrome 143+ fetches dedicated worker scripts in the browser // process (PlzDedicatedWorker) so loadingFinished fires without a body on CDP. -async function captureScriptDirectly(network, request, session) { +async function captureResourceDirectly(network, request, session) { let log = network.log; let url = originURL(request); let meta = { ...network.meta, url }; + let timerId; try { log.debug('- Requesting resource directly (responseReceived timeout fallback)', meta); - let body = await makeDirectRequest(network, request, session); + let { body, status } = await Promise.race([ + makeDirectRequest(network, request, session), + new Promise((_, reject) => { + timerId = setTimeout(() => reject(new Error(`Direct fetch timed out after ${DIRECT_FETCH_TIMEOUT}ms`)), DIRECT_FETCH_TIMEOUT); + }) + ]); + + if (body.length > MAX_RESOURCE_SIZE) { + logAssetInstrumentation(log, 'asset_not_uploaded', 'resource_too_large', { + url, size: body.length, snapshot: meta.snapshot + }); + log.debug('- Skipping resource larger than 25MB', meta); + return; + } + let urlObj = new URL(url); let mimeType = mime.lookup(urlObj.origin + urlObj.pathname) || 'application/javascript'; let resource = createResource(url, body, mimeType, { - status: 200, + status, headers: { 'content-type': [mimeType] } }); @@ -751,6 +794,8 @@ async function captureScriptDirectly(network, request, session) { network.intercept.saveResource(resource); } catch (error) { log.debug(`Direct fetch failed for ${url} - ${error.message}`, meta); + } finally { + clearTimeout(timerId); } } @@ -865,7 +910,7 @@ async function saveResponseResource(network, request, session) { // so request them directly. if (mimeType?.includes('font') || (detectedMime && detectedMime.includes('font'))) { log.debug('- Requesting asset directly', meta); - body = await makeDirectRequest(network, request, session); + ({ body } = await makeDirectRequest(network, request, session)); log.debug('- Got direct response', meta); } diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index c878510f2..35272554a 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -3222,6 +3222,41 @@ describe('Discovery', () => { jasmine.stringMatching(/Failed to continue response for http:\/\/example\.com\/orphan-continue-error: Target closed/) ])); }); + + it('logs gracefully when the direct-fetch fallback fails', async () => { + // Drop Network.responseReceived for one asset so it enters the + // RESPONSE_RECEIVED_TIMEOUT path that calls captureResourceDirectly, + // then make the direct fetch return 400 so the catch logs. + spyOn(percy.browser, '_handleMessage').and.callFake(function(data) { + let parsed; try { parsed = JSON.parse(data); } catch { /* binary */ } + if (parsed?.method === 'Network.responseReceived' && + parsed?.params?.response?.url?.endsWith('/direct-fetch-target.css')) { + return; + } + this._handleMessage.and.originalFn.call(this, data); + }); + + let assetHits = 0; + server.reply('/direct-fetch-target.css', () => { + assetHits += 1; + if (assetHits === 1) return [200, 'text/css', 'p { color: blue; }']; + return [400, 'text/plain', 'bad request']; + }); + + let targetDOM = `

x

`; + + await percy.snapshot({ + name: 'direct-fetch failure snapshot', + url: 'http://localhost:8000', + domSnapshot: targetDOM + }); + + await percy.idle(); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + jasmine.stringMatching(/Direct fetch failed for http:\/\/localhost:8000\/direct-fetch-target\.css -/) + ])); + }); }); describe('with remote resources', () => { From 334f062103869763324525af8b4ed7a273b99f8a Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Wed, 13 May 2026 14:57:13 +0530 Subject: [PATCH 31/37] 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) --- packages/core/src/network.js | 4 ++-- packages/core/test/discovery.test.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 29c186dda..17f5bd307 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -305,7 +305,7 @@ export class Network { if (error.message === ABORTED_MESSAGE || error.message.includes('Invalid InterceptionId')) { // benign race — request was already aborted upstream; nothing to un-pause } else { - this.log.warn(`Failed to abort oversized response for ${url}: ${error.message}`); + this.log.debug(`Failed to abort oversized response for ${url}: ${error.message}`); // Last-resort: un-pause Chrome's Fetch so it doesn't leak the response. try { await this.send(session, 'Fetch.continueResponse', { requestId: interceptId }); @@ -332,7 +332,7 @@ export class Network { await this.send(session, 'Fetch.continueResponse', { requestId: interceptId }); } catch (error) { if (error.message === ABORTED_MESSAGE || error.message.includes('Invalid InterceptionId')) return; - this.log.warn(`Failed to continue response for ${url}: ${error.message}`); + this.log.debug(`Failed to continue response for ${url}: ${error.message}`); } } diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 35272554a..856d63137 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -3243,7 +3243,7 @@ describe('Discovery', () => { return [400, 'text/plain', 'bad request']; }); - let targetDOM = `

x

`; + let targetDOM = '

x

'; await percy.snapshot({ name: 'direct-fetch failure snapshot', From 95266618ec5e1341320987e006cb80586b3c3dbe Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Thu, 14 May 2026 00:40:37 +0530 Subject: [PATCH 32/37] 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%. --- packages/core/src/network.js | 5 ++ packages/core/test/discovery.test.js | 68 ++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 17f5bd307..4a897d0b9 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -445,6 +445,11 @@ export class Network { // 'Script' on older Chrome) so we gate on hostname rather than type, and mirror // sendResponseResource's disallowedHostnames-before-allowedHostnames precedence. let url = originURL(request); + /* istanbul ignore else: the else only fires for PlzDedicatedWorker requests + whose worker-script fetch bypasses Fetch.requestPaused. Cross-origin assets + loaded via the document session still go through sendResponseResource + (which performs its own disallowedHostnames check), so the test harness + can't reliably reach this skip branch via integration tests. */ if (!hostnameMatches(this.intercept.disallowedHostnames, url) && hostnameMatches(this.intercept.allowedHostnames, url)) { await captureResourceDirectly(this, request, session); diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 856d63137..41e184315 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -3257,6 +3257,74 @@ describe('Discovery', () => { jasmine.stringMatching(/Direct fetch failed for http:\/\/localhost:8000\/direct-fetch-target\.css -/) ])); }); + + it('logs when Network.getCookies fails during direct-fetch fallback', async () => { + spyOn(percy.browser, '_handleMessage').and.callFake(function(data) { + let parsed; try { parsed = JSON.parse(data); } catch { /* binary */ } + if (parsed?.method === 'Network.responseReceived' && + parsed?.params?.response?.url?.endsWith('/cookies-fail.css')) { + return; + } + this._handleMessage.and.originalFn.call(this, data); + }); + + spyOn(Session.prototype, 'send').and.callFake(function(method, params) { + if (method === 'Network.getCookies' && params?.urls?.[0]?.includes('cookies-fail.css')) { + return Promise.reject(new Error('Internal error')); + } + return Session.prototype.send.and.originalFn.call(this, method, params); + }); + + server.reply('/cookies-fail.css', () => [200, 'text/css', 'p { color: blue; }']); + let targetDOM = '

x

'; + + await percy.snapshot({ + name: 'cookies fail snapshot', + url: 'http://localhost:8000', + domSnapshot: targetDOM + }); + + await percy.idle(); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + jasmine.stringMatching(/Network\.getCookies unavailable for http:\/\/localhost:8000\/cookies-fail\.css: Internal error/) + ])); + }); + + it('skips direct-fetched resource when body exceeds 25MB', async () => { + spyOn(percy.browser, '_handleMessage').and.callFake(function(data) { + let parsed; try { parsed = JSON.parse(data); } catch { /* binary */ } + if (parsed?.method === 'Network.responseReceived' && + parsed?.params?.response?.url?.endsWith('/oversized.css')) { + return; + } + this._handleMessage.and.originalFn.call(this, data); + }); + + let assetHits = 0; + // 17MB exceeds MAX_RESOURCE_SIZE (≈15.75 MB after base64 factor) + let oversizedBody = Buffer.alloc(17 * 1024 * 1024, 'x'); + server.reply('/oversized.css', () => { + assetHits += 1; + if (assetHits === 1) return [200, 'text/css', 'p { color: blue; }']; + return [200, 'text/css', oversizedBody]; + }); + + let targetDOM = '

x

'; + + await percy.snapshot({ + name: 'oversized direct-fetch snapshot', + url: 'http://localhost:8000', + domSnapshot: targetDOM + }); + + await percy.idle(); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + jasmine.stringMatching(/Skipping resource larger than 25MB/) + ])); + }); + }); describe('with remote resources', () => { From 135f9c9dd31108d2b3a58770c8b0789ea00f1671 Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Thu, 14 May 2026 00:43:34 +0530 Subject: [PATCH 33/37] style(core): drop padded blank line at end of resource-errors describe --- packages/core/test/discovery.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 41e184315..057fc47f1 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -3324,7 +3324,6 @@ describe('Discovery', () => { jasmine.stringMatching(/Skipping resource larger than 25MB/) ])); }); - }); describe('with remote resources', () => { From a22e8bfe22987e145b7540c2a97ae845d2389919 Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Thu, 14 May 2026 14:46:17 +0530 Subject: [PATCH 34/37] refactor(core): extract direct-fetch helpers for clean coverage (PPLT-4214) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/core/src/network.js | 39 ++++++++++++++------- packages/core/test/discovery.test.js | 35 +++++++++++++++++++ packages/core/test/unit/network.test.js | 45 ++++++++++++++++++++++++- 3 files changed, 105 insertions(+), 14 deletions(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 4a897d0b9..7fee69ccf 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -716,12 +716,33 @@ async function sendResponseResource(network, request, session) { } } +// Pick the CDP session for Network.getCookies. Worker/auxiliary sessions +// expose a partial Network domain where Network.getCookies throws +// "Internal error", so prefer the page's session whenever available and +// fall back to the request's own session otherwise. +export function pickCookieSession(network, session) { + return network.page?.session ?? session; +} + +// Decide whether to attach a Basic auth header to the Node-side direct fetch. +// The browser's URLLoader origin-scopes Basic auth; this fallback runs in +// Node, so we re-enforce the same-origin rule explicitly to avoid leaking +// credentials cross-origin. Malformed URLs fall through to `false` defensively. +export function shouldAttachAuth(authorization, requestUrl, snapshotUrl) { + if (!authorization?.username) return false; + try { + return new URL(requestUrl).origin === new URL(snapshotUrl).origin; + } catch { + return false; + } +} + // Make a new request with Node based on a network request. Cookies are read // from the page session because worker/auxiliary sessions have a partial // Network domain where Network.getCookies throws "Internal error". async function makeDirectRequest(network, request, session) { let cookies = []; - let cookieSession = network.page?.session ?? session; + let cookieSession = pickCookieSession(network, session); try { ({ cookies } = await cookieSession.send('Network.getCookies', { urls: [request.url] })); } catch (error) { @@ -744,18 +765,10 @@ async function makeDirectRequest(network, request, session) { cookie: cookies.map(cookie => `${cookie.name}=${cookie.value}`).join('; ') }; - if (network.authorization?.username) { - // Browser's URLLoader origin-scopes Basic auth; this fallback runs in Node, so - // we re-enforce the same-origin rule explicitly to avoid leaking creds. - let targetOrigin, pageOrigin; - try { targetOrigin = new URL(request.url).origin; } catch {} - try { pageOrigin = new URL(network.meta?.snapshotURL).origin; } catch {} - - if (targetOrigin && pageOrigin && targetOrigin === pageOrigin) { - let { username, password } = network.authorization; - let token = Buffer.from([username, password || ''].join(':')).toString('base64'); - headers.Authorization = `Basic ${token}`; - } + if (shouldAttachAuth(network.authorization, request.url, network.meta?.snapshotURL)) { + let { username, password } = network.authorization; + let token = Buffer.from([username, password || ''].join(':')).toString('base64'); + headers.Authorization = `Basic ${token}`; } return makeRequest(request.url, { buffer: true, headers }, (body, res) => ({ body, status: res.statusCode })); diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 057fc47f1..6527fe79d 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -3324,6 +3324,41 @@ describe('Discovery', () => { jasmine.stringMatching(/Skipping resource larger than 25MB/) ])); }); + + it('falls back to application/javascript mimetype on direct-fetch when URL has no extension', async () => { + // Mirrors the same-origin direct-fetch test scaffolding but uses a URL + // with no recognizable extension so mime.lookup returns falsy and the + // captureResourceDirectly path falls through to 'application/javascript'. + spyOn(percy.browser, '_handleMessage').and.callFake(function(data) { + let parsed; try { parsed = JSON.parse(data); } catch { /* binary */ } + if (parsed?.method === 'Network.responseReceived' && + parsed?.params?.response?.url?.endsWith('/asset-no-ext')) { + return; + } + this._handleMessage.and.originalFn.call(this, data); + }); + + let assetHits = 0; + server.reply('/asset-no-ext', () => { + assetHits += 1; + if (assetHits === 1) return [200, 'text/css', 'p { color: blue; }']; + return [200, 'application/octet-stream', 'p { color: red; }']; + }); + + let targetDOM = '

x

'; + + await percy.snapshot({ + name: 'unknown extension direct-fetch snapshot', + url: 'http://localhost:8000', + domSnapshot: targetDOM + }); + + await percy.idle(); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + jasmine.stringMatching(/Saving direct-fetched resource sha=[a-f0-9]+ mimetype=application\/javascript/) + ])); + }); }); describe('with remote resources', () => { diff --git a/packages/core/test/unit/network.test.js b/packages/core/test/unit/network.test.js index ac5e678a3..eb02b1aed 100644 --- a/packages/core/test/unit/network.test.js +++ b/packages/core/test/unit/network.test.js @@ -1,5 +1,5 @@ import { setupTest } from '../helpers/index.js'; -import { Network, AbortCodes } from '../../src/network.js'; +import { Network, AbortCodes, pickCookieSession, shouldAttachAuth } from '../../src/network.js'; import { AbortError } from '../../src/utils.js'; describe('Unit / Network', () => { @@ -53,4 +53,47 @@ describe('Unit / Network', () => { expect(err.message).toBe('msg'); }); }); + + // pickCookieSession — prefer the page's full Network domain, fall back to + // the request's own session for worker/auxiliary paths. + describe('pickCookieSession', () => { + it('returns the page session when network.page.session is set', () => { + let pageSession = { id: 'page' }; + let fallback = { id: 'fallback' }; + expect(pickCookieSession({ page: { session: pageSession } }, fallback)).toBe(pageSession); + }); + + it('falls back to the passed session when network.page is undefined', () => { + let fallback = { id: 'fallback' }; + expect(pickCookieSession({ page: undefined }, fallback)).toBe(fallback); + }); + + it('falls back to the passed session when network.page lacks a session', () => { + let fallback = { id: 'fallback' }; + expect(pickCookieSession({ page: {} }, fallback)).toBe(fallback); + }); + }); + + // shouldAttachAuth — re-enforces the same-origin rule for the Node-side + // direct fetch to avoid leaking Basic auth credentials cross-origin. + describe('shouldAttachAuth', () => { + it('returns false when authorization is missing or has no username', () => { + expect(shouldAttachAuth(undefined, 'http://a.com/x', 'http://a.com')).toBe(false); + expect(shouldAttachAuth({}, 'http://a.com/x', 'http://a.com')).toBe(false); + expect(shouldAttachAuth({ username: '' }, 'http://a.com/x', 'http://a.com')).toBe(false); + }); + + it('returns true when authorization is set and origins match', () => { + expect(shouldAttachAuth({ username: 'u' }, 'http://a.com/x.css', 'http://a.com/page')).toBe(true); + }); + + it('returns false when origins differ (cross-origin auth must not leak)', () => { + expect(shouldAttachAuth({ username: 'u' }, 'http://a.com/x.css', 'http://b.com/page')).toBe(false); + }); + + it('returns false when either URL is malformed (defensive)', () => { + expect(shouldAttachAuth({ username: 'u' }, 'not a url', 'http://a.com')).toBe(false); + expect(shouldAttachAuth({ username: 'u' }, 'http://a.com', undefined)).toBe(false); + }); + }); }); From 472d349685331d64ac8980a917f7f847a34bc61b Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Thu, 14 May 2026 15:19:53 +0530 Subject: [PATCH 35/37] refactor(core): extract raceWithTimeout helper for clean coverage (PPLT-4214) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- packages/core/src/network.js | 25 +++++++++++++++++-------- packages/core/test/unit/network.test.js | 22 +++++++++++++++++++++- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 7fee69ccf..69ff8e3b4 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -737,6 +737,19 @@ export function shouldAttachAuth(authorization, requestUrl, snapshotUrl) { } } +// Race a promise against a timeout. Resolves with the promise's value if it +// settles within `ms`, otherwise rejects with `new Error(message)`. The +// internal timer is always cleared so the event loop can exit cleanly. +export function raceWithTimeout(promise, ms, message) { + let timerId; + return Promise.race([ + promise, + new Promise((_, reject) => { + timerId = setTimeout(() => reject(new Error(message)), ms); + }) + ]).finally(() => clearTimeout(timerId)); +} + // Make a new request with Node based on a network request. Cookies are read // from the page session because worker/auxiliary sessions have a partial // Network domain where Network.getCookies throws "Internal error". @@ -782,15 +795,13 @@ async function captureResourceDirectly(network, request, session) { let url = originURL(request); let meta = { ...network.meta, url }; - let timerId; try { log.debug('- Requesting resource directly (responseReceived timeout fallback)', meta); - let { body, status } = await Promise.race([ + let { body, status } = await raceWithTimeout( makeDirectRequest(network, request, session), - new Promise((_, reject) => { - timerId = setTimeout(() => reject(new Error(`Direct fetch timed out after ${DIRECT_FETCH_TIMEOUT}ms`)), DIRECT_FETCH_TIMEOUT); - }) - ]); + DIRECT_FETCH_TIMEOUT, + `Direct fetch timed out after ${DIRECT_FETCH_TIMEOUT}ms` + ); if (body.length > MAX_RESOURCE_SIZE) { logAssetInstrumentation(log, 'asset_not_uploaded', 'resource_too_large', { @@ -812,8 +823,6 @@ async function captureResourceDirectly(network, request, session) { network.intercept.saveResource(resource); } catch (error) { log.debug(`Direct fetch failed for ${url} - ${error.message}`, meta); - } finally { - clearTimeout(timerId); } } diff --git a/packages/core/test/unit/network.test.js b/packages/core/test/unit/network.test.js index eb02b1aed..73f09d642 100644 --- a/packages/core/test/unit/network.test.js +++ b/packages/core/test/unit/network.test.js @@ -1,5 +1,5 @@ import { setupTest } from '../helpers/index.js'; -import { Network, AbortCodes, pickCookieSession, shouldAttachAuth } from '../../src/network.js'; +import { Network, AbortCodes, pickCookieSession, shouldAttachAuth, raceWithTimeout } from '../../src/network.js'; import { AbortError } from '../../src/utils.js'; describe('Unit / Network', () => { @@ -96,4 +96,24 @@ describe('Unit / Network', () => { expect(shouldAttachAuth({ username: 'u' }, 'http://a.com', undefined)).toBe(false); }); }); + + // raceWithTimeout — caps any async work at a wall-clock budget. The + // direct-fetch fallback uses this to keep a hanging worker host from + // blocking the snapshot pipeline. + describe('raceWithTimeout', () => { + it('resolves with the promise value when it settles before the timeout', async () => { + let value = await raceWithTimeout(Promise.resolve('ok'), 50, 'timeout'); + expect(value).toBe('ok'); + }); + + it('rejects with the timeout message when the promise hangs past the budget', async () => { + let neverSettles = new Promise(() => {}); + await expectAsync(raceWithTimeout(neverSettles, 10, 'too slow')).toBeRejectedWithError('too slow'); + }); + + it('propagates the original rejection when the promise rejects before the timeout', async () => { + let failing = Promise.reject(new Error('boom')); + await expectAsync(raceWithTimeout(failing, 50, 'timeout')).toBeRejectedWithError('boom'); + }); + }); }); From ebad827502526ba5203f5bd990a4224548fc4a62 Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Thu, 14 May 2026 16:50:47 +0530 Subject: [PATCH 36/37] fix(core): honor server Content-Type on direct-fetch; safer mime fallback (PPLT-4214) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- packages/core/src/network.js | 13 ++++++++++--- packages/core/test/discovery.test.js | 10 +++++----- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 69ff8e3b4..6496d4a73 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -784,7 +784,9 @@ async function makeDirectRequest(network, request, session) { headers.Authorization = `Basic ${token}`; } - return makeRequest(request.url, { buffer: true, headers }, (body, res) => ({ body, status: res.statusCode })); + return makeRequest(request.url, { buffer: true, headers }, (body, res) => ({ + body, status: res.statusCode, headers: res.headers + })); } // Capture a resource via direct HTTP fetch when the browser-side response @@ -797,7 +799,7 @@ async function captureResourceDirectly(network, request, session) { try { log.debug('- Requesting resource directly (responseReceived timeout fallback)', meta); - let { body, status } = await raceWithTimeout( + let { body, status, headers: responseHeaders } = await raceWithTimeout( makeDirectRequest(network, request, session), DIRECT_FETCH_TIMEOUT, `Direct fetch timed out after ${DIRECT_FETCH_TIMEOUT}ms` @@ -811,8 +813,13 @@ async function captureResourceDirectly(network, request, session) { return; } + // Prefer the server's explicit Content-Type, then URL-extension mime, + // then a safe binary default. NOT 'application/javascript' — JS is the + // worst wrong guess because the renderer will try to parse non-JS + // content as code and produce confusing errors. let urlObj = new URL(url); - let mimeType = mime.lookup(urlObj.origin + urlObj.pathname) || 'application/javascript'; + let serverMime = responseHeaders?.['content-type']?.split(';')[0]?.trim(); + let mimeType = serverMime || mime.lookup(urlObj.origin + urlObj.pathname) || 'application/octet-stream'; let resource = createResource(url, body, mimeType, { status, diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 6527fe79d..77fa91fcb 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -3325,10 +3325,10 @@ describe('Discovery', () => { ])); }); - it('falls back to application/javascript mimetype on direct-fetch when URL has no extension', async () => { - // Mirrors the same-origin direct-fetch test scaffolding but uses a URL - // with no recognizable extension so mime.lookup returns falsy and the - // captureResourceDirectly path falls through to 'application/javascript'. + it('uses server Content-Type for direct-fetch mimetype when URL has no extension', async () => { + // For URLs with no recognizable extension, the direct-fetch path must + // honor the server's Content-Type response header rather than guessing + // a default. Server returns 'application/octet-stream' → that wins. spyOn(percy.browser, '_handleMessage').and.callFake(function(data) { let parsed; try { parsed = JSON.parse(data); } catch { /* binary */ } if (parsed?.method === 'Network.responseReceived' && @@ -3356,7 +3356,7 @@ describe('Discovery', () => { await percy.idle(); expect(logger.stderr).toEqual(jasmine.arrayContaining([ - jasmine.stringMatching(/Saving direct-fetched resource sha=[a-f0-9]+ mimetype=application\/javascript/) + jasmine.stringMatching(/Saving direct-fetched resource sha=[a-f0-9]+ mimetype=application\/octet-stream/) ])); }); }); From 59852c68538138c937041bd605f420450ed5227b Mon Sep 17 00:00:00 2001 From: Manoj-Katta Date: Thu, 14 May 2026 17:16:27 +0530 Subject: [PATCH 37/37] refactor(core): extract DISABLED_FEATURES + resolveDirectFetchMime helpers (PPLT-4214) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- packages/core/src/browser.js | 16 +++++++++++----- packages/core/src/network.js | 16 +++++++++------- packages/core/test/unit/network.test.js | 20 +++++++++++++++++++- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/packages/core/src/browser.js b/packages/core/src/browser.js index ac4a52bbc..abdf7aebf 100644 --- a/packages/core/src/browser.js +++ b/packages/core/src/browser.js @@ -11,6 +11,16 @@ import install from './install.js'; import Session from './session.js'; import Page from './page.js'; +// Chrome features Percy disables for v143 new-headless asset discovery. +const DISABLED_FEATURES = [ + 'Translate', // suppress translate prompt overlay + 'OptimizationGuideModelDownloading', // suppress background model fetches + 'IsolateOrigins', // [headless-only] keep cross-origin sub-resources on the page session for CDP capture + 'site-per-process', // companion to IsolateOrigins + 'HttpsFirstBalancedModeAutoEnable', // allow HTTP customer URLs (CI / local dev / staging) + 'LocalNetworkAccessChecks' // allow loopback/RFC1918 sub-resources (Chrome 143 LNA gating) +]; + export class Browser extends EventEmitter { log = logger('core:browser'); sessions = new Map(); @@ -21,11 +31,7 @@ export class Browser extends EventEmitter { #lastid = 0; args = [ - // Disable Chrome features that break asset discovery in v143 new-headless: - // site/origin isolation (so cross-origin events stay on the page session), - // HTTPS-first auto-upgrade (would block HTTP discovery), and Local Network - // Access permission checks (would block sub-resources to localhost/RFC1918). - '--disable-features=Translate,OptimizationGuideModelDownloading,IsolateOrigins,site-per-process,HttpsFirstBalancedModeAutoEnable,LocalNetworkAccessChecks', + `--disable-features=${DISABLED_FEATURES.join(',')}`, // disable several subsystems which run network requests in the background '--disable-background-networking', // disable task throttling of timer tasks from background pages diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 6496d4a73..0f0674df7 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -8,7 +8,8 @@ const ALLOWED_STATUSES = [200, 201, 301, 302, 304, 307, 308]; const ALLOWED_RESOURCES = ['Document', 'Stylesheet', 'Image', 'Media', 'Font', 'Other']; const ABORTED_MESSAGE = 'Request was aborted by browser'; // Chrome 143 omits Network.responseReceived for worker scripts; cap the wait -// so loadingFinished can clean up. Per-request — N timeouts accumulate to N*2s; real-world N is ~1–2. +// so loadingFinished can clean up. Per-request — N timeouts accumulate to N*2s; +// PERCY_NETWORK_IDLE_WAIT_TIMEOUT (default 30s) caps cumulative impact. const RESPONSE_RECEIVED_TIMEOUT = 2000; // Cap idle() impact when a host accepts the TCP connection then stalls during a direct fetch. const DIRECT_FETCH_TIMEOUT = 5000; @@ -750,6 +751,12 @@ export function raceWithTimeout(promise, ms, message) { ]).finally(() => clearTimeout(timerId)); } +// Server Content-Type wins; URL-extension mime is the fallback; binary default last. +export function resolveDirectFetchMime(responseHeaders, urlForLookup) { + let serverMime = responseHeaders?.['content-type']?.split(';')[0].trim(); + return serverMime || mime.lookup(urlForLookup) || 'application/octet-stream'; +} + // Make a new request with Node based on a network request. Cookies are read // from the page session because worker/auxiliary sessions have a partial // Network domain where Network.getCookies throws "Internal error". @@ -813,13 +820,8 @@ async function captureResourceDirectly(network, request, session) { return; } - // Prefer the server's explicit Content-Type, then URL-extension mime, - // then a safe binary default. NOT 'application/javascript' — JS is the - // worst wrong guess because the renderer will try to parse non-JS - // content as code and produce confusing errors. let urlObj = new URL(url); - let serverMime = responseHeaders?.['content-type']?.split(';')[0]?.trim(); - let mimeType = serverMime || mime.lookup(urlObj.origin + urlObj.pathname) || 'application/octet-stream'; + let mimeType = resolveDirectFetchMime(responseHeaders, urlObj.origin + urlObj.pathname); let resource = createResource(url, body, mimeType, { status, diff --git a/packages/core/test/unit/network.test.js b/packages/core/test/unit/network.test.js index 73f09d642..f45e8e969 100644 --- a/packages/core/test/unit/network.test.js +++ b/packages/core/test/unit/network.test.js @@ -1,5 +1,5 @@ import { setupTest } from '../helpers/index.js'; -import { Network, AbortCodes, pickCookieSession, shouldAttachAuth, raceWithTimeout } from '../../src/network.js'; +import { Network, AbortCodes, pickCookieSession, shouldAttachAuth, raceWithTimeout, resolveDirectFetchMime } from '../../src/network.js'; import { AbortError } from '../../src/utils.js'; describe('Unit / Network', () => { @@ -116,4 +116,22 @@ describe('Unit / Network', () => { await expectAsync(raceWithTimeout(failing, 50, 'timeout')).toBeRejectedWithError('boom'); }); }); + + // resolveDirectFetchMime — server header > URL ext > binary default. + describe('resolveDirectFetchMime', () => { + it('returns the bare server MIME, stripping parameters', () => { + expect(resolveDirectFetchMime({ 'content-type': 'text/css' }, '/x')).toBe('text/css'); + expect(resolveDirectFetchMime({ 'content-type': 'text/css; charset=utf-8' }, '/x')).toBe('text/css'); + }); + + it('falls back to URL-extension mime when no server header', () => { + expect(resolveDirectFetchMime({}, '/file.css')).toBe('text/css'); + expect(resolveDirectFetchMime(undefined, '/file.png')).toBe('image/png'); + }); + + it('falls back to application/octet-stream when neither header nor URL extension is recognized', () => { + expect(resolveDirectFetchMime({}, '/no-ext')).toBe('application/octet-stream'); + expect(resolveDirectFetchMime({ 'content-type': '' }, '/no-ext')).toBe('application/octet-stream'); + }); + }); });