⬆️ Bump elliptic from 6.5.2 to 6.5.3#6
Merged
Conversation
Bumps [elliptic](https://github.com/indutny/elliptic) from 6.5.2 to 6.5.3. - [Release notes](https://github.com/indutny/elliptic/releases) - [Commits](indutny/elliptic@v6.5.2...v6.5.3) Signed-off-by: dependabot[bot] <support@github.com>
dependabot
Bot
added
the
⬆️⬇️ dependencies
pranavz28
added a commit
that referenced
this pull request
Apr 22, 2026
… egress, effective cap) Addresses items 1-8 from PR #2192 review. Must-fix: #1 discovery.js: always increment unsetModeBytes; only gate the warn-log emission on warningFired. Previously the byte counter froze at the threshold crossing so cache_summary.peak_bytes misreported every Map-mode run that hit the threshold. #2 byte-lru.js: reorder ByteLRU.set — reject oversize BEFORE touching any existing entry. Fixes a failed oversize re-set silently evicting the prior (valid) value for the same key. Should-fix: #3 percy.js: move sendCacheSummary AFTER sendBuildLogs in stop()'s finally. A slow/stalled pager hop on the analytics event can no longer delay the primary log egress. #4 discovery.js: do not mutate percy.config.discovery.maxCacheRam on clamp. Store effectiveMaxCacheRamMB on CACHE_STATS_KEY; percy.js sendCacheSummary reads it from there. User config stays read-only. Nits: #5 discovery.js: read PERCY_CACHE_WARN_THRESHOLD_BYTES once at queue construction instead of on every saveResource. #6 discovery.test.js: use 'instanceof ByteLRU' (imported) instead of string match on constructor.name. #7 discovery.js: emit a debug log when PERCY_CACHE_WARN_THRESHOLD_BYTES override is active, so support can spot misconfigured overrides. #8 README: note decimal MB (1,000,000 bytes) vs binary MiB. Coverage fill-in (closes gaps visible in earlier nyc run): * byte-lru.test.js: .clear(), oversize re-set regression, onEvict reason='too-big' path. * discovery.test.js: - --max-cache-ram between 25 and 50 MB warns without clamping. - PERCY_CACHE_WARN_THRESHOLD_BYTES override emits debug log. - cache_eviction_started info log fires when LRU evicts. - unsetModeBytes keeps growing post-warningFired (regression for #1). - sendCacheSummary swallows client rejections without throwing. - sendCacheSummary short-circuits when build / cache / stats missing. Tests: 19 unit specs (byte-lru) + 13 integration specs (discovery). Lint clean. Built dist/ regenerated.
pranavz28
added a commit
that referenced
this pull request
May 6, 2026
…95] (#2192) * feat(core): add ByteLRU + entrySize helpers Hand-rolled byte-budget LRU cache backing the forthcoming --max-cache-ram flag. Pure/sync (no logger or external calls) so callers can log after .set() returns without risking event-loop yield mid-mutation. Exposes a Map-compatible surface (.get/.set/.has/.delete/.values/.size) plus .calculatedSize and .stats for telemetry. entrySize() computes body-bytes + fixed per-entry overhead, handling both single-resource entries and array-of-resources (root-resource with multiple widths from discovery.js:465). 16 unit specs covering LRU semantics, recency bump, multi-evict, oversized-entry skip, peak-bytes transient high-water, and array-entry sizing. Zero new dependencies. * feat(cli-command): add --max-cache-ram flag (MB units) New flag/env/percyrc surface for the forthcoming bounded asset-discovery cache. Value is an integer MB (e.g. --max-cache-ram 300 means 300MB). Flows through env PERCY_MAX_CACHE_RAM or percyrc discovery.maxCacheRam. Precedence follows existing Percy convention (flag > env > percyrc). Raw parse is Number; full validation happens at Percy startup once the flag is consumed (subsequent commit). * feat(core): extend discovery config schema with maxCacheRam Add maxCacheRam integer-or-null property under discovery. Value is the cap in MB (so percyrc users write 'maxCacheRam: 300' for 300MB, matching the flag). Null/unset preserves today's unbounded behavior. Schema validation catches non-integer and negative inputs at config load time; additional startup validation (e.g. minimum floor based on MAX_RESOURCE_SIZE) happens in the discovery integration commit. * feat(core): swap Map for ByteLRU when --max-cache-ram is set Replaces the unbounded resource cache Map at discovery.js:408 with a byte-budget-aware ByteLRU when percy.config.discovery.maxCacheRam is configured. Without the flag, behaviour is byte-for-byte identical to today (new Map(), no eviction). In saveResource, oversized entries (size > cap) are skipped from the global cache with a debug log but still land in snapshot.resources so the current snapshot renders correctly. ByteLRU's onEvict callback emits a debug log for each LRU eviction. Startup validation (runs once in the discovery queue's start handler): * Rejects caps below 25MB (MAX_RESOURCE_SIZE floor) with a clear error * Warns on caps below 50MB (silently-useless territory) * Info log when --max-cache-ram and --disable-cache are both set (max-cache-ram becomes a no-op) A CACHE_STATS_KEY Symbol is exported alongside RESOURCE_CACHE_KEY to hold per-run stats the telemetry layer will read in a later commit. Existing discovery tests remain green. * feat(core): add warning-at-threshold when --max-cache-ram is unset When the flag is unset (Map mode), track cumulative bytes written to the global resource cache in a side-channel counter. On first crossing of 500MB, emit a one-shot warn-level log pointing the user at the --max-cache-ram flag before the CI runner OOMs. * warn level (not info) so --quiet users still see it * one-shot guarded by a per-run stats flag — does not re-fire on shrink/regrow cycles, and is bypassed entirely when the flag is set (opt-in users do not need the discovery hint) * threshold override via PERCY_CACHE_WARN_THRESHOLD_BYTES for post-ship tuning (undocumented) This is the primary discovery mechanism for the flag — users find it through normal CI output before they need support. * feat(core): emit cache_eviction_started + cache_summary telemetry Two CLI-side events travel through the existing sendBuildEvents pipeline (UDP pager → Amplitude). No Percy API changes. * cache_eviction_started — fires exactly once per run, from ByteLRU's onEvict callback on the first LRU eviction. Payload includes the configured budget, peak bytes at eviction time, and eviction count. Emits an info log alongside telling the user eviction is active. * cache_summary — fires once per run from Percy.stop()'s finally block. Payload includes budget + hits/misses/evictions/peak_bytes/final_bytes/ entry_count/oversize_skipped. Feeds Amplitude for adoption, hit-rate, and sizing calibration metrics post-GA. Both are fire-and-forget; exceptions are logged at debug and swallowed so telemetry loss cannot fail a Percy run (same pattern as sendBuildLogs at percy.js:707). Both gate on percy.build?.id being set so they cannot emit before the build exists. * test(core): integration coverage for --max-cache-ram Two new describe blocks in discovery.test.js: 'with --max-cache-ram' (5 specs): * installs a ByteLRU when the flag is set (type + initial stats shape) * falls back to a plain Map when the flag is unset (backward compat) * rejects a cap below the 25MB MAX_RESOURCE_SIZE floor with a clear error * emits an info log when the flag and --disable-cache are both set * records oversize_skipped and leaves calculatedSize at 0 when a single entry exceeds the cap 'warning-at-threshold (unset cap)' (1 spec): * PERCY_CACHE_WARN_THRESHOLD_BYTES override triggers the warn log once; does not re-fire on a subsequent snapshot run (one-shot gate holds). Filter: --filter='max-cache-ram|warning-at-threshold' runs 6 of 149 specs in ~16s for focused iteration. Full suite stays green. * docs(core): document --max-cache-ram flag Adds maxCacheRam to the discovery options list in @percy/core README. Covers: value semantics (MB), default (unset/unbounded), eviction behavior, the cap-body-bytes vs RSS relationship users need to know for sizing, the 25 MB floor, and the three equivalent surfaces (flag / env / percyrc). * fix(core): clamp --max-cache-ram below 25MB instead of failing Follow-up to the initial --max-cache-ram implementation. Previously a value below the 25 MB MAX_RESOURCE_SIZE floor threw an error at Percy startup, killing otherwise-healthy builds for a misconfigured cap. Switch to a warn-level log and continue with the 25 MB minimum: --max-cache-ram=10MB is below the 25MB minimum (individual resources up to 25MB would otherwise be dropped). Continuing with the minimum: 25MB. Also mutates percy.config.discovery.maxCacheRam to the clamped value so the cache_summary telemetry event reports the effective cap. Updates: * discovery.js — throw → warn + clamp * discovery.test.js — integration test asserts warn log + clamped cap * README.md — docs reflect the clamp behaviour * cli-command/test/flags.test.js — stale help-text fixture: inserts the --max-cache-ram line between --disable-cache and --debug (broken since the flag was added; unrelated to the clamp, bundled here since both are help-surface / startup-UX fixes) Verified in-anger with percy snapshot --max-cache-ram 10 against https://example.com (build #356): [percy:core] --max-cache-ram=10MB is below the 25MB minimum (individual resources up to 25MB would otherwise be dropped). Continuing with the minimum: 25MB. * fix(core): address PR review (frozen counter, reorder checks, reorder egress, effective cap) Addresses items 1-8 from PR #2192 review. Must-fix: #1 discovery.js: always increment unsetModeBytes; only gate the warn-log emission on warningFired. Previously the byte counter froze at the threshold crossing so cache_summary.peak_bytes misreported every Map-mode run that hit the threshold. #2 byte-lru.js: reorder ByteLRU.set — reject oversize BEFORE touching any existing entry. Fixes a failed oversize re-set silently evicting the prior (valid) value for the same key. Should-fix: #3 percy.js: move sendCacheSummary AFTER sendBuildLogs in stop()'s finally. A slow/stalled pager hop on the analytics event can no longer delay the primary log egress. #4 discovery.js: do not mutate percy.config.discovery.maxCacheRam on clamp. Store effectiveMaxCacheRamMB on CACHE_STATS_KEY; percy.js sendCacheSummary reads it from there. User config stays read-only. Nits: #5 discovery.js: read PERCY_CACHE_WARN_THRESHOLD_BYTES once at queue construction instead of on every saveResource. #6 discovery.test.js: use 'instanceof ByteLRU' (imported) instead of string match on constructor.name. #7 discovery.js: emit a debug log when PERCY_CACHE_WARN_THRESHOLD_BYTES override is active, so support can spot misconfigured overrides. #8 README: note decimal MB (1,000,000 bytes) vs binary MiB. Coverage fill-in (closes gaps visible in earlier nyc run): * byte-lru.test.js: .clear(), oversize re-set regression, onEvict reason='too-big' path. * discovery.test.js: - --max-cache-ram between 25 and 50 MB warns without clamping. - PERCY_CACHE_WARN_THRESHOLD_BYTES override emits debug log. - cache_eviction_started info log fires when LRU evicts. - unsetModeBytes keeps growing post-warningFired (regression for #1). - sendCacheSummary swallows client rejections without throwing. - sendCacheSummary short-circuits when build / cache / stats missing. Tests: 19 unit specs (byte-lru) + 13 integration specs (discovery). Lint clean. Built dist/ regenerated. * test(core): close remaining coverage gaps Targets the branches nyc still flagged after the review-fix commit: byte-lru.js: * .delete() on a non-existent key returns false (line 66) * entrySize() handles null entries + entries without content inside an array (line 97 optional-chain branches) discovery.js: * fireCacheEventSafe's .catch debug-log path (line 440) — spy sendBuildEvents to reject, force eviction, microtask-wait * saveResource oversize-skip branch (lines 598-607) — serve a 25MB resource from the test server so the real intercept flow triggers the oversize path, not just the direct ByteLRU.set test percy.js: * sendCacheSummary entry_count '?? 0' fallback (line 409) — call directly with a defensive-shape cache lacking .size 21 unit specs + additional integration specs; lint clean. * feat(core): spill evicted resources to disk, restore on lookup Extends --max-cache-ram with a disk-backed overflow tier so evictions no longer drop resources. When the in-memory ByteLRU evicts, the full resource is written to a per-run temp directory under os.tmpdir() and a slim metadata reference stays in memory. getResource falls through RAM miss → disk tier before the browser ever refetches from origin. On any disk I/O failure we return false/undefined and fall back to the old drop behaviour — the browser refetches exactly as it would without spill, so disk-tier failure is strictly additive. What lives in byte-lru.js: - ByteLRU.onEvict(key, reason, value) — adds the evicted value so the discovery wiring can capture it before it is GC'd. - DiskSpillStore — sync mkdirSync/writeFileSync/readFileSync/rmSync. Counter-based filenames (no URL-derived data flows to path.join). Self-healing index: a read failure purges the stale entry so the next lookup cleanly misses. Best-effort destroy swallows errors. - createSpillDir — os.tmpdir()/percy-cache-<pid>-<random-hex>. - lookupCacheResource — pulled out of the inline getResource closure so the RAM-miss-to-disk-hit path is directly unit-testable. Discovery wiring (discovery.js): - start handler constructs the DiskSpillStore alongside the ByteLRU. - onEvict calls diskStore.set(key, value); debug-log differentiates `cache spill:` (success) from `cache evict:` (disk failed or disk not ready). - saveResource clears any prior disk entry up front so a fresh discovery write wins over a spilled copy — prevents a race where getResource would serve stale content. - end handler calls diskStore.destroy(); cleanup errors are swallowed by DiskSpillStore so they cannot fail percy.stop(). Telemetry (percy.js sendCacheSummary): Eight new disk-tier fields on cache_summary: disk_spill_enabled, disk_spilled_count, disk_restored_count, disk_spill_failures, disk_read_failures, disk_peak_bytes, disk_final_bytes, disk_final_entries. cache_eviction_started also carries disk_spill_enabled so the dashboard can distinguish "disabled" from "enabled with zero activity" from "enabled but failing." Tests (all pass locally): - 44 unit specs in byte-lru.test.js exercise ByteLRU, entrySize, DiskSpillStore, createSpillDir, and lookupCacheResource end-to-end — including init failure via /dev/null, write failure via mocked EACCES, read self-heal via mocked ENOENT, unlinkSync error tolerance on delete + overwrite, destroy error swallowing, the not-ready short-circuit branches, and array-root width matching. - 8 integration specs in discovery.test.js cover DiskSpillStore presence, spill-on-eviction, byte-for-byte rehydration, ENOSPC fallback, saveResource clearing stale entries, queue-end teardown (asserts both disk.ready flag and fs.existsSync for race-safety), and graceful handling when the store fails to init. Zero new npm dependencies (fs/os/path/crypto are built-ins). Node engine unchanged. Clean semgrep run on all changed files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): preserve disk-tier stats for cache_summary telemetry Real-build verification surfaced an ordering bug: percy.stop() calls discovery.end() before sendCacheSummary, and the 'end' handler destroys the DiskSpillStore and deletes percy[DISK_SPILL_KEY]. sendCacheSummary then read a null diskStore and emitted disk_spill_enabled=false with all eight disk_* fields zeroed, regardless of actual activity. A run that spilled 97 resources and restored 96 from disk shipped zeros to Percy. Snapshot the disk stats onto stats.finalDiskStats in the 'end' handler before destroy() runs. sendCacheSummary prefers the live store and falls back to the snapshot, so both the in-discovery path (existing tests) and the post-discovery path (real builds) populate the telemetry correctly. Two new specs cover (a) sendCacheSummary using the finalDiskStats fallback when DISK_SPILL_KEY is unset, and (b) the discovery 'end' handler copying diskStore.stats onto the snapshot before destroy. * fix(core): drop dead stats guard in disk-spill end handler CACHE_STATS_KEY is always set alongside DISK_SPILL_KEY in the 'start' handler, so the false branch of \`if (stats)\` was unreachable and showed up as a 99.46% branch coverage gap on discovery.js line 553. Drop the guard; write directly through the stats reference. Behavior is unchanged — coverage lands back at 100/100/100/100. * test(core): make DiskSpillStore mkdir-failure tests Windows-portable Four specs forced mkdirSync to fail by passing '/dev/null/cannot-mkdir-here'. On POSIX, /dev/null is a character device so the nested path errors with ENOTDIR. On Windows, /dev/null does not exist and mkdirSync(..., recursive) just creates the directory tree, leaving the store in the ready state and flipping every assertion. Spy on fs.mkdirSync and throw directly — same behavior on every platform, no path tricks. * test(core): drop redundant disk-stats integration test The discovery 'end' handler is already covered by 'calls diskStore.destroy ... in the queue end handler' test. Extend that existing test to also assert that stats.finalDiskStats is snapshotted before destroy clears the live store, instead of duplicating the percy.start/stop dance in a second spec. Removes a percy.stop() graceful path that doubled the file's end-to-end cost on slower runners (Windows core was hitting 6h job timeouts) without adding meaningful coverage. * refactor(core): address review nits — leak fix, dedupe, perms, drop dead code Five small follow-ups from PR review: 1. discovery 'end' handler now wraps browser.close() in try/finally so the per-run spill dir is destroyed even when the browser teardown throws, instead of leaking under os.tmpdir(). 2. fireCacheEventSafe (discovery) and sendCacheSummary (percy) shared the exact { message, cliVersion, clientInfo, extra } shape. Extracted a single sendCacheTelemetry(message, extra) instance method on Percy and route both call sites through it, so the pager schema only lives in one place. 3. DiskSpillStore mkdirSync now uses mode 0o700 — spilled bytes are origin refetchable so the threat model is small, but on shared-tenant CI hosts other users on the same box should not be able to read them. 4. ByteLRU.values() was unreferenced anywhere in src/ and only had a half-implemented iterator (no .return/.throw). Dropped it (and its test) per YAGNI; if a future caller needs Map-style iteration they can use the standard protocol on a real Map. 5. Two new unit tests: - lookupCacheResource on a disk index hit whose readFileSync throws — covers the combined self-heal + caller-refetch path that the existing isolated tests miss. - DiskSpillStore handles back-to-back saves of the same URL without doubling the index, the byte total, or leaving prior spill files on disk. Regression guard for any future parallel-discovery work. * fix(core): restore strict pre-refactor parity in cache telemetry path Two parity restores from the second-pass review on aeae5da: 1. sendCacheSummary's outer try/catch was lost when the egress moved into sendCacheTelemetry. Nothing in the payload realistically throws today, but the original contract was "nothing in sendCacheSummary can fail percy.stop()", not "nothing in the egress". Re-wrap the whole method. 2. fireCacheEventSafe lost its Promise.resolve().then(...) microtask hop; without it, sendCacheTelemetry's synchronous prelude (header build, payload serialization inside the client) ran inside the onEvict callback. The eviction event fires once per run so this is noise in practice, but the hop keeps eviction-loop timing bit-for-bit identical to the pre-refactor version. * test(core): cover sendCacheSummary payload-construction catch The outer try/catch restored in f92aee6 was uncovered (percy.js:442 showed 99.74% line coverage, blocking the 100% gate). Adds a spec that makes the cache.stats getter throw, exercising the catch and asserting sendCacheSummary still resolves and emits a 'cache_summary build failed' debug log. * test(core): assert cache_summary build catch via spy not logger mock The previous spec relied on logger.mock({ level: 'debug' }) to capture the catch-branch debug log, but logger.mock has to run before percy.start to stub the right writer — by then percy was already started in the outer beforeEach, so logger.stderr stayed empty and the assertion failed on Linux CI (it happened to look fine locally only because of preceding crashed-Chromium output bleed). Spy on percy.log.debug directly to assert the call without depending on stream capture order. * test(core): force-stop instead of graceful in disk-spill destroy spec The 'end' handler runs the same way under both stop modes, so we still exercise the destroy path + finalDiskStats snapshot. Force-stop skips the flush + saveHostnamesToAutoConfigure pipeline, which is the slow path on Windows runners and the most likely contributor to the core test job blowing past master's 40-minute baseline. * fix(core): guard fireCacheEventSafe microtask chain against unhandled rejection Node 14 (still the engine on the Windows CI runner) treats unhandled promise rejections as fatal in some configurations. sendCacheTelemetry already catches pager errors, but if its catch arm itself ever throws — e.g. a stubbed log.debug in a test — the rejection escapes the chain and can take down the runner. Trailing .catch(() => {}) closes that window without changing observable behavior. * test(core): cover fireCacheEventSafe trailing .catch The new defensive .catch on the discovery.js microtask chain was the last 0.04% gap on the function-coverage line. Drives sendCacheTelemetry to a rejection (spy + rejectWith) and lets two setImmediate ticks settle the chain — verifies the path is reachable without depending on the inner catch arm actually throwing in production. Locally green; coverage gate should clear on the next CI run. * fix(core): address PR review fixes — RAM promotion, array spill, byte-len, overcounting, schema floor Five fixes from PR #2192 review by amandeepsingh333: 1. HIGH: Promote disk-tier hits back to RAM in lookupCacheResource. A hot URL evicted once should not pay a readFileSync round-trip on every subsequent access. After disk.get returns, re-insert into ByteLRU and delete the disk entry — the LRU naturally re-evicts the actual coldest resource if room is needed. 2. HIGH: Spill multi-width root arrays via JSON+base64. Arrays of root resources (multi-width DOM snapshots) used to silently fail to spill because resource.content is undefined on arrays — fall through the `if (content == null) return false` and the whole array got dropped on eviction. Now serialize the array elementwise with binary content base64-encoded so the entire shape roundtrips through disk. 3. MEDIUM: entrySize uses Buffer.byteLength for string content. content.length on a string returns JS string units (UTF-16 code units), not bytes, so multi-byte UTF-8 (CJK, emoji) was undercounted. The cache budget could drift past its cap. 4. MEDIUM: Subtract prior entry size before overwriting in unset-cap saveResource path. Shared CSS/JS reused across many snapshots used to inflate unsetModeBytes by N× because every saveResource added entrySize without reclaiming the prior entry's footprint, triggering the 500MB warning prematurely. 5. LOW: Bump config schema maxCacheRam minimum from 0 to 1. Zero has no meaningful semantics — null means unbounded, --disable-cache means off — so reject it at schema time rather than silently bumping to 25MB via the discovery clamp. Tests: - byte-lru.test.js: UTF-8 byte-length asserts, multi-width array spill roundtrip, array-decode self-heal, JSON.stringify-throws refusal, RAM-promotion + disk-cleanup verified via two consecutive lookups with a readFileSync spy on the second. - discovery.test.js: new "does not over-count unsetModeBytes when the same URL is saved twice" regression guard for fix #4. Existing "keeps incrementing" test rewired to use a direct cache.set instead of relying on browser discovery to add new URLs. * test(core): cover byte-lru contentBytes/encode/decode fallthrough branches Adds three small tests to close the coverage gap that was failing the test:coverage gate (lines 96, 116, 125 in byte-lru.js): - contentBytes returns .length ?? 0 for non-Buffer non-string content - encodeArrayElement coerces non-Buffer non-null array content to string - decodeArrayElement passes null/string content through without decoding * test(core): cover encode/decodeArrayElement falsy-entry early-return Adds a null entry to the multi-width round-trip array so both encode/decodeArrayElement's `if (!r) return r;` short-circuits are exercised, closing the remaining branch-coverage gap on byte-lru.js (lines 112, 120). * test(core): cover saveResource race-window guard for stale disk entries Existing 'stale disk entry' test goes through lookupCacheResource's promotion path (which also disk.deletes), so saveResource's own guard at discovery.js:636 was uncovered. Force a lookup miss while the disk entry remains, so the saveResource branch fires — which is exactly the race the comment above the guard describes. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Manoj-Katta
added a commit
that referenced
this pull request
May 13, 2026
…rectly (PPLT-4214) Bundle of ce:review + PR review fixes for the v143 PlzDedicatedWorker direct-fetch fallback path in network.js. Closes the L755 coverage gap that's been blocking CI since the unstable worker-based Test D was reverted in 2d48f20. Renames: - captureScriptDirectly → captureResourceDirectly. The function captures any allowlisted resource that fell through to direct fetch, not just worker scripts; the old name was misleading. Module-private; no external callers affected. Must-fix (P1/P2 production-risk items): - Cookies: read from network.page.session (full Network domain) instead of the triggering session, since worker sessions throw "Internal error" on Network.getCookies. Defensive try/catch retained. - DIRECT_FETCH_TIMEOUT (5s) caps captureResourceDirectly via Promise.race; prevents idle() from hanging the full networkIdleWaitTimeout (~30s) when a worker host accepts TCP and stalls. (P1 #1) - makeDirectRequest now returns { body, status }; captureResourceDirectly enforces the 25MB MAX_RESOURCE_SIZE guard before saveResource and records the real HTTP status instead of hardcoded 200. Font path call site updated to destructure. (P1 #2) - Direct-fetch gate at _handleLoadingFinished mirrors saveResponseResource's disallowedHostnames-then-allowedHostnames precedence; emits a debug log when fallback is skipped due to hostname gating. (P2 #10 + C12) - Authorization header in makeDirectRequest now requires target origin to match the page's snapshot origin; prevents Basic-auth credential leak to redirected third-party origins. (P2 #11) Reviewer polish: - _handleResponsePaused malformed/oversized branch: Fetch.failRequest runs BEFORE _forgetRequest so Chrome's Fetch state can't leak paused if failRequest throws. Unknown errors trigger a last-resort Fetch.continueResponse to un-pause. Known races (ABORTED_MESSAGE / Invalid InterceptionId) remain silent. (P2 #4) - Unknown errors in _handleResponsePaused failRequest catch and _continueResponse catch now log at warn (was debug) for production observability. (P2 #5 + C8) - _handleResponsePaused: inline comment explaining when the untracked-request branch fires (service-worker-fulfilled responses or cleanup races). (C2) - _handleResponsePaused: url normalization consistent between tracked and untracked paths via normalizeURL on the untracked fallback. (C3) - parseInt now uses explicit radix 10 at both call sites. (C7) - RESPONSE_RECEIVED_TIMEOUT comment notes the cumulative N*2s worst case. (C9) Test: - New deterministic spec "logs gracefully when the direct-fetch fallback fails" exercises captureResourceDirectly's catch path by dropping Network.responseReceived for a CSS asset (no JS execution, no real worker). Closes the L755 coverage gap. (C1 / P1 #3) Intentionally deferred (reviewer comments replied separately): - P2 #15 — no-response branch flip-flop predates this PR (commit ae1d388, 2022-09-21); out of scope. - P2 #6 — sec-ch-ua hardcoded version is one of several stale literals in the makeDirectRequest header block; deferring full audit. - C4 + S4 — DISABLED_FEATURES extraction; existing block-level comment adequate. - C5 — percy.test.js timing fix; reviewer pre-approved deferring. - S2 / S3 — example / reference already present in existing comments. - Favicon Task A — pending separate investigation of snapshot.test.js timing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Manoj-Katta
added a commit
that referenced
this pull request
May 18, 2026
* chore: upgrade Chromium from v126 to v143 for asset discovery Upgrade bundled Chromium from v126.0.6478.184 to v143.0.7499.169 to match the renderer browser version. Update sec-ch-ua header from v123 to v143 in direct font request headers. PPLT-4214 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(core): ignore favicon.ico in test server request log Chrome >=128 new headless auto-requests /favicon.ico on page load, breaking discovery tests that asserted exact request counts. Filter at the helper level so all tests benefit without per-assertion churn. * fix(core): disable site isolation for Chrome 143 new headless Chrome >=128 new headless enforces site isolation via IsolateOrigins and site-per-process, putting cross-origin sub-resources and worker fetches into separate renderer processes. The existing --disable-site-isolation-trials flag only covers opt-in trials and no longer keeps everything in one renderer. Extend the existing --disable-features list with IsolateOrigins and site-per-process so the main page's Fetch.enable / Network.enable listeners continue to see cross-origin and worker traffic, matching the v126 old-headless behavior Percy relies on. Also add HttpsFirstBalancedModeAutoEnable to prevent new headless from blocking HTTP asset discovery with ERR_BLOCKED_BY_CLIENT. * chore(ci): bump cache-key for Chromium v143 upgrade Invalidates actions/cache entries that still contain the old v126 Chromium binary so CI downloads the new v143 revisions fresh. * test(core): align test helpers with Chrome new-headless behavior (PPLT-5285) Two adjustments for Chrome >=128 (new headless) without changing production code: - test server now replies 204 to the auto-fetched /favicon.ico so it doesn't leak into a snapshot's resource list and shift downstream request indices (the previous helper only filtered favicon from the request log; the browser still received the default reply and the resource still got captured). - iframe-srcdoc test regex accepts both serializations: pre-Chrome-128 left "<" / ">" literal inside attribute values, while >=128 entity-escapes them per HTML5 spec. Both are valid HTML. Fixes 4 of the v143 failures: takes additional snapshots after running each execute, runs the execute callback in the correct frame, can execute scripts at various states, can execute scripts that wait for specific states. Cluster A (cross-origin / domain-validation, 14 specs) is being tracked separately — root cause is Chrome's stricter CORS / Opaque Response Blocking; chasing a narrower fix than --disable-web-security before landing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): disable Local Network Access checks for Chrome 143 (PPLT-5284) Chrome 143 enables Local Network Access (LNA) checks by default. When asset discovery serves the root document via Fetch.fulfillRequest (the domSnapshot path), Chrome treats sub-resource requests to local network addresses (*.localhost, 127.0.0.1, RFC 1918) as needing explicit user permission. Headless can't grant the prompt, so the request fails with corsError = LocalNetworkAccessPermissionDenied and the resource is never delivered to Percy's CDP listeners. Add LocalNetworkAccessChecks to the existing --disable-features list so the fulfilled-root flow keeps loading cross-origin local sub-resources, matching v126 behavior. This is narrower than --disable-web-security: it preserves CORS / CORB / ORB enforcement on real cross-origin internet hosts and only relaxes Chrome's loopback / private-network gating, which is exactly what asset discovery needs. Resolves the 12 cross-origin and auto-domain-validation discovery test failures from PR #2187 CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(core): document Chrome 143 upgrade decisions per PR review Address inline review comments on PR #2187: - src/browser.js: split the --disable-features comment into one bullet per feature. Each bullet documents the specific Chrome >=128/143 behavior that broke and why disabling is the right fix - src/install.js: add the procedure for picking per-platform revision numbers (chromiumdash + snapshot index) so future upgrades don't have to re-derive it - test/helpers/server.js: only short-circuit /favicon.ico to 204 when the test hasn't supplied an explicit reply for it. Tests that want favicon-as-asset (none today, but future-proof) keep working via `server.reply('/favicon.ico', ...)` - test/snapshot.test.js: expand the iframe srcdoc-serialization comment with a literal example of each form so the regex's "either form" intent is obvious No behavioral change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): handle Chrome 143 split request lifecycle and malformed Content-Length (PPLT-4214) Two v143 regressions surfaced after the launch-flag fixes landed: 1. `does not capture remote files with content-length NAN greater than 25MB` — Chrome 143 won't terminate the body of a malformed `Content-Length` response, so Network.loadingFinished never fires, the page's <link href="large.css"> blocks the load event, and the navigation times out (3 retries × 30s = 93s). The existing size guard in saveResponseResource is unreachable. 2. `captures requests from workers` — for page-fetched worker scripts, Chrome 143 splits the lifecycle across two CDP sessions: the page session sees Fetch.requestPaused under requestId X, but Network.responseReceived / loadingFinished for the same script fire on the worker session under a fresh requestId Y once the worker target attaches. Percy's #requests entry under X is never cleaned up, idle() blocks indefinitely, retries fail. Confirmed via raw CDP probe (/tmp/worker-probe.mjs) — Chrome itself behaves correctly; the bug is in Percy's requestId-keyed bookkeeping. Fix: enable Fetch response-stage interception (Fetch.continueRequest with interceptResponse:true) and add _handleResponsePaused with four branches: - responseErrorReason set: confirm the failure with Fetch.failRequest so _handleLoadingFailed logs the network error as before. - Content-Length oversized or malformed: forget the request, Fetch.failRequest with 'Aborted' before the body streams. This unsticks the page load for the NaN test. - 3xx redirect: skip — _handleRequest already builds the redirect chain off the next request-stage event with redirectResponse. - Otherwise: schedule a 1s backstop that forgets the request only if Network.loadingFinished / loadingFailed haven't already done so. For normal requests this is a no-op (Network events fire within tens of ms). For worker scripts where loadingFinished never fires on the page session, this is what unblocks idle(). Trade-off: every Fetch-intercepted request now does one extra CDP roundtrip (response-stage pause -> continueResponse). Sub-millisecond per request on localhost websocket. Puppeteer and Playwright run this same dual-stage pattern. Helpers added: inspectContentLength (returns tooLarge / malformed / rawValue) and headersArrayToObject (Fetch event headers come as [{name,value}]). Constant RESPONSE_STAGE_BACKSTOP_MS = 1000. Both target tests pass in 2-3s in isolation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): preserve original errorText for response-stage Fetch errors (PPLT-4214) CI on 56306df surfaced a regression in `logs instrumentation for network errors`: when a server destroys the socket mid-response, Network.loadingFailed used to fire with errorText='net::ERR_EMPTY_RESPONSE' (or similar concrete reason), and _handleLoadingFailed logged asset_load_missing/network_error. After enabling response-stage Fetch interception, the error now surfaces first as Fetch.requestPaused with responseErrorReason set. The previous handler called Fetch.failRequest({errorReason: responseErrorReason}), which forces Chrome to fire Network.loadingFailed with errorText synthesized from that reason. For socket-destroy, responseErrorReason is the generic 'Failed', which collapses to net::ERR_FAILED — the exact errorText the asset_load_missing branch explicitly excludes, so the instrumentation was lost. Switch to Fetch.continueResponse for errored responses. This lets Chrome propagate the original errorText through Network.loadingFailed naturally, restoring the asset_load_missing log. Verified locally: - logs instrumentation for network errors: passes (3s) - captures requests from workers: still passes (4s) - does not capture remote files with content-length NAN: still passes (2s) - captures redirected resources, does not capture event-stream requests, logs failed request errors with a debug loglevel, logs unhandled response errors gracefully: all still pass Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): cover favicon capture path explicitly The other discovery specs opt out of /favicon.ico via the test server's 204 short-circuit so favicon noise doesn't shift their captured[] indices. This spec opts back in by registering an explicit /favicon.ico reply, asserting that Percy correctly captures the favicon when the server actually serves one — the production scenario where a customer's site has a real favicon. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(core): capture body at Fetch response stage instead of using a timer (PPLT-4214) Replaces the 1-second backstop timer with a principled fix: read the response body via Fetch.getResponseBody at response stage, save the resource, and forget the request — so the entire lifecycle is complete before Chrome would dispatch Network.loadingFinished. The backstop approach was a workaround that silently dropped requests where Network.loadingFinished never fired on the page session (Chrome 143's split-session worker scripts being the motivating case). The worker test passed because the orphan was deleted, but the worker script itself was never captured as a Percy resource — a real fidelity gap, even if invisible to visual diffs. Approach 2 captures the resource using the data Chrome gives us at response stage: - Errored response: continue, let Network.loadingFailed propagate the original errorText to _handleLoadingFailed (unchanged from before). - Oversized / malformed Content-Length: log skip, forget, Fetch.failRequest before body streams (unchanged from before). - Redirect (3xx): continue; _handleRequest builds the chain on the next request stage event. - Streaming (text/event-stream): continue without reading body — SSE bodies never complete and Fetch.getResponseBody would hang. - Normal: read body via Fetch.getResponseBody, build a synthetic response object, run saveResponseResource, forget the request. Network.loadingFinished still fires later for normal page-session requests but finds nothing in #requests and exits early (the existing handler already guards for this case). Other changes in this commit: - Drop RESPONSE_STAGE_BACKSTOP_MS constant and the setTimeout block. - Trim the multi-paragraph block comments in _handleResponsePaused, the helpers, and the dispatch in _handleRequestPaused down to one-liners. - Update `logs unhandled response errors gracefully` test to mock Fetch.getResponseBody (the body-read path is now Fetch-based). - Make `captures favicon when the server provides one` deterministic by adding <link rel="icon"> to the test DOM — the previous version relied on Chrome's auto-fetch which has non-deterministic timing on Windows CI. Net diff vs. backstop approach: -27 lines in network.js, +5 lines in discovery.test.js. All affected tests pass locally: - captures favicon when the server provides one (2s) - does not capture remote files with content-length NAN greater than 25MB (2s) - captures requests from workers (2s) - captures redirected resources (3s) - does not capture event-stream requests (3s) - logs failed request errors with a debug loglevel (2s) - logs instrumentation for network errors (2s) - logs unhandled response errors gracefully (2s) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): cover saveResponseResource catch and auto-fetched favicon path (PPLT-4214) Drop the dead Content-Length size check from saveResponseResource — the oversized/malformed header path is now rejected earlier in _handleResponsePaused via Fetch.failRequest, so this guard never runs. The body.length check below still covers cached responses whose headers lie about size. Add two discovery specs to keep coverage at 100%: - "logs gracefully when direct font request fails" exercises the catch in saveResponseResource by serving the font as application/octet-stream (forces makeDirectRequest) and returning 400 on the direct fetch (makes makeRequest throw without retry). - "captures auto-fetched favicon when the page does not declare one" validates that Percy captures Chrome's implicit /favicon.ico fetch. Bumps networkIdleTimeout to 1000ms so the auto-fetch lands before idle declares the network done — Chrome's auto-fetch timing varies by platform and was previously flaky on Windows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): guard Network constructor against null session.browser (PPLT-4214) The Network constructor reads page.session.browser.version.userAgent. If the session crashes mid-construction (close() sets browser to null), the read throws a TypeError that masks the real "Session crashed!" error. Add optional chaining so userAgent stays unset on the race; the next CDP call in Network.watch surfaces the real session-closed error as expected. Fixes Snapshot > handles page crashes on Linux CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): cover untracked-request branch and simplify error message read (PPLT-4214) - Add discovery spec that emits a response-stage Fetch.requestPaused for a request that was never tracked at the request stage. Exercises the defensive null-request branches in _handleResponsePaused (line 236, 254, 278-282). - Simplify error?.message ?? '' to error && error.message ? error.message : '' in browser.js handleError so all branches are reachable from existing launch-failure tests. Behaviorally identical for any realistic input. - Update install.js comment to point to the browseable index.html for Chromium snapshots. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): stabilize two timing-fragile tests under suite load (PPLT-4214) - Replace the 500ms polling loop in 'returns the same promise for concurrent validation requests to the same hostname' with a deterministic Promise that the validation mock resolves on its first call. The hardcoded ceiling was too tight for a loaded CI suite and the v143 response-stage refactor's per-request overhead pushed the test over the edge consistently. - Attach a no-op catch on the sync-job promise in 'handleSyncJob should handle promise reject' so the rejection isn't reported as unhandled in the race window between snapshot returning and handleSyncJob attaching its own handler. expectAsync still observes the rejection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): attach catch handler synchronously on sync-snapshot promise (PPLT-4214) The previous fix attached .catch() after percy.snapshot returned, but percy.js:495 creates and assigns the upload promise synchronously inside the snapshot call, and the upload can reject before the test's next line runs. Use a setter on the promise property so the no-op catch is attached the instant percy assigns the promise — eliminating the race window. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): bump waitForTimeout to give failing upload reliable headroom (PPLT-4214) The 'stops accepting snapshots when an in-progress build fails' spec relied on a 100ms waitForTimeout to keep snapshot-2 in discovery while snapshot-1's upload failed. The v143 response-stage refactor makes the suite ~50% slower under CI load, so 100ms is no longer enough — both uploads were getting through. Bump to 1000ms. The race window the test exercises is the same; we just give the failing upload more time to be observed before the second snapshot leaves discovery. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): cover oversized/malformed branch for untracked requests (PPLT-4214) Adds a discovery spec that emits a response-stage Fetch.requestPaused with a malformed Content-Length header for a request that was never tracked at the request stage. This exercises the if (request) false branch inside _handleResponsePaused's oversized/malformed handler — line 254 was flagged as a coverage gap on CI after the response-stage refactor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): timeout responseReceived wait so worker scripts don't leak (PPLT-4214) Root cause: Chrome 143 doesn't fire Network.responseReceived for worker scripts — only requestWillBeSent and loadingFinished. Percy's _handleLoadingFinished awaited responseReceived unconditionally, blocking forever and leaking the request in #requests so idle() never returned. Fix: - Cap the responseReceived wait in _handleLoadingFinished at 2s. When responseReceived never fires (worker scripts), forget the request without saving rather than blocking. - Simplify _handleResponsePaused: keep response-stage interception only for the malformed/oversized Content-Length abort. Stop reading the body at response stage — let it be captured later via the v126 path (Network.getResponseBody after loadingFinished). The body-read at response stage was added in bd44f96 as a workaround for the leak above; the proper fix is the timeout. Test fix: 'logs unhandled response errors gracefully' now mocks Network.getResponseBody (the actual call site after this change), not Fetch.getResponseBody. Verified locally: - captures requests from workers: 5s (was 90s timeout) - does not capture remote files with content-length NAN: 4s - 3 content-length casing variants: pass - logs instrumentation for network errors / 5xx / empty: pass - captures redirected resources: pass - does not capture event-stream requests: pass - captures stylesheet initiated fonts: pass - infers mime type when CDP returns text/plain: pass - captures responsive assets / responsive srcset+mediaquery: pass - handles page crashes: pass - captures with valid auth credentials + 3 font-auth variants: pass - handleSyncJob / in-progress build fails / concurrent validation: pass Auth test 'does not capture without auth credentials' still slow at 1m34s — that's a fundamental Chrome 143 behavior (load event doesn't fire after auth cancel), not fixable in Percy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): drop trailing blank line that broke yarn lint (PPLT-4214) CI flagged no-multiple-empty-lines at discovery.test.js:3817 — leftover from removing the failing browser.js coverage test attempt earlier. Verified yarn lint clean locally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): make browser-launch failure path testable across Chrome versions (PPLT-4214) Pass a real Error from handleExitClose so the rejection has one shape on every code path; switch the failing-launch test to --version, which makes Chrome exit deterministically without depending on its arg-parsing quirks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): make worker-script responseReceived timeout observable (PPLT-4214) Lift the 2s cap to a named constant, clear the timer on the responseReceived-wins path, and log a debug line when the cap fires so a regression of the v143 worker-script hang would surface in CI rather than silently dropping the resource. Asserts the log in the existing worker test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): use platform-specific early-exit arg in launch-failure test (PPLT-4214) Chrome 143 with --version exits cleanly on Linux/macOS but keeps running on Windows. Use --remote-debugging-port=null on Windows (still exits there, per original v126 behavior) and --version elsewhere so each runner exercises the handleExitClose path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): capture v143 worker scripts via direct HTTP fallback (PPLT-4214) Chrome 143+ fetches dedicated worker scripts in the browser process (PlzDedicatedWorker) and never surfaces the response on any CDP session, so the bytes can't be retrieved via Network.getResponseBody. For enableJavaScript:true snapshots the cloud renderer needs the script to construct the worker. Fall back to a direct HTTP fetch (same pattern makeDirectRequest already uses for fonts) when the responseReceived timeout fires for a Script-type request, and assert worker.js is captured in the existing worker test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): widen v143 worker-script fallback for PlzDedicatedWorker (PPLT-4214) Two issues left worker.js missing from the snapshot resource list on Linux + Windows CI even after the direct-HTTP fallback in 9adeecd: 1. Chrome 143 PlzDedicatedWorker reports the worker script's `resourceType` as 'Other', not 'Script'. The strict `request.type === 'Script'` gate skipped the fallback. Replace it with an `allowedHostnames` filter so the gate matches what the snapshot would have captured anyway, regardless of how Chrome classifies the request type. 2. `Network.loadingFinished` for the worker script fires on the *worker* CDP session in v143. `Network.getCookies` on the worker session throws "Internal error", which made `makeDirectRequest` throw silently inside the fallback. Wrap the cookies call in try/catch and proceed with an empty cookies array. Page-session callers (fonts) are unaffected. Also derive the resource mimetype from the URL via `mime.lookup()` instead of hardcoding `application/javascript`, so any timed-out allowed-hostname resource is saved with the correct mimetype. Verified locally: focused worker test passes in 5s. All discovery and network specs pass on macOS Node 25 (the 27 unrelated failures in the local full run are Node 25-only mock issues in install/doctor tests; CI runs Node 14 where they pass). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(core): remove unreachable no-response branch in saveResponseResource (PPLT-4214) The `if (!response)` block was dead code: saveResponseResource is only called from _handleLoadingFinished after the `if (!request.response)` early-return guards against a null response, so the inner check could never fire. The pre-existing `/* istanbul ignore if */` was masking the unreachability; deleting it surfaces the cleaner control flow. CI flagged this line as the only remaining coverage gap on Linux after the worker-test fix in 37f5176. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Revert "refactor(core): remove unreachable no-response branch in saveResponseResource (PPLT-4214)" This reverts commit ec13879. * test(core): cover network.js race-catch branches; drop istanbul annotations (PPLT-4214) Drops the two `/* istanbul ignore next: race with abort/close */` comments in _handleResponsePaused (Fetch.failRequest catch) and _continueResponse (Fetch.continueResponse catch), and backs each branch with a real test. New specs under "Discovery / with resource errors": - logs gracefully when Fetch.failRequest fails during malformed-CL abort - silently swallows Fetch.continueResponse benign races (ABORTED_MESSAGE) - logs gracefully when Fetch.continueResponse fails with unexpected error - logs gracefully when direct worker-script fetch fails Together with the existing untracked-request specs, every branch in the two catch blocks plus the captureScriptDirectly catch is now exercised without `istanbul ignore`, addressing the post-merge coverage gap on network.js (previously L317, L755). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): drop CI-unstable direct worker-script direct-fetch spec (PPLT-4214) The companion to c20c581's network.js cleanup, "logs gracefully when direct worker-script fetch fails", passed locally on macOS Node 25 but failed on CI Linux Node 14 because its worker scaffolding (real `new Worker()` + `waitForSelector: '.done'`) interacted badly with PlzDedicatedWorker when /worker.js had to return 200 on the first hit and 400 on the second. The worker initialization never completed, captureScriptDirectly was never reached, the expected debug log never appeared. The other three new specs from c20c581 (failRequest unexpected error, continueResponse ABORTED_MESSAGE silent return, continueResponse unexpected error) all pass on CI Linux and are kept. Network.js:755 returns to "uncovered" status; a deterministic test for that catch (no JS execution, no real worker) will land in a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): harden v143 direct-fetch fallback + rename captureScriptDirectly (PPLT-4214) Bundle of ce:review + PR review fixes for the v143 PlzDedicatedWorker direct-fetch fallback path in network.js. Closes the L755 coverage gap that's been blocking CI since the unstable worker-based Test D was reverted in 2d48f20. Renames: - captureScriptDirectly → captureResourceDirectly. The function captures any allowlisted resource that fell through to direct fetch, not just worker scripts; the old name was misleading. Module-private; no external callers affected. Must-fix (P1/P2 production-risk items): - Cookies: read from network.page.session (full Network domain) instead of the triggering session, since worker sessions throw "Internal error" on Network.getCookies. Defensive try/catch retained. - DIRECT_FETCH_TIMEOUT (5s) caps captureResourceDirectly via Promise.race; prevents idle() from hanging the full networkIdleWaitTimeout (~30s) when a worker host accepts TCP and stalls. (P1 #1) - makeDirectRequest now returns { body, status }; captureResourceDirectly enforces the 25MB MAX_RESOURCE_SIZE guard before saveResource and records the real HTTP status instead of hardcoded 200. Font path call site updated to destructure. (P1 #2) - Direct-fetch gate at _handleLoadingFinished mirrors saveResponseResource's disallowedHostnames-then-allowedHostnames precedence; emits a debug log when fallback is skipped due to hostname gating. (P2 #10 + C12) - Authorization header in makeDirectRequest now requires target origin to match the page's snapshot origin; prevents Basic-auth credential leak to redirected third-party origins. (P2 #11) Reviewer polish: - _handleResponsePaused malformed/oversized branch: Fetch.failRequest runs BEFORE _forgetRequest so Chrome's Fetch state can't leak paused if failRequest throws. Unknown errors trigger a last-resort Fetch.continueResponse to un-pause. Known races (ABORTED_MESSAGE / Invalid InterceptionId) remain silent. (P2 #4) - Unknown errors in _handleResponsePaused failRequest catch and _continueResponse catch now log at warn (was debug) for production observability. (P2 #5 + C8) - _handleResponsePaused: inline comment explaining when the untracked-request branch fires (service-worker-fulfilled responses or cleanup races). (C2) - _handleResponsePaused: url normalization consistent between tracked and untracked paths via normalizeURL on the untracked fallback. (C3) - parseInt now uses explicit radix 10 at both call sites. (C7) - RESPONSE_RECEIVED_TIMEOUT comment notes the cumulative N*2s worst case. (C9) Test: - New deterministic spec "logs gracefully when the direct-fetch fallback fails" exercises captureResourceDirectly's catch path by dropping Network.responseReceived for a CSS asset (no JS execution, no real worker). Closes the L755 coverage gap. (C1 / P1 #3) Intentionally deferred (reviewer comments replied separately): - P2 #15 — no-response branch flip-flop predates this PR (commit ae1d388, 2022-09-21); out of scope. - P2 #6 — sec-ch-ua hardcoded version is one of several stale literals in the makeDirectRequest header block; deferring full audit. - C4 + S4 — DISABLED_FEATURES extraction; existing block-level comment adequate. - C5 — percy.test.js timing fix; reviewer pre-approved deferring. - S2 / S3 — example / reference already present in existing comments. - Favicon Task A — pending separate investigation of snapshot.test.js timing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(core): revert log.warn promotion in CDP-race catches; satisfy lint Two related CI fixes for the previous commit: 1. Revert log.warn -> log.debug in _handleResponsePaused's failRequest catch and _continueResponse's catch. The promotion was too aggressive: it treated Session closed / Target closed (lifecycle teardown races) as unknown errors and fired a warn line, breaking exact-stderr-equality assertions in tests like snapshot.test.js's `handles the page closing early` (which intentionally tears down the browser mid-fetch) and producing noise on routine percy.stop() teardown for customers. The other parts of the earlier reorder commit are kept: - Fetch.failRequest BEFORE _forgetRequest (lifecycle correctness) - Last-resort Fetch.continueResponse on unknown failRequest errors - Distinction between known-race silent return vs unknown-error log Production observability for stuck-snapshot incidents will be addressed in a follow-up via instrumentation telemetry rather than user-facing logs. 2. Convert the targetDOM template literal in `logs gracefully when the direct-fetch fallback fails` to a single-quoted string. The template literal had no interpolation and tripped ESLint's quotes rule (single-quote preference). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(core): close network.js coverage gap; mark unreachable else (PPLT-4214) Covers the two integration-testable uncovered lines in `network.js`: - `Network.getCookies` failure catch in `makeDirectRequest` - 25 MB direct-fetch body-size guard in `captureResourceDirectly` The third uncovered branch (the hostname-not-allowed skip in `_handleLoadingFinished`) only fires for PlzDedicatedWorker requests whose worker-script fetch bypasses `Fetch.requestPaused`. Cross-origin assets loaded via the document session still flow through `sendResponseResource`, so the integration test harness can't reliably exercise that branch. Annotated with `/* istanbul ignore else */` plus a justification comment so the gate stays at 100%. * style(core): drop padded blank line at end of resource-errors describe * refactor(core): extract direct-fetch helpers for clean coverage (PPLT-4214) Extracts two pure helpers from `makeDirectRequest` so the previously uncovered branches become unit-testable without `/* istanbul ignore */` annotations: - `pickCookieSession(network, session)` — picks the page's full Network domain when set, falls back to the request's own session otherwise. - `shouldAttachAuth(authorization, requestUrl, snapshotUrl)` — re-enforces the same-origin rule for Node-side Basic auth (cross-origin or malformed URLs return false defensively). `makeDirectRequest` now reads as straight-line composition: pick session → fetch cookies → assemble headers → `if (shouldAttachAuth(...))` attach. Tests: - 7 unit tests in `test/unit/network.test.js` cover every branch of both helpers (page-set, page-undefined, page-no-session, no-auth, empty-auth, same-origin, cross-origin, malformed URLs). - 1 integration test in `test/discovery.test.js` covers the mime-lookup fallback to `application/javascript` for direct-fetch URLs with no recognizable extension. Behavior is byte-identical to the previous inline implementation. * refactor(core): extract raceWithTimeout helper for clean coverage (PPLT-4214) Same pattern as pickCookieSession / shouldAttachAuth: lifts the Promise.race + setTimeout dance out of captureResourceDirectly into an exported pure helper so its branches become unit-testable. The setTimeout callback (only fires on a 5s+ direct-fetch stall) was the last remaining function-coverage gap in network.js — no realistic integration test stalls a fetch long enough to trip it. With the helper unit-tested via a 10ms timeout, the gap closes without an istanbul-ignore marker. Side benefit: the outer try/finally in captureResourceDirectly no longer needs to clear the timer (the helper does it internally), which slightly simplifies the function. Tests: 3 unit tests in test/unit/network.test.js cover all 3 outcomes of raceWithTimeout (resolves-before-timeout, rejects-on-timeout, inner-rejection-propagates). * fix(core): honor server Content-Type on direct-fetch; safer mime fallback (PPLT-4214) Previously, captureResourceDirectly derived mimetype from URL extension alone, falling back to 'application/javascript' for extensionless paths. That default is the worst wrong guess — extensionless REST endpoints (/api/data, worker registries) often serve JSON/HTML/binary, and treating those as JS produces confusing renderer parse errors. New precedence in captureResourceDirectly: 1. HTTP Content-Type response header from the direct fetch 2. mime.lookup() on the URL extension 3. application/octet-stream (safe binary default, never JS) makeDirectRequest's makeRequest callback now also returns `headers` so captureResourceDirectly can read the server's Content-Type. The font path in saveResponseResource continues to destructure { body } only and is unaffected. Test: the unknown-extension integration spec now asserts the server's declared 'application/octet-stream' is preserved (was previously asserting the JS fallback that this commit removes). * refactor(core): extract DISABLED_FEATURES + resolveDirectFetchMime helpers (PPLT-4214) Two minimal changes: 1. browser.js — pull the inline `--disable-features=` string into a `DISABLED_FEATURES` array with a per-entry comment for each flag. Typos in feature names are now diff-visible; security-boundary flags (IsolateOrigins, site-per-process) are explicitly tagged as headless-only. No behavior change. 2. network.js — extract the direct-fetch mimetype resolution into an exported `resolveDirectFetchMime(responseHeaders, urlForLookup)` helper. Same playbook as pickCookieSession / shouldAttachAuth / raceWithTimeout: pure function, all branches unit-testable without integration test gymnastics. Closes the last `network.js` branch coverage gap (line 822's optional-chain F-branch). 3. network.js — RESPONSE_RECEIVED_TIMEOUT comment updated to mention PERCY_NETWORK_IDLE_WAIT_TIMEOUT as the cumulative safety valve. Tests: 3 unit tests in test/unit/network.test.js cover all branches of resolveDirectFetchMime (server-MIME bare, server-MIME with charset parameter, URL-extension fallback, application/octet-stream fallback, empty content-type). --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bumps elliptic from 6.5.2 to 6.5.3.
Commits
86478036.5.3856fe4dsignature: prevent malleability and overflowsDependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)You can disable automated security fix PRs for this repo from the Security Alerts page.