fix(core): allow PERCY_GZIP to bypass raw-size cap on asset discovery (PER-8648)

[pranavz28]

Fixes PER-8648.

Problem

Resources whose raw bytes exceed MAX_RESOURCE_SIZE (~15.75 MB) are rejected at network capture time in network.js, even when PERCY_GZIP=true is set. Because discovery.js only gzips resources after they've been added to resources[], PERCY_GZIP could never compress a resource that the size cap rejected first — making the env var effectively useless for the case it is most needed: large AEM/Sitecore clientlib-* CSS bundles on enterprise sites.

Real customer impact today on PER-8648: cooperlighting.com (Signify, AEM) has a single 29.3 MB clientlib-main.min.ACSHASH<hash>.css that Percy can never capture. The renderer falls back to fetching from origin, but the AEM ACSHASH cache-busts on every publish — by render time the hash has rolled and origin returns 404 — so the page renders unstyled. None of the previously-suggested workarounds (PERCY_GZIP, enableJavaScript, visual_allow_all_hostname) bypass network.js:643 by design.

Change

packages/core/src/network.js

• New shouldSkipForSize(size) helper. When PERCY_GZIP is set it relaxes the raw cap to an 80 MB ceiling (kept well under the 100 MB CDP WebSocket payload limit, bounds CLI memory). Without PERCY_GZIP, behavior is unchanged — still MAX_RESOURCE_SIZE.
• All four > MAX_RESOURCE_SIZE sites (inspectContentLength, captureResourceDirectly, saveResponseResource body check, and via tooLarge in the Fetch interceptor) now go through shouldSkipForSize.
• Log message text (- Skipping resource larger than 25MB) and the structured asset_not_uploaded / resource_too_large instrumentation are preserved — downstream parsers and existing tests are unaffected.

packages/core/src/discovery.js

• In the existing PERCY_GZIP loop, after gzipping each resource, drop it if the gzipped bytes still exceed MAX_RESOURCE_SIZE. This preserves the original guarantee on the upload payload (oversized resources cannot reach the server) and surfaces the rejection in CLI logs at the gzip stage instead of at capture time.

Tests added (packages/core/test/discovery.test.js)

• captures resource larger than 25MB raw when PERCY_GZIP is enabled — 30 MB text/css of 'A'.repeat(30_000_000) is captured into the snapshot, gzipped, and uploaded.
• still skips resource above PERCY_GZIP raw-size ceiling — 90 MB resource is still rejected even with PERCY_GZIP=true (verifies the ceiling).
• All existing size-cap tests (does not capture large files, checks if no header is send, does not capture remote files with content-length NAN…, the three Content-Length casing cases, skips file greater than 100MB) continue to pass unchanged — shouldSkipForSize falls through to the original MAX_RESOURCE_SIZE check when PERCY_GZIP is unset.

Verification (live builds on prod Percy)

Same Python Playwright snapshot of https://www.cooperlighting.com/global, identical except for PERCY_GZIP:

	Baseline (no PERCY_GZIP) — build 50211556	Fix path (PERCY_GZIP=true) — build 50211623
CLI log for 29.3 MB clientlib-main.min.ACSHASH<hash>.css	[ASSET_NOT_UPLOADED] Resource too large → skipped	Processing resource: …/clientlib-main.min.ACSHASH<hash>.css ✅
Renderer head status on the CSS	404 (origin fallback failed on cache-busted hash)	bundled in snapshot — fallback not needed
Renderer success rate	91.7% (1 critical failure)	100.0%
Post-gzip cap rejections (new check)	n/a	0 (29.3 MB gzipped → ~30 KB, well under cap)

