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+ diff --git a/packages/core/src/browser.js b/packages/core/src/browser.js index e3fdc53d5..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,8 +31,7 @@ export class Browser extends EventEmitter { #lastid = 0; args = [ - // disable the translate popup and optimization downloads - '--disable-features=Translate,OptimizationGuideModelDownloading', + `--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 @@ -300,9 +309,11 @@ export class Browser extends EventEmitter { if (match) cleanup(() => resolve(match[1])); }; - let handleExitClose = () => handleError(); + let handleExitClose = () => handleError( + new Error('Browser exited before devtools address') + ); let handleError = error => cleanup(() => reject(new Error( - `Failed to launch browser. ${error?.message ?? ''}\n${stderr}'\n\n` + `Failed to launch browser. ${error.message}\n${stderr}'\n\n` ))); let cleanup = callback => { diff --git a/packages/core/src/install.js b/packages/core/src/install.js index ac87c89bd..d5f3ff391 100644 --- a/packages/core/src/install.js +++ b/packages/core/src/install.js @@ -175,13 +175,14 @@ export function chromium({ }); } -// default chromium revisions corresponds to v126.0.6478.184 +// Chrome 143.0.7499.169 (base position 1536371) — closest per-platform +// revision from https://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html 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 98c15f260..0f0674df7 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -7,6 +7,12 @@ 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. 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; // Stable, machine-readable codes for abort errors thrown from this module. // Consumers should prefer `error.code` over string matching on `error.message`. @@ -74,7 +80,7 @@ export class Network { this.captureMockedServiceWorker = options.captureMockedServiceWorker ?? false; this.userAgent = options.userAgent ?? // by default, emulate a non-headless browser - page.session.browser.version.userAgent.replace('Headless', ''); + page.session.browser?.version?.userAgent?.replace('Headless', ''); this.fontDomains = options.fontDomains || []; this.intercept = options.intercept; this.meta = options.meta; @@ -250,6 +256,13 @@ export class Network { _handleRequestPaused = async (session, event) => { let { networkId: requestId, requestId: interceptId, resourceType } = event; + // Response-stage events arrive here when Fetch.continueRequest was called + // with interceptResponse:true (see sendResponseResource). + 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); @@ -262,6 +275,68 @@ export class Network { await this._handleRequest(session, { ...pending, resourceType, interceptId }); } + // Response-stage interception is kept ONLY to detect oversized/malformed + // Content-Length and abort the request before Chrome streams a body it + // would never terminate (Chrome 143 quirk). For everything else we just + // continue — body capture happens later via Network.loadingFinished → + // Network.getResponseBody (the v126 path). Reading the body at this stage + // 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 && normalizeURL(event.request.url)); + let headersObj = headersArrayToObject(responseHeaders); + let { tooLarge, malformed, rawValue } = inspectContentLength(headersObj); + + 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); + + // 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) { + if (error.message === ABORTED_MESSAGE || error.message.includes('Invalid InterceptionId')) { + // benign race — request was already aborted upstream; nothing to un-pause + } else { + 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 }); + } 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; + } + + return this._continueResponse(session, interceptId, url); + } + + // Tell the browser to continue the paused response, swallowing expected + // races (request already aborted, interception ID no longer valid). + _continueResponse = async (session, interceptId, url) => { + try { + 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}`); + } + } + // 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 => { @@ -351,12 +426,41 @@ export class Network { // callback. The request should have an associated response and be finished with any redirects. _handleLoadingFinished = async (session, event) => { let { requestId } = event; - // wait for upto 2 seconds or check if response has been sent - await this.#requestsLifeCycleHandler.get(requestId).responseReceived; let request = this.#requests.get(requestId); /* istanbul ignore if: race condition paranoia */ if (!request) return; + if (!request.response) { + let timerId; + await Promise.race([ + this.#requestsLifeCycleHandler.get(requestId).responseReceived, + new Promise(resolve => { timerId = setTimeout(resolve, RESPONSE_RECEIVED_TIMEOUT); }) + ]); + clearTimeout(timerId); + } + + if (!request.response) { + this.log.debug(`Skipping resource: responseReceived not received within ${RESPONSE_RECEIVED_TIMEOUT}ms - ${request.url}`); + // 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); + /* 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); + } else { + this.log.debug(`- Skipping direct-fetch fallback for ${url}: hostname not allowed`, this.meta); + } + this._forgetRequest(request); + return; + } + await saveResponseResource(this, request, session); this._forgetRequest(request); } @@ -431,7 +535,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. ' + @@ -479,6 +583,24 @@ function originURL(request) { return normalizeURL((request.redirectChain[0] || request).url); } +// Convert Fetch event responseHeaders ([{name, value}, …]) to a header object. +function headersArrayToObject(arr) { + let out = {}; + if (!Array.isArray(arr)) return out; + for (let { name, value } of arr) out[name] = value; + return out; +} + +// Returns { tooLarge, malformed, rawValue } for Content-Length classification. +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, 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 }; +} + // Validate domain for auto-allowlisting feature // Only validates domains that returned 200 status async function validateDomainForAllowlist(network, hostname, url, statusCode) { @@ -557,8 +679,10 @@ async function sendResponseResource(network, request, session) { .map(([k, v]) => ({ name: k.toLowerCase(), value: String(v) })) }); } else { + // interceptResponse:true triggers a second pause at the response stage. See _handleResponsePaused. await send('Fetch.continueRequest', { - requestId: request.interceptId + requestId: request.interceptId, + interceptResponse: true }); } } catch (error) { @@ -593,9 +717,57 @@ async function sendResponseResource(network, request, session) { } } -// Make a new request with Node based on a network request +// 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; + } +} + +// 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)); +} + +// 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". async function makeDirectRequest(network, request, session) { - const { cookies } = await session.send('Network.getCookies', { urls: [request.url] }); + let cookies = []; + let cookieSession = pickCookieSession(network, session); + try { + ({ cookies } = await cookieSession.send('Network.getCookies', { urls: [request.url] })); + } catch (error) { + network.log.debug(`Network.getCookies unavailable for ${request.url}: ${error.message}`); + } let headers = { // add default browser @@ -603,7 +775,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', @@ -613,14 +785,54 @@ async function makeDirectRequest(network, request, session) { cookie: cookies.map(cookie => `${cookie.name}=${cookie.value}`).join('; ') }; - if (network.authorization?.username) { - // include basic authorization username and password + 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 }); + 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 +// never surfaces — Chrome 143+ fetches dedicated worker scripts in the browser +// process (PlzDedicatedWorker) so loadingFinished fires without a body on CDP. +async function captureResourceDirectly(network, request, session) { + let log = network.log; + let url = originURL(request); + let meta = { ...network.meta, url }; + + try { + log.debug('- Requesting resource directly (responseReceived timeout fallback)', meta); + let { body, status, headers: responseHeaders } = await raceWithTimeout( + makeDirectRequest(network, request, session), + 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', { + url, size: body.length, snapshot: meta.snapshot + }); + log.debug('- Skipping resource larger than 25MB', meta); + return; + } + + let urlObj = new URL(url); + let mimeType = resolveDirectFetchMime(responseHeaders, urlObj.origin + urlObj.pathname); + + let resource = createResource(url, body, mimeType, { + status, + headers: { 'content-type': [mimeType] } + }); + + log.debug(`- Saving direct-fetched resource sha=${resource.sha} mimetype=${mimeType}`, meta); + network.intercept.saveResource(resource); + } catch (error) { + log.debug(`Direct fetch failed for ${url} - ${error.message}`, meta); + } } // Save a resource from a request, skipping it if specific parameters are not met @@ -635,19 +847,8 @@ async function saveResponseResource(network, request, session) { url, responseStatus: response?.status }; - // Checking for content length more than 100MB, to prevent websocket error which is governed by - // maxPayload option of websocket defaulted to 100MB. - // If content-length is more than our allowed 25MB, no need to process that resouce we can return log. - let contentLength = response.headers?.[Object.keys(response.headers).find(key => key.toLowerCase() === 'content-length')]; - contentLength = parseInt(contentLength); - if (contentLength > MAX_RESOURCE_SIZE) { - logAssetInstrumentation(log, 'asset_not_uploaded', 'resource_too_large', { - url, - size: contentLength, - snapshot: meta.snapshot - }); - return log.debug('- Skipping resource larger than 25MB', meta); - } + // Oversized/malformed Content-Length is rejected earlier in _handleResponsePaused; + // the body.length check below still guards cached responses where headers may lie. let resource = network.intercept.getResource(url); if (!resource || (!resource.root && !resource.provided && disableCache)) { @@ -745,7 +946,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 dede31b93..77fa91fcb 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -1,4 +1,5 @@ import { sha256hash } from '@percy/client/utils'; +import { waitFor } from '@percy/core/utils'; import { logger, api, setupTest, createTestServer, dedent, mockRequests } from './helpers/index.js'; import Percy from '@percy/core'; import { RESOURCE_CACHE_KEY, CACHE_STATS_KEY, DISK_SPILL_KEY } from '../src/discovery.js'; @@ -459,6 +460,48 @@ describe('Discovery', () => { ])); }); + it('captures favicon when the server provides one', async () => { + server.reply('/favicon.ico', () => [200, 'image/x-icon', pixel]); + let faviconDOM = testDOM.replace('', ''); + + await percy.snapshot({ + name: 'favicon snapshot', + url: 'http://localhost:8000', + domSnapshot: faviconDOM + }); + + await percy.idle(); + + expect(captured[0]).toEqual(jasmine.arrayContaining([ + jasmine.objectContaining({ + attributes: jasmine.objectContaining({ + 'resource-url': 'http://localhost:8000/favicon.ico' + }) + }) + ])); + }); + + it('captures auto-fetched favicon when the page does not declare one', async () => { + percy.set({ discovery: { networkIdleTimeout: 1000 } }); + server.reply('/favicon.ico', () => [200, 'image/x-icon', pixel]); + + await percy.snapshot({ + name: 'auto-fetch 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`