Discussion / alignment in Slack thread above with Ninad. cc @rishigupta1599 (author of the original c0cafd33 / PR #1803 that tightened the cap to account for base64 inflation), @rahul-barnwal, @aryan2611.

🤖 Generated with Claude Code

[pranavz28]

Pushed review-fix commit (5f5a744) addressing P1/P2 findings from /ce:review:

P1 (production safety):

• Ceiling lowered 80 MB → 50 MB raw. CDP returns binary bodies as base64 (+33%), so 80 MB became ~107 MB on the wire — over the ws library's 100 MB maxPayload default. 50 MB raw → ~67 MB on the wire, comfortably under. (Final value pending storage-cost review.)
• browser.js: explicit maxPayload: 150 MB on the CDP WebSocket so it survives base64+framing for any body up to the new ceiling.
• Pako.gzip + sha now wrapped in try/catch per resource — one bad resource can no longer abort the whole snapshot.
• DIRECT_FETCH_TIMEOUT scaled to 30 s when PERCY_GZIP=true (was 5 s — far too short for tens of MB).

P2 (correctness + observability):

• response.buffer() wrapped in raceWithTimeout (30 s / 60 s under PERCY_GZIP) — a hung CDP socket can no longer stall the discovery queue indefinitely.
• Direct-fetch catch and buffer-fetch catch emit logAssetInstrumentation('asset_load_missing', 'network_error', …) so support tools (analyse_build, analyze_cli_logs_tool) see the failure instead of just a debug log.
• Post-gzip drop in discovery.js now emits the same structured [ASSET_NOT_UPLOADED] resource_too_large signal as every other size-skip site.
• Root and log resources are exempted from the post-gzip drop — shipping an oversized root and letting the API surface a clear error is safer than silently producing a broken snapshot.
• MAX_RESOURCE_SIZE is now exported from network.js and imported in discovery.js — single source of truth, eliminates the drift hazard.
• Constant renamed MAX_RESOURCE_SIZE_GZIP_CEILING → MAX_RAW_RESOURCE_SIZE_WITH_GZIP (it gates raw bytes, not gzipped output — the previous name read as the opposite).
• Log message "Skipping resource larger than 25MB" → "Skipping resource larger than allowed size" (the previous wording was factually wrong under PERCY_GZIP; the actual size is already in the structured logAssetInstrumentation payload).
• ByteLRU cache floor: minimum --max-cache-ram is now 50 MB when PERCY_GZIP is set (was hard-coded 25 MB; large raw bodies were being silently dropped from the cache, forcing re-gzip on every snapshot).

Tests:

• New: drops resource post-gzip when gzipped size still exceeds the cap — uses a 20 MB crypto.randomBytes payload (incompressible), exercising the new discovery.js post-gzip branch that the previous tests bypassed.
• All Skipping resource larger than 25MB assertions updated to the new wording.
• afterEach now delete process.env.PERCY_GZIP so state can't leak between tests.

Re-verified end-to-end: cooperlighting.com build 50215292 — clientlib-main.min.ACSHASH<hash>.css (29.3 MB) is processed into the resource bundle, no [ASSET_NOT_UPLOADED] for it; only the unrelated Signify CDN HTTP 206 entries remain (separate, pre-existing issue).

Knowingly deferred (with notes):

• Synchronous Pako.gzip on large bodies still blocks the event loop (~300–500 ms for a 30–40 MB body). Swapping to node:zlib.gzip is async/off-thread but would require making processSnapshotResources async — out of scope for this PR. Tracking as follow-up.
• await response.buffer() materializes body into V8 heap before any --max-cache-ram accounting. With the new 50 MB ceiling + default concurrency 10, peak transient heap can reach ~500 MB. Real-world impact bounded by discovery.concurrency and snapshot composition. Worth a follow-up to do Content-Length-based budgeting before buffering.

🤖 Generated with Claude Code

[pranavz28]

PR Review — PER-8648

Summary: Allows PERCY_GZIP runs to capture raw asset bodies up to 50 MB (instead of the ~15.75 MB base64-adjusted cap), gzips them during snapshot processing, then re-applies the upload cap on the compressed size. Also raises the WebSocket maxPayload, adds timeouts around body buffering and direct fetches, and adds structured instrumentation on failure paths.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets/credentials	✅ Pass	None introduced
High	Security	AuthN/AuthZ checks present	N/A	No auth surface touched
High	Security	Input validation & sanitization	✅ Pass	shouldSkipForSize guards non-finite sizes; size caps bound memory
High	Security	No IDOR	N/A
High	Security	No SQL injection	N/A
High	Correctness	Logic correct, edge cases	✅ Pass	gzip try/catch, root/log resources preserved, finite-size guard all sound
High	Correctness	Explicit error handling, no swallowed exceptions	✅ Pass	42edeb24 fixed swallowed response.buffer() errors; timeouts propagate to outer catch
High	Correctness	No race conditions/concurrency	✅ Pass	raceWithTimeout clears its timer via .finally()
Medium	Testing	New code has tests	✅ Pass	Specs for raw-bypass, post-gzip drop, gzip throw, longer timeout, cache floor
Medium	Testing	Error/edge paths tested	✅ Pass	gzip-throws + timeout branches covered
Medium	Testing	Existing tests pass (no regressions)	✅ Pass	CI green
Medium	Performance	No unbounded data fetching	✅ Pass	This PR adds the timeouts that bound buffering
Medium	Performance	Long tasks backgrounded	N/A
Medium	Quality	Follows codebase patterns	✅ Pass	Reuses logAssetInstrumentation, existing constants style
Medium	Quality	Focused/single concern	✅ Pass	All scoped to PER-8648
Low	Quality	Meaningful names, no dead code	✅ Fixed	MAX_CDP_PAYLOAD was exported-but-unused → now wired up
Low	Quality	Comments explain why	✅ Pass	Comments are genuinely explanatory
Low	Quality	No unnecessary deps	✅ Pass	No new deps

Findings (both addressed in 004724e)

1 — Dead constant (Low): MAX_CDP_PAYLOAD (network.js) was exported but never referenced; the real ws limit lived as a magic literal 150 * (1024 ** 2) in browser.js. → Set MAX_CDP_PAYLOAD = 150 MB and browser.js now imports and uses it, so the constant and the literal can't drift.

2 — Stale comment (Low): The network.js comment said "keep the raw ceiling small enough … under [the 100 MB default]", but browser.js overrides the default to 150 MB. → Comment now accurately states the 100 MB default is raised to MAX_CDP_PAYLOAD.

Observations (non-blocking)

• Two cap semantics (intentional): network.js caps the raw body at 50 MB during capture; discovery.js caps the gzipped size at MAX_RESOURCE_SIZE (~15.75 MB). Text compresses and is kept; incompressible 15–50 MB assets are captured then dropped post-gzip — slightly wasteful but correct and well-commented.
• Known TODO: The 50 MB ceiling is documented as "pending a storage-cost review (PER-8648)" — fine as a documented interim value.

Verdict

✅ Approve — solid, well-tested fix with correct error propagation and bounded memory. The two Low-severity cleanups have been applied.

_{🤖 Generated with Claude Code}

[prklm10]

we can add one log line after we set resouce sha that resource has been zipped

[pranavz28]

Added a debug log line right after the SHA is set: log.debug(- Gzipped resource: ${resource.url}). Pushed in 18feb25.