From 537b336f0af273ccc86ef4cc36c2deaf95123b06 Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Wed, 22 Apr 2026 12:49:37 +0530 Subject: [PATCH 01/14] 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. --- packages/core/src/cache/byte-lru.js | 98 +++++++++++++ packages/core/test/unit/byte-lru.test.js | 178 +++++++++++++++++++++++ 2 files changed, 276 insertions(+) create mode 100644 packages/core/src/cache/byte-lru.js create mode 100644 packages/core/test/unit/byte-lru.test.js diff --git a/packages/core/src/cache/byte-lru.js b/packages/core/src/cache/byte-lru.js new file mode 100644 index 000000000..f26755232 --- /dev/null +++ b/packages/core/src/cache/byte-lru.js @@ -0,0 +1,98 @@ +// Hand-rolled byte-budget LRU cache. Map insertion order is LRU order; +// .get() deletes and re-sets to move an entry to MRU. Synchronous by design — +// no logger calls, no awaits inside mutation paths, so callers can log +// after .set() returns without risking a mid-state event-loop yield. + +const DEFAULT_PER_ENTRY_OVERHEAD = 512; + +export class ByteLRU { + #map = new Map(); + #bytes = 0; + #max; + #stats = { hits: 0, misses: 0, evictions: 0, peakBytes: 0 }; + onEvict; + + constructor(maxBytes, { onEvict } = {}) { + this.#max = maxBytes; + this.onEvict = onEvict; + } + + get(key) { + if (!this.#map.has(key)) { + this.#stats.misses++; + return undefined; + } + const rec = this.#map.get(key); + this.#map.delete(key); + this.#map.set(key, rec); + this.#stats.hits++; + return rec.value; + } + + set(key, value, size) { + if (!Number.isFinite(size) || size < 0) return false; + + if (this.#map.has(key)) { + this.#bytes -= this.#map.get(key).size; + this.#map.delete(key); + } + + if (this.#max !== undefined && size > this.#max) { + if (this.onEvict) this.onEvict(key, 'too-big'); + return false; + } + + this.#map.set(key, { value, size }); + this.#bytes += size; + if (this.#bytes > this.#stats.peakBytes) this.#stats.peakBytes = this.#bytes; + + while (this.#max !== undefined && this.#bytes > this.#max) { + const oldestKey = this.#map.keys().next().value; + const rec = this.#map.get(oldestKey); + this.#bytes -= rec.size; + this.#map.delete(oldestKey); + this.#stats.evictions++; + if (this.onEvict) this.onEvict(oldestKey, 'lru'); + } + + return true; + } + + has(key) { return this.#map.has(key); } + + delete(key) { + if (!this.#map.has(key)) return false; + this.#bytes -= this.#map.get(key).size; + return this.#map.delete(key); + } + + clear() { + this.#map.clear(); + this.#bytes = 0; + } + + values() { + const iter = this.#map.values(); + return { + next: () => { + const r = iter.next(); + return r.done ? r : { value: r.value.value, done: false }; + }, + [Symbol.iterator]() { return this; } + }; + } + + get size() { return this.#map.size; } + get calculatedSize() { return this.#bytes; } + get stats() { return { ...this.#stats, currentBytes: this.#bytes }; } +} + +// Compute the byte size attributable to a cache entry. Handles the two Percy +// shapes: a single resource object, or an array of resources (root-resource +// captured at multiple widths per discovery.js:465). +export function entrySize(resource, overhead = DEFAULT_PER_ENTRY_OVERHEAD) { + if (Array.isArray(resource)) { + return resource.reduce((n, r) => n + (r?.content?.length ?? 0) + overhead, 0); + } + return (resource?.content?.length ?? 0) + overhead; +} diff --git a/packages/core/test/unit/byte-lru.test.js b/packages/core/test/unit/byte-lru.test.js new file mode 100644 index 000000000..b8f14325f --- /dev/null +++ b/packages/core/test/unit/byte-lru.test.js @@ -0,0 +1,178 @@ +import { ByteLRU, entrySize } from '../../src/cache/byte-lru.js'; + +describe('Unit / ByteLRU', () => { + describe('unbounded mode (no cap)', () => { + it('behaves like a Map', () => { + const c = new ByteLRU(); + c.set('a', { body: 'A' }, 100); + c.set('b', { body: 'B' }, 200); + expect(c.size).toEqual(2); + expect(c.get('a')).toEqual({ body: 'A' }); + expect(c.calculatedSize).toEqual(300); + }); + }); + + describe('eviction', () => { + it('evicts LRU when adding over-budget entry', () => { + const c = new ByteLRU(300); + c.set('a', 'A', 100); + c.set('b', 'B', 100); + c.set('c', 'C', 100); + c.set('d', 'D', 100); + expect(c.has('a')).toBe(false); + expect(c.has('d')).toBe(true); + expect(c.calculatedSize).toEqual(300); + }); + + it('evicts multiple entries if new one needs room', () => { + const c = new ByteLRU(500); + c.set('a', 'A', 100); + c.set('b', 'B', 100); + c.set('c', 'C', 100); + c.set('d', 'D', 100); + c.set('e', 'E', 100); + c.set('big', 'BIG', 300); + expect(c.has('a')).toBe(false); + expect(c.has('b')).toBe(false); + expect(c.has('c')).toBe(false); + expect(c.has('d')).toBe(true); + expect(c.has('e')).toBe(true); + expect(c.has('big')).toBe(true); + expect(c.calculatedSize).toEqual(500); + }); + }); + + describe('recency', () => { + it('.get() bumps recency', () => { + const c = new ByteLRU(300); + c.set('a', 'A', 100); + c.set('b', 'B', 100); + c.set('c', 'C', 100); + c.get('a'); + c.set('d', 'D', 100); + expect(c.has('a')).toBe(true); + expect(c.has('b')).toBe(false); + }); + + it('re-set same key updates size & recency', () => { + const c = new ByteLRU(500); + c.set('a', 'A1', 100); + c.set('b', 'B', 100); + c.set('a', 'A2', 200); + expect(c.get('a')).toEqual('A2'); + expect(c.calculatedSize).toEqual(300); + }); + }); + + describe('oversized entry', () => { + it('is skipped; cache unaffected', () => { + const c = new ByteLRU(100); + c.set('a', 'A', 50); + const ok = c.set('huge', 'HUGE', 200); + expect(ok).toBe(false); + expect(c.has('huge')).toBe(false); + expect(c.has('a')).toBe(true); + expect(c.calculatedSize).toEqual(50); + }); + }); + + describe('.values()', () => { + it('iterates yielding plain values (Percy discovery.test.js call-site shape)', () => { + const c = new ByteLRU(); + c.set('a', { root: true }, 100); + c.set('b', { root: false }, 100); + c.set('c', { root: true }, 100); + const rootResources = Array.from(c.values()).filter(r => !!r.root); + expect(rootResources.length).toEqual(2); + }); + }); + + describe('.delete()', () => { + it('updates bytes correctly and prevents double-count on re-insert', () => { + const c = new ByteLRU(1000); + c.set('a', 'A', 100); + c.set('b', 'B', 200); + c.delete('a'); + expect(c.has('a')).toBe(false); + expect(c.calculatedSize).toEqual(200); + c.set('a', 'A', 100); + expect(c.calculatedSize).toEqual(300); + }); + }); + + describe('stats', () => { + it('peakBytes captures transient high-water before eviction (honest reporting)', () => { + const c = new ByteLRU(300); + c.set('a', 'A', 100); + c.set('b', 'B', 100); + c.set('c', 'C', 100); + c.set('d', 'D', 100); // transient 400, then evict → 300 + c.delete('b'); + c.delete('c'); + c.delete('d'); + expect(c.calculatedSize).toEqual(0); + expect(c.stats.peakBytes).toEqual(400); + }); + + it('tracks hits / misses / evictions', () => { + const c = new ByteLRU(300); + c.set('a', 'A', 100); + c.set('b', 'B', 100); + c.get('a'); c.get('a'); c.get('missing'); + c.set('c', 'C', 100); + c.set('d', 'D', 100); + const s = c.stats; + expect(s.hits).toEqual(2); + expect(s.misses).toEqual(1); + expect(s.evictions).toBeGreaterThan(0); + expect(s.currentBytes).toEqual(300); + }); + }); + + describe('sanity under alternating get/set', () => { + it('calculated bytes stay consistent across heavy churn', () => { + const c = new ByteLRU(1000); + for (let i = 0; i < 100; i++) c.set(`k${i}`, i, 10); + expect(c.calculatedSize).toEqual(1000); + for (let i = 0; i < 100; i++) c.get(`k${i}`); + for (let i = 0; i < 50; i++) c.set(`n${i}`, i, 20); + expect(c.calculatedSize).toEqual(1000); + for (let i = 0; i < 50; i++) expect(c.has(`n${i}`)).toBe(true); + for (let i = 0; i < 100; i++) expect(c.has(`k${i}`)).toBe(false); + }); + }); + + describe('input guards', () => { + it('refuses NaN/negative sizes', () => { + const c = new ByteLRU(1000); + expect(c.set('a', 'A', NaN)).toBe(false); + expect(c.set('b', 'B', -1)).toBe(false); + expect(c.size).toEqual(0); + }); + }); +}); + +describe('Unit / entrySize', () => { + it('sums content.length + overhead for a single resource', () => { + const r = { content: Buffer.alloc(1000) }; + expect(entrySize(r)).toEqual(1000 + 512); + }); + + it('sums across array-valued root-resource-with-widths', () => { + const arr = [ + { root: true, content: Buffer.alloc(100) }, + { root: true, content: Buffer.alloc(150) }, + { root: true, content: Buffer.alloc(200) } + ]; + expect(entrySize(arr)).toEqual(450 + 3 * 512); + }); + + it('tolerates missing content field', () => { + expect(entrySize({})).toEqual(512); + expect(entrySize(null)).toEqual(512); + }); + + it('accepts custom overhead', () => { + expect(entrySize({ content: Buffer.alloc(100) }, 0)).toEqual(100); + }); +}); From eed9c3239d83454df4aaa8b9d466703201004ccb Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Wed, 22 Apr 2026 12:55:02 +0530 Subject: [PATCH 02/14] 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). --- packages/cli-command/src/flags.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/cli-command/src/flags.js b/packages/cli-command/src/flags.js index 56b7d7f9f..a8f8e1766 100644 --- a/packages/cli-command/src/flags.js +++ b/packages/cli-command/src/flags.js @@ -94,6 +94,16 @@ export const disableCache = { group: 'Percy' }; +export const maxCacheRam = { + name: 'max-cache-ram', + description: 'Cap asset-discovery cache memory in MB (e.g. 300)', + env: 'PERCY_MAX_CACHE_RAM', + percyrc: 'discovery.maxCacheRam', + type: 'integer', + parse: Number, + group: 'Percy' +}; + export const debug = { name: 'debug', description: 'Debug asset discovery and do not upload snapshots', @@ -134,6 +144,7 @@ export const DISCOVERY = [ disallowedHostnames, networkIdleTimeout, disableCache, + maxCacheRam, debug ]; From 0e5364c3b2e1e03d5599d08588b691cb4ed9a01b Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Wed, 22 Apr 2026 12:55:35 +0530 Subject: [PATCH 03/14] 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. --- packages/core/src/config.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/core/src/config.js b/packages/core/src/config.js index c34e45427..cd1a8f13c 100644 --- a/packages/core/src/config.js +++ b/packages/core/src/config.js @@ -369,6 +369,10 @@ export const configSchema = { disableCache: { type: 'boolean' }, + maxCacheRam: { + type: ['integer', 'null'], + minimum: 0 + }, captureMockedServiceWorker: { type: 'boolean', default: false From 9e173eccea01862012d87d761ce4a0ee7ba0a1aa Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Wed, 22 Apr 2026 13:38:03 +0530 Subject: [PATCH 04/14] 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. --- packages/core/src/discovery.js | 79 +++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 2 deletions(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index a36f0df2d..323394d22 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -16,6 +16,7 @@ import { isGzipped, maybeScrollToBottom } from './utils.js'; +import { ByteLRU, entrySize } from './cache/byte-lru.js'; import { sha256hash } from '@percy/client/utils'; @@ -393,6 +394,24 @@ export async function* discoverSnapshotResources(queue, options, callback) { // Used to cache resources across core instances export const RESOURCE_CACHE_KEY = Symbol('resource-cache'); +// Per-run cache stats (hits/misses/evictions/peak + telemetry-gate flags). +export const CACHE_STATS_KEY = Symbol('resource-cache-stats'); + +// MAX_RESOURCE_SIZE in network.js is 25MB; a cap below that dooms every +// resource to be skipped. MIN_REASONABLE_CAP_MB warns users picking a +// silently-useless cap. +const BYTES_PER_MB = 1_000_000; +const MAX_RESOURCE_SIZE_MB = 25; +const MIN_REASONABLE_CAP_MB = 50; + +function makeCacheStats() { + return { + oversizeSkipped: 0, + firstEvictionEventFired: false, + warningFired: false, + unsetModeBytes: 0 + }; +} // Creates an asset discovery queue that uses the percy browser instance to create a page for each // snapshot which is used to intercept and capture snapshot resource requests. @@ -400,12 +419,53 @@ export function createDiscoveryQueue(percy) { let { concurrency } = percy.config.discovery; let queue = new Queue('discovery'); let cache; + // Bytes cap (null = unbounded). Validated at queue start. + let capBytes = null; return queue .set({ concurrency }) // on start, launch the browser and run the queue .handle('start', async () => { - cache = percy[RESOURCE_CACHE_KEY] = new Map(); + const maxCacheRamMB = percy.config.discovery.maxCacheRam; + + // Startup validation. Runs once per Percy run. + if (maxCacheRamMB != null) { + if (maxCacheRamMB < MAX_RESOURCE_SIZE_MB) { + throw new Error( + `--max-cache-ram must be at least ${MAX_RESOURCE_SIZE_MB}MB ` + + '(individual resources up to 25MB are otherwise dropped). ' + + `Received: ${maxCacheRamMB}MB.` + ); + } + if (maxCacheRamMB < MIN_REASONABLE_CAP_MB) { + percy.log.warn( + `--max-cache-ram=${maxCacheRamMB}MB is very small; ` + + 'most resources will not fit and hit rate will be near zero.' + ); + } + if (percy.config.discovery.disableCache) { + percy.log.info('--max-cache-ram is ignored because --disable-cache is set.'); + } + capBytes = maxCacheRamMB * BYTES_PER_MB; + } + + percy[CACHE_STATS_KEY] = makeCacheStats(); + + if (capBytes != null) { + cache = percy[RESOURCE_CACHE_KEY] = new ByteLRU(capBytes, { + onEvict: (key, reason) => { + if (reason === 'lru') { + percy.log.debug( + `cache eviction: evicted ${key} ` + + `(cache now ${Math.round(cache.calculatedSize / BYTES_PER_MB)}` + + `/${maxCacheRamMB} MB)` + ); + } + } + }); + } else { + cache = percy[RESOURCE_CACHE_KEY] = new Map(); + } // If browser.launch() fails it will get captured in // *percy.start() @@ -476,7 +536,22 @@ export function createDiscoveryQueue(percy) { return; } snapshot.resources.set(r.url, r); - if (!snapshot.discovery.disableCache) { + if (snapshot.discovery.disableCache) return; + + if (capBytes != null) { + // Bounded ByteLRU mode — compute size, skip oversized entries + // so a single big resource cannot thrash the cache. + const size = entrySize(r); + if (size > capBytes) { + percy[CACHE_STATS_KEY].oversizeSkipped++; + percy.log.debug( + `Skipping cache for resource ${r.url} ` + + `(${size} bytes > cap ${capBytes})` + ); + return; + } + cache.set(r.url, r, size); + } else { cache.set(r.url, r); } } From ff45eb386cf43c06235761ea61be7d4be2a48cfc Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Wed, 22 Apr 2026 13:45:00 +0530 Subject: [PATCH 05/14] feat(core): add warning-at-threshold when --max-cache-ram is unset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/core/src/discovery.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 323394d22..16a01c7be 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -403,6 +403,10 @@ export const CACHE_STATS_KEY = Symbol('resource-cache-stats'); const BYTES_PER_MB = 1_000_000; const MAX_RESOURCE_SIZE_MB = 25; const MIN_REASONABLE_CAP_MB = 50; +// Default warn threshold when no cap is set: nudge the user toward +// --max-cache-ram before their CI runner OOMs. Override via +// PERCY_CACHE_WARN_THRESHOLD_BYTES for post-ship tuning. +const DEFAULT_WARN_THRESHOLD_BYTES = 500 * BYTES_PER_MB; function makeCacheStats() { return { @@ -413,6 +417,11 @@ function makeCacheStats() { }; } +function warnThresholdBytes() { + const raw = Number(process.env.PERCY_CACHE_WARN_THRESHOLD_BYTES); + return Number.isFinite(raw) && raw > 0 ? raw : DEFAULT_WARN_THRESHOLD_BYTES; +} + // Creates an asset discovery queue that uses the percy browser instance to create a page for each // snapshot which is used to intercept and capture snapshot resource requests. export function createDiscoveryQueue(percy) { @@ -553,6 +562,25 @@ export function createDiscoveryQueue(percy) { cache.set(r.url, r, size); } else { cache.set(r.url, r); + // Unset-cap path: track running bytes and fire a one-shot + // warning at the threshold. Fires at warn level so users on + // --quiet still see it before they OOM. + const stats = percy[CACHE_STATS_KEY]; + if (stats && !stats.warningFired) { + stats.unsetModeBytes += entrySize(r); + if (stats.unsetModeBytes >= warnThresholdBytes()) { + stats.warningFired = true; + const bytes = stats.unsetModeBytes; + const pretty = bytes >= BYTES_PER_MB + ? `${(bytes / BYTES_PER_MB).toFixed(1)}MB` + : `${Math.round(bytes / 1000)}KB`; + percy.log.warn( + `Percy cache is using ${pretty}. If your CI is ` + + 'memory-constrained, set --max-cache-ram. ' + + 'See https://www.browserstack.com/docs/percy/cli/managing-cache-memory' + ); + } + } } } } From 2e26f1a56916da30ba0d8ced9f4fe1699c53d669 Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Wed, 22 Apr 2026 13:54:56 +0530 Subject: [PATCH 06/14] feat(core): emit cache_eviction_started + cache_summary telemetry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/core/src/discovery.js | 35 ++++++++++++++++++++++++----- packages/core/src/percy.js | 40 +++++++++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 16a01c7be..5bd6c61fe 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -422,6 +422,20 @@ function warnThresholdBytes() { return Number.isFinite(raw) && raw > 0 ? raw : DEFAULT_WARN_THRESHOLD_BYTES; } +// Fire-and-forget send-events telemetry. Never blocks discovery, never +// surfaces errors to the user — telemetry loss must not fail a build. +function fireCacheEventSafe(percy, message, extra) { + if (!percy.build?.id) return; + Promise.resolve() + .then(() => percy.client.sendBuildEvents(percy.build.id, { + message, + cliVersion: percy.client.cliVersion, + clientInfo: percy.clientInfo, + extra + })) + .catch(err => percy.log.debug(`${message} telemetry failed`, err)); +} + // Creates an asset discovery queue that uses the percy browser instance to create a page for each // snapshot which is used to intercept and capture snapshot resource requests. export function createDiscoveryQueue(percy) { @@ -463,12 +477,23 @@ export function createDiscoveryQueue(percy) { if (capBytes != null) { cache = percy[RESOURCE_CACHE_KEY] = new ByteLRU(capBytes, { onEvict: (key, reason) => { - if (reason === 'lru') { - percy.log.debug( - `cache eviction: evicted ${key} ` + - `(cache now ${Math.round(cache.calculatedSize / BYTES_PER_MB)}` + - `/${maxCacheRamMB} MB)` + if (reason !== 'lru') return; + percy.log.debug( + `cache eviction: evicted ${key} ` + + `(cache now ${Math.round(cache.calculatedSize / BYTES_PER_MB)}` + + `/${maxCacheRamMB} MB)` + ); + const stats = percy[CACHE_STATS_KEY]; + if (stats && !stats.firstEvictionEventFired) { + stats.firstEvictionEventFired = true; + percy.log.info( + 'Cache eviction active — cap reached, oldest entries being dropped.' ); + fireCacheEventSafe(percy, 'cache_eviction_started', { + cache_budget_ram_mb: maxCacheRamMB, + cache_peak_bytes_seen: cache.stats.peakBytes, + eviction_count: cache.stats.evictions + }); } } }); diff --git a/packages/core/src/percy.js b/packages/core/src/percy.js index 3f1a9eb48..d24bba7ff 100644 --- a/packages/core/src/percy.js +++ b/packages/core/src/percy.js @@ -26,7 +26,9 @@ import { } from './snapshot.js'; import { discoverSnapshotResources, - createDiscoveryQueue + createDiscoveryQueue, + RESOURCE_CACHE_KEY, + CACHE_STATS_KEY } from './discovery.js'; import Monitoring from '@percy/monitoring'; import { WaitForJob } from './wait-for-job.js'; @@ -376,10 +378,46 @@ export class Percy { // This issue doesn't comes under regular error logs, // it's detected if we just and stop percy server await this.checkForNoSnapshotCommandError(); + await this.sendCacheSummary(); await this.sendBuildLogs(); } } + async sendCacheSummary() { + // Fire-and-forget end-of-run summary of asset-discovery cache usage. + // Feeds the Amplitude dashboard (adoption, hit rate, peak bytes). + // Never blocks or fails percy.stop(). + try { + if (!this.build?.id) return; + const cache = this[RESOURCE_CACHE_KEY]; + const stats = this[CACHE_STATS_KEY]; + if (!cache || !stats) return; + + const cacheStats = typeof cache.stats === 'object' ? cache.stats : null; + const budgetMB = this.config?.discovery?.maxCacheRam ?? null; + + const extra = { + cache_budget_ram_mb: budgetMB, + hits: cacheStats?.hits ?? 0, + misses: cacheStats?.misses ?? 0, + evictions: cacheStats?.evictions ?? 0, + peak_bytes: cacheStats?.peakBytes ?? stats.unsetModeBytes, + final_bytes: cache.calculatedSize ?? stats.unsetModeBytes, + entry_count: cache.size ?? 0, + oversize_skipped: stats.oversizeSkipped + }; + + await this.client.sendBuildEvents(this.build.id, { + message: 'cache_summary', + cliVersion: this.client.cliVersion, + clientInfo: this.clientInfo, + extra + }); + } catch (err) { + this.log.debug('cache_summary telemetry failed', err); + } + } + checkAndUpdateConcurrency() { // early exit if monitoring is disabled if (!this.systemMonitoringEnabled()) return; From d1cf2c0d6d514dd63ab066d70940782d1c6f2871 Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Wed, 22 Apr 2026 14:13:49 +0530 Subject: [PATCH 07/14] 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. --- packages/core/test/discovery.test.js | 101 +++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 898bc8b42..d6b4f661c 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -2314,6 +2314,107 @@ describe('Discovery', () => { }); }); + describe('with --max-cache-ram', () => { + async function startPercyWith(discoveryExtras = {}) { + await percy.stop(true); + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { concurrency: 1, ...discoveryExtras } + }); + } + + afterEach(() => { + delete process.env.PERCY_CACHE_WARN_THRESHOLD_BYTES; + }); + + it('installs a ByteLRU when maxCacheRam is set', async () => { + await startPercyWith({ maxCacheRam: 25 }); + const cache = percy[RESOURCE_CACHE_KEY]; + expect(cache.constructor.name).toEqual('ByteLRU'); + expect(cache.calculatedSize).toEqual(0); + expect(cache.stats).toEqual(jasmine.objectContaining({ + hits: 0, misses: 0, evictions: 0, peakBytes: 0 + })); + }); + + it('uses a plain Map when maxCacheRam is unset (backward compat)', () => { + // percy was started in beforeEach without maxCacheRam + expect(percy[RESOURCE_CACHE_KEY] instanceof Map).toBe(true); + }); + + it('rejects a cap below the 25MB resource-size floor', async () => { + await percy.stop(true); + await expectAsync(Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { concurrency: 1, maxCacheRam: 10 } + })).toBeRejectedWithError(/must be at least 25MB/); + }); + + it('emits an info log when cap and --disable-cache are both set', async () => { + await startPercyWith({ maxCacheRam: 50, disableCache: true }); + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + jasmine.stringContaining('--max-cache-ram is ignored because --disable-cache is set') + ])); + }); + + it('records oversize_skipped in stats when an entry is bigger than cap', async () => { + await startPercyWith({ maxCacheRam: 25 }); + const cache = percy[RESOURCE_CACHE_KEY]; + // Inject a synthetic oversized resource directly — the real network + // path caps individual resources at 25MB so this simulates the edge + // case where entrySize exceeds the cap. + const saved = cache.set('http://x/huge', { content: Buffer.alloc(26_000_001) }, 26_000_001 + 512); + expect(saved).toBe(false); + expect(cache.calculatedSize).toEqual(0); + }); + }); + + describe('warning-at-threshold (unset cap)', () => { + afterEach(() => { + delete process.env.PERCY_CACHE_WARN_THRESHOLD_BYTES; + }); + + it('fires a warn-level log once when cache crosses the threshold', async () => { + // Override to a tiny threshold so the snapshot's real resources trip it. + process.env.PERCY_CACHE_WARN_THRESHOLD_BYTES = '100'; + await percy.stop(true); + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { concurrency: 1 } + }); + + await percy.snapshot({ + name: 'warning snapshot', + url: 'http://localhost:8000', + domSnapshot: testDOM + }); + await percy.idle(); + + const warnHits = logger.stderr.filter(line => + line.includes('Percy cache is using') && + line.includes('--max-cache-ram') + ); + expect(warnHits.length).toEqual(1); + + // Trigger another resource save; the gate should stay closed. + await percy.snapshot({ + name: 'warning snapshot second', + url: 'http://localhost:8000', + domSnapshot: testDOM + }); + await percy.idle(); + + const warnHitsAfter = logger.stderr.filter(line => + line.includes('Percy cache is using') && + line.includes('--max-cache-ram') + ); + expect(warnHitsAfter.length).toEqual(1); + }); + }); + describe('with resource errors', () => { // sabotage this method to trigger unexpected error handling async function triggerSessionEventError(event, error) { From 6ae0659b4d6bfc0e4669ee0f4e2065c72d510b06 Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Wed, 22 Apr 2026 14:14:34 +0530 Subject: [PATCH 08/14] 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). --- packages/core/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/README.md b/packages/core/README.md index 04c8989af..cda8d1061 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -53,6 +53,7 @@ The following options can also be defined within a Percy config file - `requestHeaders` — Request headers used when discovering snapshot assets - `authorization` — Basic auth `username` and `password` for protected snapshot assets - `disableCache` — Disable asset caching (**default** `false`) + - `maxCacheRam` — Cap the asset-discovery cache at this many MB (**default** unset/unbounded). When set, least-recently-used resources are evicted to stay within the cap. The cap measures cache body bytes only; process RSS is typically 1.5–2× the cap due to Node's Buffer slab allocator. Minimum 25 MB (floor — below this, the 25 MB per-resource ceiling would reject every resource). Also settable via the `--max-cache-ram ` CLI flag or the `PERCY_MAX_CACHE_RAM` env var - `userAgent` — Custom user-agent string used when requesting assets - `cookies` — Browser cookies to use when requesting assets - `networkIdleTimeout` — Milliseconds to wait for the network to idle (**default** `100`) From dad8a08edd8bd3a3c9945c2a0cd48aa6783d5d2a Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Wed, 22 Apr 2026 14:48:10 +0530 Subject: [PATCH 09/14] fix(core): clamp --max-cache-ram below 25MB instead of failing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/cli-command/test/flags.test.js | 1 + packages/core/README.md | 2 +- packages/core/src/discovery.js | 15 ++++++++------- packages/core/test/discovery.test.js | 14 +++++++------- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/cli-command/test/flags.test.js b/packages/cli-command/test/flags.test.js index d0c21c87e..b630e493d 100644 --- a/packages/cli-command/test/flags.test.js +++ b/packages/cli-command/test/flags.test.js @@ -104,6 +104,7 @@ describe('Built-in flags:', () => { --disallowed-hostname Disallowed hostnames to abort in asset discovery -t, --network-idle-timeout Asset discovery network idle timeout --disable-cache Disable asset discovery caches + --max-cache-ram Cap asset-discovery cache memory in MB (e.g. 300) --debug Debug asset discovery and do not upload snapshots `); diff --git a/packages/core/README.md b/packages/core/README.md index cda8d1061..518d95ebf 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -53,7 +53,7 @@ The following options can also be defined within a Percy config file - `requestHeaders` — Request headers used when discovering snapshot assets - `authorization` — Basic auth `username` and `password` for protected snapshot assets - `disableCache` — Disable asset caching (**default** `false`) - - `maxCacheRam` — Cap the asset-discovery cache at this many MB (**default** unset/unbounded). When set, least-recently-used resources are evicted to stay within the cap. The cap measures cache body bytes only; process RSS is typically 1.5–2× the cap due to Node's Buffer slab allocator. Minimum 25 MB (floor — below this, the 25 MB per-resource ceiling would reject every resource). Also settable via the `--max-cache-ram ` CLI flag or the `PERCY_MAX_CACHE_RAM` env var + - `maxCacheRam` — Cap the asset-discovery cache at this many MB (**default** unset/unbounded). When set, least-recently-used resources are evicted to stay within the cap. The cap measures cache body bytes only; process RSS is typically 1.5–2× the cap due to Node's Buffer slab allocator. Values below 25 MB are clamped to 25 MB with a warn log (the per-resource ceiling is 25 MB, so smaller caps would reject every resource). Also settable via the `--max-cache-ram ` CLI flag or the `PERCY_MAX_CACHE_RAM` env var - `userAgent` — Custom user-agent string used when requesting assets - `cookies` — Browser cookies to use when requesting assets - `networkIdleTimeout` — Milliseconds to wait for the network to idle (**default** `100`) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 5bd6c61fe..66d95615d 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -449,18 +449,19 @@ export function createDiscoveryQueue(percy) { .set({ concurrency }) // on start, launch the browser and run the queue .handle('start', async () => { - const maxCacheRamMB = percy.config.discovery.maxCacheRam; + let maxCacheRamMB = percy.config.discovery.maxCacheRam; // Startup validation. Runs once per Percy run. if (maxCacheRamMB != null) { if (maxCacheRamMB < MAX_RESOURCE_SIZE_MB) { - throw new Error( - `--max-cache-ram must be at least ${MAX_RESOURCE_SIZE_MB}MB ` + - '(individual resources up to 25MB are otherwise dropped). ' + - `Received: ${maxCacheRamMB}MB.` + percy.log.warn( + `--max-cache-ram=${maxCacheRamMB}MB is below the ${MAX_RESOURCE_SIZE_MB}MB minimum ` + + '(individual resources up to 25MB would otherwise be dropped). ' + + `Continuing with the minimum: ${MAX_RESOURCE_SIZE_MB}MB.` ); - } - if (maxCacheRamMB < MIN_REASONABLE_CAP_MB) { + maxCacheRamMB = MAX_RESOURCE_SIZE_MB; + percy.config.discovery.maxCacheRam = MAX_RESOURCE_SIZE_MB; + } else if (maxCacheRamMB < MIN_REASONABLE_CAP_MB) { percy.log.warn( `--max-cache-ram=${maxCacheRamMB}MB is very small; ` + 'most resources will not fit and hit rate will be near zero.' diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index d6b4f661c..c820a92fb 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -2343,13 +2343,13 @@ describe('Discovery', () => { expect(percy[RESOURCE_CACHE_KEY] instanceof Map).toBe(true); }); - it('rejects a cap below the 25MB resource-size floor', async () => { - await percy.stop(true); - await expectAsync(Percy.start({ - token: 'PERCY_TOKEN', - snapshot: { widths: [1000] }, - discovery: { concurrency: 1, maxCacheRam: 10 } - })).toBeRejectedWithError(/must be at least 25MB/); + it('clamps a cap below the 25MB resource-size floor and warns', async () => { + await startPercyWith({ maxCacheRam: 10 }); + expect(percy[RESOURCE_CACHE_KEY].constructor.name).toEqual('ByteLRU'); + expect(percy.config.discovery.maxCacheRam).toEqual(25); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + jasmine.stringContaining('--max-cache-ram=10MB is below the 25MB minimum') + ])); }); it('emits an info log when cap and --disable-cache are both set', async () => { From e1d6e1c206697e8592853bad80aa8f75e1febc36 Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Wed, 22 Apr 2026 15:31:49 +0530 Subject: [PATCH 10/14] fix(core): address PR review (frozen counter, reorder checks, reorder egress, effective cap) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/core/README.md | 2 +- packages/core/src/cache/byte-lru.js | 12 ++- packages/core/src/discovery.js | 57 +++++++---- packages/core/src/percy.js | 11 ++- packages/core/test/discovery.test.js | 118 ++++++++++++++++++++++- packages/core/test/unit/byte-lru.test.js | 29 ++++++ 6 files changed, 195 insertions(+), 34 deletions(-) diff --git a/packages/core/README.md b/packages/core/README.md index 518d95ebf..24622e7ed 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -53,7 +53,7 @@ The following options can also be defined within a Percy config file - `requestHeaders` — Request headers used when discovering snapshot assets - `authorization` — Basic auth `username` and `password` for protected snapshot assets - `disableCache` — Disable asset caching (**default** `false`) - - `maxCacheRam` — Cap the asset-discovery cache at this many MB (**default** unset/unbounded). When set, least-recently-used resources are evicted to stay within the cap. The cap measures cache body bytes only; process RSS is typically 1.5–2× the cap due to Node's Buffer slab allocator. Values below 25 MB are clamped to 25 MB with a warn log (the per-resource ceiling is 25 MB, so smaller caps would reject every resource). Also settable via the `--max-cache-ram ` CLI flag or the `PERCY_MAX_CACHE_RAM` env var + - `maxCacheRam` — Cap the asset-discovery cache at this many MB (**default** unset/unbounded). When set, least-recently-used resources are evicted to stay within the cap. MB is decimal (1 MB = 1,000,000 bytes), not binary MiB (1,048,576). The cap measures cache body bytes only; process RSS is typically 1.5–2× the cap due to Node's Buffer slab allocator. Values below 25 MB are clamped to 25 MB with a warn log (the per-resource ceiling is 25 MB, so smaller caps would reject every resource). Also settable via the `--max-cache-ram ` CLI flag or the `PERCY_MAX_CACHE_RAM` env var - `userAgent` — Custom user-agent string used when requesting assets - `cookies` — Browser cookies to use when requesting assets - `networkIdleTimeout` — Milliseconds to wait for the network to idle (**default** `100`) diff --git a/packages/core/src/cache/byte-lru.js b/packages/core/src/cache/byte-lru.js index f26755232..e8b2c27ce 100644 --- a/packages/core/src/cache/byte-lru.js +++ b/packages/core/src/cache/byte-lru.js @@ -32,16 +32,18 @@ export class ByteLRU { set(key, value, size) { if (!Number.isFinite(size) || size < 0) return false; - if (this.#map.has(key)) { - this.#bytes -= this.#map.get(key).size; - this.#map.delete(key); - } - + // Reject oversize BEFORE touching any existing entry — a failed oversize + // set on an existing key must not evict the prior (valid) entry. if (this.#max !== undefined && size > this.#max) { if (this.onEvict) this.onEvict(key, 'too-big'); return false; } + if (this.#map.has(key)) { + this.#bytes -= this.#map.get(key).size; + this.#map.delete(key); + } + this.#map.set(key, { value, size }); this.#bytes += size; if (this.#bytes > this.#stats.peakBytes) this.#stats.peakBytes = this.#bytes; diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 66d95615d..69210876d 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -410,6 +410,10 @@ const DEFAULT_WARN_THRESHOLD_BYTES = 500 * BYTES_PER_MB; function makeCacheStats() { return { + // The effective cap actually used this run, in MB (after any clamp to the + // 25MB floor). null when the flag was unset. Read by cache_summary telemetry + // so it reports what ran, not what the user typed. + effectiveMaxCacheRamMB: null, oversizeSkipped: 0, firstEvictionEventFired: false, warningFired: false, @@ -417,7 +421,7 @@ function makeCacheStats() { }; } -function warnThresholdBytes() { +function readWarnThresholdBytes() { const raw = Number(process.env.PERCY_CACHE_WARN_THRESHOLD_BYTES); return Number.isFinite(raw) && raw > 0 ? raw : DEFAULT_WARN_THRESHOLD_BYTES; } @@ -445,35 +449,50 @@ export function createDiscoveryQueue(percy) { // Bytes cap (null = unbounded). Validated at queue start. let capBytes = null; + // Read the unset-cap warning threshold once per queue construction; this + // closure is consulted on every saveResource call, so we avoid reparsing + // process.env each time. + const warnThreshold = readWarnThresholdBytes(); + return queue .set({ concurrency }) // on start, launch the browser and run the queue .handle('start', async () => { - let maxCacheRamMB = percy.config.discovery.maxCacheRam; - - // Startup validation. Runs once per Percy run. - if (maxCacheRamMB != null) { - if (maxCacheRamMB < MAX_RESOURCE_SIZE_MB) { + const configuredMaxCacheRamMB = percy.config.discovery.maxCacheRam; + let effectiveMaxCacheRamMB = configuredMaxCacheRamMB; + + // Startup validation. Runs once per Percy run. The user's config object + // is NOT mutated — the effective value lives on CACHE_STATS_KEY for the + // duration of the run. + if (configuredMaxCacheRamMB != null) { + if (configuredMaxCacheRamMB < MAX_RESOURCE_SIZE_MB) { percy.log.warn( - `--max-cache-ram=${maxCacheRamMB}MB is below the ${MAX_RESOURCE_SIZE_MB}MB minimum ` + + `--max-cache-ram=${configuredMaxCacheRamMB}MB is below the ${MAX_RESOURCE_SIZE_MB}MB minimum ` + '(individual resources up to 25MB would otherwise be dropped). ' + `Continuing with the minimum: ${MAX_RESOURCE_SIZE_MB}MB.` ); - maxCacheRamMB = MAX_RESOURCE_SIZE_MB; - percy.config.discovery.maxCacheRam = MAX_RESOURCE_SIZE_MB; - } else if (maxCacheRamMB < MIN_REASONABLE_CAP_MB) { + effectiveMaxCacheRamMB = MAX_RESOURCE_SIZE_MB; + } else if (configuredMaxCacheRamMB < MIN_REASONABLE_CAP_MB) { percy.log.warn( - `--max-cache-ram=${maxCacheRamMB}MB is very small; ` + + `--max-cache-ram=${configuredMaxCacheRamMB}MB is very small; ` + 'most resources will not fit and hit rate will be near zero.' ); } if (percy.config.discovery.disableCache) { percy.log.info('--max-cache-ram is ignored because --disable-cache is set.'); } - capBytes = maxCacheRamMB * BYTES_PER_MB; + capBytes = effectiveMaxCacheRamMB * BYTES_PER_MB; + } + + if (warnThreshold !== DEFAULT_WARN_THRESHOLD_BYTES) { + percy.log.debug( + `PERCY_CACHE_WARN_THRESHOLD_BYTES override active: ${warnThreshold} bytes ` + + `(default ${DEFAULT_WARN_THRESHOLD_BYTES}).` + ); } percy[CACHE_STATS_KEY] = makeCacheStats(); + percy[CACHE_STATS_KEY].effectiveMaxCacheRamMB = capBytes != null ? effectiveMaxCacheRamMB : null; if (capBytes != null) { cache = percy[RESOURCE_CACHE_KEY] = new ByteLRU(capBytes, { @@ -482,7 +501,7 @@ export function createDiscoveryQueue(percy) { percy.log.debug( `cache eviction: evicted ${key} ` + `(cache now ${Math.round(cache.calculatedSize / BYTES_PER_MB)}` + - `/${maxCacheRamMB} MB)` + `/${effectiveMaxCacheRamMB} MB)` ); const stats = percy[CACHE_STATS_KEY]; if (stats && !stats.firstEvictionEventFired) { @@ -491,7 +510,7 @@ export function createDiscoveryQueue(percy) { 'Cache eviction active — cap reached, oldest entries being dropped.' ); fireCacheEventSafe(percy, 'cache_eviction_started', { - cache_budget_ram_mb: maxCacheRamMB, + cache_budget_ram_mb: effectiveMaxCacheRamMB, cache_peak_bytes_seen: cache.stats.peakBytes, eviction_count: cache.stats.evictions }); @@ -588,13 +607,13 @@ export function createDiscoveryQueue(percy) { cache.set(r.url, r, size); } else { cache.set(r.url, r); - // Unset-cap path: track running bytes and fire a one-shot - // warning at the threshold. Fires at warn level so users on - // --quiet still see it before they OOM. + // Unset-cap path: track running bytes unconditionally so + // cache_summary telemetry reports the true peak. Gate only + // the one-shot warn-log emission on whether we've fired yet. const stats = percy[CACHE_STATS_KEY]; - if (stats && !stats.warningFired) { + if (stats) { stats.unsetModeBytes += entrySize(r); - if (stats.unsetModeBytes >= warnThresholdBytes()) { + if (!stats.warningFired && stats.unsetModeBytes >= warnThreshold) { stats.warningFired = true; const bytes = stats.unsetModeBytes; const pretty = bytes >= BYTES_PER_MB diff --git a/packages/core/src/percy.js b/packages/core/src/percy.js index d24bba7ff..4a051ac67 100644 --- a/packages/core/src/percy.js +++ b/packages/core/src/percy.js @@ -378,15 +378,18 @@ export class Percy { // This issue doesn't comes under regular error logs, // it's detected if we just and stop percy server await this.checkForNoSnapshotCommandError(); - await this.sendCacheSummary(); + // sendBuildLogs goes first — it's the primary egress. cache_summary is + // analytics, ordered after so a slow pager hop cannot delay the logs. await this.sendBuildLogs(); + await this.sendCacheSummary(); } } async sendCacheSummary() { // Fire-and-forget end-of-run summary of asset-discovery cache usage. // Feeds the Amplitude dashboard (adoption, hit rate, peak bytes). - // Never blocks or fails percy.stop(). + // Never blocks or fails percy.stop(); telemetry loss is preferable to a + // failed stop. try { if (!this.build?.id) return; const cache = this[RESOURCE_CACHE_KEY]; @@ -394,10 +397,10 @@ export class Percy { if (!cache || !stats) return; const cacheStats = typeof cache.stats === 'object' ? cache.stats : null; - const budgetMB = this.config?.discovery?.maxCacheRam ?? null; const extra = { - cache_budget_ram_mb: budgetMB, + // Report the effective cap (post-clamp), not the raw config value. + cache_budget_ram_mb: stats.effectiveMaxCacheRamMB, hits: cacheStats?.hits ?? 0, misses: cacheStats?.misses ?? 0, evictions: cacheStats?.evictions ?? 0, diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index c820a92fb..3c3073321 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -1,7 +1,8 @@ import { sha256hash } from '@percy/client/utils'; import { logger, api, setupTest, createTestServer, dedent, mockRequests } from './helpers/index.js'; import Percy from '@percy/core'; -import { RESOURCE_CACHE_KEY } from '../src/discovery.js'; +import { RESOURCE_CACHE_KEY, CACHE_STATS_KEY } from '../src/discovery.js'; +import { ByteLRU } from '../src/cache/byte-lru.js'; import Session from '../src/session.js'; import Pako from 'pako'; import * as CoreConfig from '@percy/core/config'; @@ -2331,7 +2332,7 @@ describe('Discovery', () => { it('installs a ByteLRU when maxCacheRam is set', async () => { await startPercyWith({ maxCacheRam: 25 }); const cache = percy[RESOURCE_CACHE_KEY]; - expect(cache.constructor.name).toEqual('ByteLRU'); + expect(cache instanceof ByteLRU).toBe(true); expect(cache.calculatedSize).toEqual(0); expect(cache.stats).toEqual(jasmine.objectContaining({ hits: 0, misses: 0, evictions: 0, peakBytes: 0 @@ -2343,10 +2344,12 @@ describe('Discovery', () => { expect(percy[RESOURCE_CACHE_KEY] instanceof Map).toBe(true); }); - it('clamps a cap below the 25MB resource-size floor and warns', async () => { + it('clamps a cap below the 25MB floor, warns, and leaves user config intact', async () => { await startPercyWith({ maxCacheRam: 10 }); - expect(percy[RESOURCE_CACHE_KEY].constructor.name).toEqual('ByteLRU'); - expect(percy.config.discovery.maxCacheRam).toEqual(25); + expect(percy[RESOURCE_CACHE_KEY] instanceof ByteLRU).toBe(true); + // User config is NOT mutated — the effective value lives on stats. + expect(percy.config.discovery.maxCacheRam).toEqual(10); + expect(percy[CACHE_STATS_KEY].effectiveMaxCacheRamMB).toEqual(25); expect(logger.stderr).toEqual(jasmine.arrayContaining([ jasmine.stringContaining('--max-cache-ram=10MB is below the 25MB minimum') ])); @@ -2359,6 +2362,33 @@ describe('Discovery', () => { ])); }); + it('warns when cap is >= 25MB but below the reasonable floor', async () => { + // Between MAX_RESOURCE_SIZE_MB (25) and MIN_REASONABLE_CAP_MB (50). + // No clamp, but surface the likely-too-tight hit-rate warning. + await startPercyWith({ maxCacheRam: 30 }); + expect(percy[CACHE_STATS_KEY].effectiveMaxCacheRamMB).toEqual(30); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + jasmine.stringContaining('--max-cache-ram=30MB is very small') + ])); + }); + + it('fires cache_eviction_started info log + sets gate when an LRU eviction happens', async () => { + await startPercyWith({ maxCacheRam: 25 }); + const cache = percy[RESOURCE_CACHE_KEY]; + // Drive the cache past cap so onEvict fires the lru branch. + const capBytes = 25 * 1_000_000; + const chunk = Math.floor(capBytes / 3); + cache.set('a', { content: Buffer.alloc(chunk) }, chunk + 512); + cache.set('b', { content: Buffer.alloc(chunk) }, chunk + 512); + cache.set('c', { content: Buffer.alloc(chunk) }, chunk + 512); + cache.set('d', { content: Buffer.alloc(chunk) }, chunk + 512); + expect(cache.stats.evictions).toBeGreaterThan(0); + expect(percy[CACHE_STATS_KEY].firstEvictionEventFired).toBe(true); + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + jasmine.stringContaining('Cache eviction active') + ])); + }); + it('records oversize_skipped in stats when an entry is bigger than cap', async () => { await startPercyWith({ maxCacheRam: 25 }); const cache = percy[RESOURCE_CACHE_KEY]; @@ -2376,6 +2406,20 @@ describe('Discovery', () => { delete process.env.PERCY_CACHE_WARN_THRESHOLD_BYTES; }); + it('emits a debug log when PERCY_CACHE_WARN_THRESHOLD_BYTES is overridden', async () => { + process.env.PERCY_CACHE_WARN_THRESHOLD_BYTES = '100'; + await percy.stop(true); + await logger.mock({ level: 'debug' }); + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { concurrency: 1 } + }); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + jasmine.stringContaining('PERCY_CACHE_WARN_THRESHOLD_BYTES override active') + ])); + }); + it('fires a warn-level log once when cache crosses the threshold', async () => { // Override to a tiny threshold so the snapshot's real resources trip it. process.env.PERCY_CACHE_WARN_THRESHOLD_BYTES = '100'; @@ -2413,6 +2457,70 @@ describe('Discovery', () => { ); expect(warnHitsAfter.length).toEqual(1); }); + + it('sendCacheSummary swallows telemetry failures (never throws)', async () => { + // Ensure sendCacheSummary's early-return guards don't fire so the + // sendBuildEvents call actually runs and enters the catch. + percy.build = { id: '123' }; + percy[RESOURCE_CACHE_KEY] = new Map(); + percy[CACHE_STATS_KEY] = { + effectiveMaxCacheRamMB: null, + oversizeSkipped: 0, + unsetModeBytes: 0 + }; + const spy = spyOn(percy.client, 'sendBuildEvents') + .and.rejectWith(new Error('pager down')); + await expectAsync(percy.sendCacheSummary()).toBeResolved(); + expect(spy).toHaveBeenCalled(); + }); + + it('sendCacheSummary short-circuits when the build has not been created', async () => { + percy.build = undefined; + const spy = spyOn(percy.client, 'sendBuildEvents'); + await percy.sendCacheSummary(); + expect(spy).not.toHaveBeenCalled(); + }); + + it('sendCacheSummary short-circuits when the cache or stats are missing', async () => { + percy.build = { id: '123' }; + percy[RESOURCE_CACHE_KEY] = undefined; + percy[CACHE_STATS_KEY] = undefined; + const spy = spyOn(percy.client, 'sendBuildEvents'); + await percy.sendCacheSummary(); + expect(spy).not.toHaveBeenCalled(); + }); + + it('keeps incrementing unsetModeBytes after the warning has fired', async () => { + // Regression: the byte counter used to freeze as soon as the warning + // flag flipped, so cache_summary.peak_bytes was always pinned to the + // threshold. Now it grows through the whole run. + process.env.PERCY_CACHE_WARN_THRESHOLD_BYTES = '100'; + await percy.stop(true); + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { concurrency: 1 } + }); + + await percy.snapshot({ + name: 'counter grows 1', + url: 'http://localhost:8000', + domSnapshot: testDOM + }); + await percy.idle(); + const afterFirst = percy[CACHE_STATS_KEY].unsetModeBytes; + expect(afterFirst).toBeGreaterThan(100); + expect(percy[CACHE_STATS_KEY].warningFired).toBe(true); + + await percy.snapshot({ + name: 'counter grows 2', + url: 'http://localhost:8000', + domSnapshot: testDOM + }); + await percy.idle(); + // Counter continues to climb even though warningFired is already true. + expect(percy[CACHE_STATS_KEY].unsetModeBytes).toBeGreaterThan(afterFirst); + }); }); describe('with resource errors', () => { diff --git a/packages/core/test/unit/byte-lru.test.js b/packages/core/test/unit/byte-lru.test.js index b8f14325f..a53061ee7 100644 --- a/packages/core/test/unit/byte-lru.test.js +++ b/packages/core/test/unit/byte-lru.test.js @@ -74,6 +74,23 @@ describe('Unit / ByteLRU', () => { expect(c.has('a')).toBe(true); expect(c.calculatedSize).toEqual(50); }); + + it('onEvict fires with reason "too-big" on oversize', () => { + const reasons = []; + const c = new ByteLRU(100, { onEvict: (k, r) => reasons.push({ k, r }) }); + c.set('huge', 'HUGE', 200); + expect(reasons).toEqual([{ k: 'huge', r: 'too-big' }]); + }); + + it('oversized re-set of an existing key leaves the prior entry intact', () => { + const c = new ByteLRU(100); + c.set('k', 'small', 50); + const ok = c.set('k', 'huge', 200); + expect(ok).toBe(false); + expect(c.has('k')).toBe(true); + expect(c.get('k')).toEqual('small'); + expect(c.calculatedSize).toEqual(50); + }); }); describe('.values()', () => { @@ -87,6 +104,18 @@ describe('Unit / ByteLRU', () => { }); }); + describe('.clear()', () => { + it('resets bytes and map', () => { + const c = new ByteLRU(1000); + c.set('a', 'A', 100); + c.set('b', 'B', 200); + c.clear(); + expect(c.size).toEqual(0); + expect(c.calculatedSize).toEqual(0); + expect(c.has('a')).toBe(false); + }); + }); + describe('.delete()', () => { it('updates bytes correctly and prevents double-count on re-insert', () => { const c = new ByteLRU(1000); From a299cd1b748c98f042435331241ef4f3e60d8eab Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Wed, 22 Apr 2026 15:52:15 +0530 Subject: [PATCH 11/14] test(core): close remaining coverage gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/core/test/discovery.test.js | 61 ++++++++++++++++++++++++ packages/core/test/unit/byte-lru.test.js | 18 +++++++ 2 files changed, 79 insertions(+) diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 3c3073321..1a7484714 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -2389,6 +2389,50 @@ describe('Discovery', () => { ])); }); + it('fireCacheEventSafe debug-logs and swallows pager rejections', async () => { + await logger.mock({ level: 'debug' }); + await startPercyWith({ maxCacheRam: 25 }); + percy.build = { id: '123' }; + spyOn(percy.client, 'sendBuildEvents').and.rejectWith(new Error('pager down')); + const cache = percy[RESOURCE_CACHE_KEY]; + const capBytes = 25 * 1_000_000; + const chunk = Math.floor(capBytes / 3); + cache.set('a', { content: Buffer.alloc(chunk) }, chunk + 512); + cache.set('b', { content: Buffer.alloc(chunk) }, chunk + 512); + cache.set('c', { content: Buffer.alloc(chunk) }, chunk + 512); + cache.set('d', { content: Buffer.alloc(chunk) }, chunk + 512); + // fireCacheEventSafe is fire-and-forget — wait a microtask tick for the + // catch handler to run. + await new Promise(r => setImmediate(r)); + expect(percy.client.sendBuildEvents).toHaveBeenCalled(); + }); + + it('saveResource debug-logs and increments oversize_skipped for real oversize resources', async () => { + await startPercyWith({ maxCacheRam: 25 }); + await logger.mock({ level: 'debug' }); + // Serve a resource exactly at the per-resource 25MB ceiling. entrySize + // adds 512B overhead so the ByteLRU path sees it as > cap and takes + // the oversize-skip branch in saveResource. + server = await createTestServer({ + '/': () => [200, 'text/html', dedent` + + big + + `], + '/big.bin': () => [200, 'application/octet-stream', Buffer.alloc(25_000_000, 'x')] + }); + await percy.snapshot({ + name: 'oversize resource', + url: 'http://localhost:8000', + widths: [1000] + }); + await percy.idle(); + expect(percy[CACHE_STATS_KEY].oversizeSkipped).toBeGreaterThan(0); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + jasmine.stringContaining('Skipping cache for resource') + ])); + }); + it('records oversize_skipped in stats when an entry is bigger than cap', async () => { await startPercyWith({ maxCacheRam: 25 }); const cache = percy[RESOURCE_CACHE_KEY]; @@ -2490,6 +2534,23 @@ describe('Discovery', () => { expect(spy).not.toHaveBeenCalled(); }); + it('sendCacheSummary falls back to 0 when cache has no size field', async () => { + percy.build = { id: '123' }; + // Defensive-path cache shape with no .size — exercises the `?? 0` + // fallback on entry_count. + percy[RESOURCE_CACHE_KEY] = { stats: {} }; + percy[CACHE_STATS_KEY] = { + effectiveMaxCacheRamMB: 25, + oversizeSkipped: 0, + unsetModeBytes: 0 + }; + const spy = spyOn(percy.client, 'sendBuildEvents').and.resolveTo(); + await percy.sendCacheSummary(); + expect(spy).toHaveBeenCalledWith('123', jasmine.objectContaining({ + extra: jasmine.objectContaining({ entry_count: 0 }) + })); + }); + it('keeps incrementing unsetModeBytes after the warning has fired', async () => { // Regression: the byte counter used to freeze as soon as the warning // flag flipped, so cache_summary.peak_bytes was always pinned to the diff --git a/packages/core/test/unit/byte-lru.test.js b/packages/core/test/unit/byte-lru.test.js index a53061ee7..6d3aead6c 100644 --- a/packages/core/test/unit/byte-lru.test.js +++ b/packages/core/test/unit/byte-lru.test.js @@ -127,6 +127,14 @@ describe('Unit / ByteLRU', () => { c.set('a', 'A', 100); expect(c.calculatedSize).toEqual(300); }); + + it('returns false when the key is not in the cache', () => { + const c = new ByteLRU(1000); + c.set('a', 'A', 100); + expect(c.delete('missing')).toBe(false); + // existing entry untouched + expect(c.calculatedSize).toEqual(100); + }); }); describe('stats', () => { @@ -201,6 +209,16 @@ describe('Unit / entrySize', () => { expect(entrySize(null)).toEqual(512); }); + it('tolerates null entries and missing content fields inside an array', () => { + // Covers the optional-chain branches in the reduce callback. + const arr = [ + null, + {}, // no content + { content: Buffer.alloc(100) } // populated + ]; + expect(entrySize(arr)).toEqual(100 + 3 * 512); + }); + it('accepts custom overhead', () => { expect(entrySize({ content: Buffer.alloc(100) }, 0)).toEqual(100); }); From 6a1a3d0e81ac37ca2c2fd2eb550993d7b1c54796 Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Thu, 23 Apr 2026 14:43:31 +0530 Subject: [PATCH 12/14] feat(core): spill evicted resources to disk, restore on lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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--. - 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) --- packages/core/src/cache/byte-lru.js | 146 ++++++++- packages/core/src/discovery.js | 146 ++++----- packages/core/src/percy.js | 42 +-- packages/core/test/discovery.test.js | 232 ++++++++++++-- packages/core/test/unit/byte-lru.test.js | 379 ++++++++++++++++++++++- 5 files changed, 803 insertions(+), 142 deletions(-) diff --git a/packages/core/src/cache/byte-lru.js b/packages/core/src/cache/byte-lru.js index e8b2c27ce..2e14966da 100644 --- a/packages/core/src/cache/byte-lru.js +++ b/packages/core/src/cache/byte-lru.js @@ -1,7 +1,15 @@ -// Hand-rolled byte-budget LRU cache. Map insertion order is LRU order; -// .get() deletes and re-sets to move an entry to MRU. Synchronous by design — -// no logger calls, no awaits inside mutation paths, so callers can log -// after .set() returns without risking a mid-state event-loop yield. +// Two-tier cache used by asset discovery: +// ByteLRU — byte-budget in-memory LRU; Map insertion order = LRU order. +// DiskSpillStore — on-disk overflow tier. RAM evictions spill here; lookups +// fall back to disk before refetching from origin. +// All operations are synchronous; callers (network intercept, ByteLRU.set) +// cannot yield to the event loop mid-op. Per-entry size is capped at 25MB +// upstream so disk I/O latency is bounded. + +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import crypto from 'crypto'; const DEFAULT_PER_ENTRY_OVERHEAD = 512; @@ -32,10 +40,10 @@ export class ByteLRU { set(key, value, size) { if (!Number.isFinite(size) || size < 0) return false; - // Reject oversize BEFORE touching any existing entry — a failed oversize - // set on an existing key must not evict the prior (valid) entry. + // Reject oversize BEFORE touching any existing entry — a failed set on an + // existing key must not evict the prior (valid) entry. if (this.#max !== undefined && size > this.#max) { - if (this.onEvict) this.onEvict(key, 'too-big'); + if (this.onEvict) this.onEvict(key, 'too-big', value); return false; } @@ -54,7 +62,7 @@ export class ByteLRU { this.#bytes -= rec.size; this.#map.delete(oldestKey); this.#stats.evictions++; - if (this.onEvict) this.onEvict(oldestKey, 'lru'); + if (this.onEvict) this.onEvict(oldestKey, 'lru', rec.value); } return true; @@ -89,12 +97,128 @@ export class ByteLRU { get stats() { return { ...this.#stats, currentBytes: this.#bytes }; } } -// Compute the byte size attributable to a cache entry. Handles the two Percy -// shapes: a single resource object, or an array of resources (root-resource -// captured at multiple widths per discovery.js:465). +// Handles the two Percy cache-entry shapes: single resource, or array of +// roots captured at multiple widths (see discovery.js parseDomResources). export function entrySize(resource, overhead = DEFAULT_PER_ENTRY_OVERHEAD) { if (Array.isArray(resource)) { return resource.reduce((n, r) => n + (r?.content?.length ?? 0) + overhead, 0); } return (resource?.content?.length ?? 0) + overhead; } + +export class DiskSpillStore { + #index = new Map(); + #bytes = 0; + #peakBytes = 0; + #stats = { spilled: 0, restored: 0, spillFailures: 0, readFailures: 0 }; + #counter = 0; + #ready = false; + + constructor(dir, { log } = {}) { + this.dir = dir; + this.log = log; + try { + fs.mkdirSync(dir, { recursive: true }); + this.#ready = true; + } catch (err) { + this.log?.debug?.(`disk-spill init failed for ${dir}: ${err.message}`); + } + } + + // Returns true on success; false on any failure so caller falls back to drop. + // Overwrites prior spill for the same URL — a fresh discovery write wins. + set(url, resource) { + if (!this.#ready) return false; + + let content = resource?.content; + if (content == null) return false; + if (!Buffer.isBuffer(content)) { + try { content = Buffer.from(content); } catch { return false; } + } + + // Counter-based filename keeps URL-derived data out of path.join — + // avoids any path-traversal surface even though sha256 would be safe. + const filepath = path.join(this.dir, String(++this.#counter)); + + try { + fs.writeFileSync(filepath, content); + } catch (err) { + this.#stats.spillFailures++; + this.log?.debug?.(`disk-spill write failed for ${url}: ${err.message}`); + return false; + } + + if (this.#index.has(url)) { + const prev = this.#index.get(url); + this.#bytes -= prev.size; + try { fs.unlinkSync(prev.path); } catch { /* best-effort */ } + } + + const meta = { ...resource }; + delete meta.content; + this.#index.set(url, { path: filepath, size: content.length, meta }); + this.#bytes += content.length; + if (this.#bytes > this.#peakBytes) this.#peakBytes = this.#bytes; + this.#stats.spilled++; + return true; + } + + get(url) { + const entry = this.#index.get(url); + if (!entry) return undefined; + let content; + try { + content = fs.readFileSync(entry.path); + } catch (err) { + this.#stats.readFailures++; + this.log?.debug?.(`disk-spill read failed for ${url}: ${err.message}`); + this.#removeEntry(url, entry); + return undefined; + } + this.#stats.restored++; + return { ...entry.meta, content }; + } + + has(url) { return this.#index.has(url); } + + delete(url) { + const entry = this.#index.get(url); + if (!entry) return false; + this.#removeEntry(url, entry); + return true; + } + + destroy() { + try { + if (this.#ready) fs.rmSync(this.dir, { recursive: true, force: true }); + } catch (err) { + this.log?.debug?.(`disk-spill cleanup failed for ${this.dir}: ${err.message}`); + } + this.#index.clear(); + this.#bytes = 0; + this.#ready = false; + } + + get size() { return this.#index.size; } + get bytes() { return this.#bytes; } + get ready() { return this.#ready; } + get stats() { + return { + ...this.#stats, + currentBytes: this.#bytes, + peakBytes: this.#peakBytes, + entries: this.#index.size + }; + } + + #removeEntry(url, entry) { + this.#bytes -= entry.size; + this.#index.delete(url); + try { fs.unlinkSync(entry.path); } catch { /* best-effort */ } + } +} + +export function createSpillDir() { + const suffix = `${process.pid}-${crypto.randomBytes(4).toString('hex')}`; + return path.join(os.tmpdir(), `percy-cache-${suffix}`); +} diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 69210876d..90b69e016 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -16,7 +16,7 @@ import { isGzipped, maybeScrollToBottom } from './utils.js'; -import { ByteLRU, entrySize } from './cache/byte-lru.js'; +import { ByteLRU, entrySize, DiskSpillStore, createSpillDir } from './cache/byte-lru.js'; import { sha256hash } from '@percy/client/utils'; @@ -392,27 +392,19 @@ export async function* discoverSnapshotResources(queue, options, callback) { }, [])); } -// Used to cache resources across core instances export const RESOURCE_CACHE_KEY = Symbol('resource-cache'); -// Per-run cache stats (hits/misses/evictions/peak + telemetry-gate flags). export const CACHE_STATS_KEY = Symbol('resource-cache-stats'); +export const DISK_SPILL_KEY = Symbol('resource-cache-disk-spill'); -// MAX_RESOURCE_SIZE in network.js is 25MB; a cap below that dooms every -// resource to be skipped. MIN_REASONABLE_CAP_MB warns users picking a -// silently-useless cap. const BYTES_PER_MB = 1_000_000; +// MAX_RESOURCE_SIZE in network.js is 25MB; caps below that would skip every +// resource, so we clamp. MIN_REASONABLE_CAP_MB warns on near-useless caps. const MAX_RESOURCE_SIZE_MB = 25; const MIN_REASONABLE_CAP_MB = 50; -// Default warn threshold when no cap is set: nudge the user toward -// --max-cache-ram before their CI runner OOMs. Override via -// PERCY_CACHE_WARN_THRESHOLD_BYTES for post-ship tuning. const DEFAULT_WARN_THRESHOLD_BYTES = 500 * BYTES_PER_MB; function makeCacheStats() { return { - // The effective cap actually used this run, in MB (after any clamp to the - // 25MB floor). null when the flag was unset. Read by cache_summary telemetry - // so it reports what ran, not what the user typed. effectiveMaxCacheRamMB: null, oversizeSkipped: 0, firstEvictionEventFired: false, @@ -426,8 +418,30 @@ function readWarnThresholdBytes() { return Number.isFinite(raw) && raw > 0 ? raw : DEFAULT_WARN_THRESHOLD_BYTES; } -// Fire-and-forget send-events telemetry. Never blocks discovery, never -// surfaces errors to the user — telemetry loss must not fail a build. +// Cache lookup shared by the network intercept path. RAM miss falls through +// to the disk tier; read failures return undefined so the browser refetches. +// Also resolves the array-valued root-resource shape used for multi-width +// DOM snapshots, regardless of which tier returned it. +export function lookupCacheResource(percy, snapshotResources, cache, url, width) { + let resource = snapshotResources.get(url) || cache.get(url); + const disk = percy[DISK_SPILL_KEY]; + if (!resource && disk) { + resource = disk.get(url); + if (resource) { + percy.log.debug( + `cache disk-hit: ${url} (disk=${disk.size}/` + + `${Math.round(disk.bytes / BYTES_PER_MB)}MB)` + ); + } + } + if (resource && Array.isArray(resource) && resource[0].root) { + const rootResource = resource.find(r => r.widths?.includes(width)); + resource = rootResource || resource[0]; + } + return resource; +} + +// Fire-and-forget: telemetry loss must not fail a build. function fireCacheEventSafe(percy, message, extra) { if (!percy.build?.id) return; Promise.resolve() @@ -446,24 +460,17 @@ export function createDiscoveryQueue(percy) { let { concurrency } = percy.config.discovery; let queue = new Queue('discovery'); let cache; - // Bytes cap (null = unbounded). Validated at queue start. let capBytes = null; - - // Read the unset-cap warning threshold once per queue construction; this - // closure is consulted on every saveResource call, so we avoid reparsing - // process.env each time. + // Read once: saveResource consults this on every call. const warnThreshold = readWarnThresholdBytes(); return queue .set({ concurrency }) - // on start, launch the browser and run the queue .handle('start', async () => { const configuredMaxCacheRamMB = percy.config.discovery.maxCacheRam; let effectiveMaxCacheRamMB = configuredMaxCacheRamMB; - // Startup validation. Runs once per Percy run. The user's config object - // is NOT mutated — the effective value lives on CACHE_STATS_KEY for the - // duration of the run. + // User's config is not mutated; the post-clamp value lives on stats. if (configuredMaxCacheRamMB != null) { if (configuredMaxCacheRamMB < MAX_RESOURCE_SIZE_MB) { percy.log.warn( @@ -495,24 +502,36 @@ export function createDiscoveryQueue(percy) { percy[CACHE_STATS_KEY].effectiveMaxCacheRamMB = capBytes != null ? effectiveMaxCacheRamMB : null; if (capBytes != null) { + // Overflow tier: RAM evictions spill here. diskStore.set returns + // false on any I/O failure → caller falls back to drop automatically. + const diskStore = new DiskSpillStore(createSpillDir(), { log: percy.log }); + percy[DISK_SPILL_KEY] = diskStore; + cache = percy[RESOURCE_CACHE_KEY] = new ByteLRU(capBytes, { - onEvict: (key, reason) => { - if (reason !== 'lru') return; + onEvict: (key, reason, value) => { + if (reason === 'too-big') { + percy[CACHE_STATS_KEY].oversizeSkipped++; + percy.log.debug(`cache skip (oversize): ${key}`); + return; + } + const spilled = diskStore.set(key, value); percy.log.debug( - `cache eviction: evicted ${key} ` + - `(cache now ${Math.round(cache.calculatedSize / BYTES_PER_MB)}` + - `/${effectiveMaxCacheRamMB} MB)` + `cache ${spilled ? 'spill' : 'evict'}: ${key} ` + + `(cache ${Math.round(cache.calculatedSize / BYTES_PER_MB)}` + + `/${effectiveMaxCacheRamMB}MB, entries=${cache.size}, ` + + `disk=${diskStore.size}/${Math.round(diskStore.bytes / BYTES_PER_MB)}MB)` ); const stats = percy[CACHE_STATS_KEY]; if (stats && !stats.firstEvictionEventFired) { stats.firstEvictionEventFired = true; percy.log.info( - 'Cache eviction active — cap reached, oldest entries being dropped.' + 'Cache eviction active — cap reached, oldest entries spilling to disk.' ); fireCacheEventSafe(percy, 'cache_eviction_started', { cache_budget_ram_mb: effectiveMaxCacheRamMB, cache_peak_bytes_seen: cache.stats.peakBytes, - eviction_count: cache.stats.evictions + eviction_count: cache.stats.evictions, + disk_spill_enabled: diskStore.ready }); } } @@ -521,14 +540,16 @@ export function createDiscoveryQueue(percy) { cache = percy[RESOURCE_CACHE_KEY] = new Map(); } - // If browser.launch() fails it will get captured in - // *percy.start() await percy.browser.launch(); queue.run(); }) - // on end, close the browser .handle('end', async () => { await percy.browser.close(); + const diskStore = percy[DISK_SPILL_KEY]; + if (diskStore) { + diskStore.destroy(); + delete percy[DISK_SPILL_KEY]; + } }) // snapshots are unique by name and testCase; when deferred also by widths .handle('find', ({ name, testCase, widths }, snapshot) => ( @@ -574,14 +595,9 @@ export function createDiscoveryQueue(percy) { disableCache: snapshot.discovery.disableCache, allowedHostnames: snapshot.discovery.allowedHostnames, disallowedHostnames: snapshot.discovery.disallowedHostnames, - getResource: (u, width = null) => { - let resource = snapshot.resources.get(u) || cache.get(u); - if (resource && Array.isArray(resource) && resource[0].root) { - const rootResource = resource.find(r => r.widths?.includes(width)); - resource = rootResource || resource[0]; - } - return resource; - }, + getResource: (u, width = null) => ( + lookupCacheResource(percy, snapshot.resources, cache, u, width) + ), saveResource: r => { const limitResources = process.env.LIMIT_SNAPSHOT_RESOURCES || false; const MAX_RESOURCES = Number(process.env.MAX_SNAPSHOT_RESOURCES) || 749; @@ -592,39 +608,29 @@ export function createDiscoveryQueue(percy) { snapshot.resources.set(r.url, r); if (snapshot.discovery.disableCache) return; + // Fresh write supersedes any prior spill — prevents races + // where getResource could serve a stale disk copy. + if (percy[DISK_SPILL_KEY]?.has(r.url)) { + percy[DISK_SPILL_KEY].delete(r.url); + } + if (capBytes != null) { - // Bounded ByteLRU mode — compute size, skip oversized entries - // so a single big resource cannot thrash the cache. - const size = entrySize(r); - if (size > capBytes) { - percy[CACHE_STATS_KEY].oversizeSkipped++; - percy.log.debug( - `Skipping cache for resource ${r.url} ` + - `(${size} bytes > cap ${capBytes})` - ); - return; - } - cache.set(r.url, r, size); + // ByteLRU fires onEvict('too-big') for oversize entries; + // the oversize_skipped stat + debug log live there. + cache.set(r.url, r, entrySize(r)); } else { cache.set(r.url, r); - // Unset-cap path: track running bytes unconditionally so - // cache_summary telemetry reports the true peak. Gate only - // the one-shot warn-log emission on whether we've fired yet. + // Track bytes unconditionally so peak is accurate; + // one-shot warn emission is gated by warningFired. const stats = percy[CACHE_STATS_KEY]; - if (stats) { - stats.unsetModeBytes += entrySize(r); - if (!stats.warningFired && stats.unsetModeBytes >= warnThreshold) { - stats.warningFired = true; - const bytes = stats.unsetModeBytes; - const pretty = bytes >= BYTES_PER_MB - ? `${(bytes / BYTES_PER_MB).toFixed(1)}MB` - : `${Math.round(bytes / 1000)}KB`; - percy.log.warn( - `Percy cache is using ${pretty}. If your CI is ` + - 'memory-constrained, set --max-cache-ram. ' + - 'See https://www.browserstack.com/docs/percy/cli/managing-cache-memory' - ); - } + stats.unsetModeBytes += entrySize(r); + if (!stats.warningFired && stats.unsetModeBytes >= warnThreshold) { + stats.warningFired = true; + percy.log.warn( + `Percy cache is using ${(stats.unsetModeBytes / BYTES_PER_MB).toFixed(1)}MB. ` + + 'If your CI is memory-constrained, set --max-cache-ram. ' + + 'See https://www.browserstack.com/docs/percy/cli/managing-cache-memory' + ); } } } diff --git a/packages/core/src/percy.js b/packages/core/src/percy.js index 4a051ac67..face5e708 100644 --- a/packages/core/src/percy.js +++ b/packages/core/src/percy.js @@ -28,7 +28,8 @@ import { discoverSnapshotResources, createDiscoveryQueue, RESOURCE_CACHE_KEY, - CACHE_STATS_KEY + CACHE_STATS_KEY, + DISK_SPILL_KEY } from './discovery.js'; import Monitoring from '@percy/monitoring'; import { WaitForJob } from './wait-for-job.js'; @@ -385,11 +386,9 @@ export class Percy { } } + // Fire-and-forget cache-usage summary. Telemetry loss is preferable to a + // failed stop, so errors are swallowed. async sendCacheSummary() { - // Fire-and-forget end-of-run summary of asset-discovery cache usage. - // Feeds the Amplitude dashboard (adoption, hit rate, peak bytes). - // Never blocks or fails percy.stop(); telemetry loss is preferable to a - // failed stop. try { if (!this.build?.id) return; const cache = this[RESOURCE_CACHE_KEY]; @@ -397,24 +396,31 @@ export class Percy { if (!cache || !stats) return; const cacheStats = typeof cache.stats === 'object' ? cache.stats : null; - - const extra = { - // Report the effective cap (post-clamp), not the raw config value. - cache_budget_ram_mb: stats.effectiveMaxCacheRamMB, - hits: cacheStats?.hits ?? 0, - misses: cacheStats?.misses ?? 0, - evictions: cacheStats?.evictions ?? 0, - peak_bytes: cacheStats?.peakBytes ?? stats.unsetModeBytes, - final_bytes: cache.calculatedSize ?? stats.unsetModeBytes, - entry_count: cache.size ?? 0, - oversize_skipped: stats.oversizeSkipped - }; + const diskStore = this[DISK_SPILL_KEY]; + const diskStats = diskStore?.stats; await this.client.sendBuildEvents(this.build.id, { message: 'cache_summary', cliVersion: this.client.cliVersion, clientInfo: this.clientInfo, - extra + extra: { + cache_budget_ram_mb: stats.effectiveMaxCacheRamMB, + hits: cacheStats?.hits ?? 0, + misses: cacheStats?.misses ?? 0, + evictions: cacheStats?.evictions ?? 0, + peak_bytes: cacheStats?.peakBytes ?? stats.unsetModeBytes, + final_bytes: cache.calculatedSize ?? stats.unsetModeBytes, + entry_count: cache.size ?? 0, + oversize_skipped: stats.oversizeSkipped, + disk_spill_enabled: !!diskStore?.ready, + disk_spilled_count: diskStats?.spilled ?? 0, + disk_restored_count: diskStats?.restored ?? 0, + disk_spill_failures: diskStats?.spillFailures ?? 0, + disk_read_failures: diskStats?.readFailures ?? 0, + disk_peak_bytes: diskStats?.peakBytes ?? 0, + disk_final_bytes: diskStats?.currentBytes ?? 0, + disk_final_entries: diskStats?.entries ?? 0 + } }); } catch (err) { this.log.debug('cache_summary telemetry failed', err); diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 1a7484714..07e65255c 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -1,8 +1,9 @@ import { sha256hash } from '@percy/client/utils'; import { logger, api, setupTest, createTestServer, dedent, mockRequests } from './helpers/index.js'; import Percy from '@percy/core'; -import { RESOURCE_CACHE_KEY, CACHE_STATS_KEY } from '../src/discovery.js'; -import { ByteLRU } from '../src/cache/byte-lru.js'; +import { RESOURCE_CACHE_KEY, CACHE_STATS_KEY, DISK_SPILL_KEY } from '../src/discovery.js'; +import { ByteLRU, DiskSpillStore } from '../src/cache/byte-lru.js'; +import fs from 'fs'; import Session from '../src/session.js'; import Pako from 'pako'; import * as CoreConfig from '@percy/core/config'; @@ -2385,10 +2386,23 @@ describe('Discovery', () => { expect(cache.stats.evictions).toBeGreaterThan(0); expect(percy[CACHE_STATS_KEY].firstEvictionEventFired).toBe(true); expect(logger.stdout).toEqual(jasmine.arrayContaining([ - jasmine.stringContaining('Cache eviction active') + jasmine.stringContaining('Cache eviction active — cap reached, oldest entries spilling to disk') ])); }); + it('fireCacheEventSafe short-circuits when percy.build is not yet set', async () => { + await startPercyWith({ maxCacheRam: 25 }); + percy.build = undefined; + const spy = spyOn(percy.client, 'sendBuildEvents'); + const cache = percy[RESOURCE_CACHE_KEY]; + const chunk = Math.floor(25_000_000 / 3); + cache.set('a', { content: Buffer.alloc(chunk) }, chunk + 512); + cache.set('b', { content: Buffer.alloc(chunk) }, chunk + 512); + cache.set('c', { content: Buffer.alloc(chunk) }, chunk + 512); + cache.set('d', { content: Buffer.alloc(chunk) }, chunk + 512); + expect(spy).not.toHaveBeenCalled(); + }); + it('fireCacheEventSafe debug-logs and swallows pager rejections', async () => { await logger.mock({ level: 'debug' }); await startPercyWith({ maxCacheRam: 25 }); @@ -2407,41 +2421,140 @@ describe('Discovery', () => { expect(percy.client.sendBuildEvents).toHaveBeenCalled(); }); - it('saveResource debug-logs and increments oversize_skipped for real oversize resources', async () => { + it('records oversize_skipped in stats and logs when an entry is bigger than cap', async () => { + await logger.mock({ level: 'debug' }); await startPercyWith({ maxCacheRam: 25 }); + const cache = percy[RESOURCE_CACHE_KEY]; + // Oversize path: ByteLRU fires onEvict('too-big') which increments + // the stat and logs — upstream network caps at ~16.5MB so this is + // the only way to exercise the branch from core code. + const saved = cache.set('http://x/huge', { content: Buffer.alloc(26_000_001) }, 26_000_001 + 512); + expect(saved).toBe(false); + expect(cache.calculatedSize).toEqual(0); + expect(percy[CACHE_STATS_KEY].oversizeSkipped).toEqual(1); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + jasmine.stringContaining('cache skip (oversize): http://x/huge') + ])); + }); + }); + + describe('with --max-cache-ram disk-spill tier', () => { + async function startWith(discoveryExtras = {}) { + await percy.stop(true); + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { concurrency: 1, ...discoveryExtras } + }); + } + + it('installs a DiskSpillStore alongside ByteLRU when cap is set', async () => { + await startWith({ maxCacheRam: 25 }); + expect(percy[DISK_SPILL_KEY] instanceof DiskSpillStore).toBe(true); + expect(percy[DISK_SPILL_KEY].ready).toBe(true); + expect(fs.existsSync(percy[DISK_SPILL_KEY].dir)).toBe(true); + }); + + it('does not install a DiskSpillStore when no cap is set', () => { + // percy was started in beforeEach without maxCacheRam + expect(percy[DISK_SPILL_KEY]).toBeUndefined(); + }); + + it('spills an LRU-evicted resource to disk instead of dropping it', async () => { await logger.mock({ level: 'debug' }); - // Serve a resource exactly at the per-resource 25MB ceiling. entrySize - // adds 512B overhead so the ByteLRU path sees it as > cap and takes - // the oversize-skip branch in saveResource. - server = await createTestServer({ - '/': () => [200, 'text/html', dedent` - - big - - `], - '/big.bin': () => [200, 'application/octet-stream', Buffer.alloc(25_000_000, 'x')] + await startWith({ maxCacheRam: 25 }); + const cache = percy[RESOURCE_CACHE_KEY]; + const disk = percy[DISK_SPILL_KEY]; + const chunk = Math.floor(25_000_000 / 3); + cache.set('http://x/a', { url: 'http://x/a', content: Buffer.alloc(chunk, 0xaa) }, chunk + 512); + cache.set('http://x/b', { url: 'http://x/b', content: Buffer.alloc(chunk, 0xbb) }, chunk + 512); + cache.set('http://x/c', { url: 'http://x/c', content: Buffer.alloc(chunk, 0xcc) }, chunk + 512); + cache.set('http://x/d', { url: 'http://x/d', content: Buffer.alloc(chunk, 0xdd) }, chunk + 512); + + expect(cache.has('http://x/a')).toBe(false); + expect(disk.has('http://x/a')).toBe(true); + expect(disk.stats.spilled).toBeGreaterThan(0); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + jasmine.stringContaining('cache spill: http://x/a') + ])); + }); + + it('rehydrates a spilled resource byte-for-byte on disk-hit', async () => { + await startWith({ maxCacheRam: 25 }); + const disk = percy[DISK_SPILL_KEY]; + const content = Buffer.from([0, 1, 2, 3, 254, 255, 128]); + disk.set('http://x/spilled', { + url: 'http://x/spilled', mimetype: 'image/png', sha: 'abc', content + }); + const got = disk.get('http://x/spilled'); + expect(got.content.equals(content)).toBe(true); + expect(got.sha).toEqual('abc'); + }); + + it('falls back to drop when disk write fails and emits cache evict debug log', async () => { + await logger.mock({ level: 'debug' }); + await startWith({ maxCacheRam: 25 }); + const cache = percy[RESOURCE_CACHE_KEY]; + const disk = percy[DISK_SPILL_KEY]; + spyOn(fs, 'writeFileSync').and.throwError(new Error('ENOSPC')); + const chunk = Math.floor(25_000_000 / 3); + cache.set('http://x/a', { url: 'http://x/a', content: Buffer.alloc(chunk) }, chunk + 512); + cache.set('http://x/b', { url: 'http://x/b', content: Buffer.alloc(chunk) }, chunk + 512); + cache.set('http://x/c', { url: 'http://x/c', content: Buffer.alloc(chunk) }, chunk + 512); + cache.set('http://x/d', { url: 'http://x/d', content: Buffer.alloc(chunk) }, chunk + 512); + + expect(cache.has('http://x/a')).toBe(false); + expect(disk.has('http://x/a')).toBe(false); + expect(disk.stats.spillFailures).toBeGreaterThan(0); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + jasmine.stringContaining('cache evict:') + ])); + }); + + it('saveResource clears a stale disk entry so fresh writes are not shadowed', async () => { + await startWith({ maxCacheRam: 25 }); + const disk = percy[DISK_SPILL_KEY]; + disk.set('http://localhost:8000/style.css', { + url: 'http://localhost:8000/style.css', + mimetype: 'text/css', + content: Buffer.from('STALE') }); + expect(disk.has('http://localhost:8000/style.css')).toBe(true); + await percy.snapshot({ - name: 'oversize resource', + name: 'stale disk test', url: 'http://localhost:8000', - widths: [1000] + domSnapshot: testDOM }); await percy.idle(); - expect(percy[CACHE_STATS_KEY].oversizeSkipped).toBeGreaterThan(0); - expect(logger.stderr).toEqual(jasmine.arrayContaining([ - jasmine.stringContaining('Skipping cache for resource') - ])); + + expect(disk.has('http://localhost:8000/style.css')).toBe(false); }); - it('records oversize_skipped in stats when an entry is bigger than cap', async () => { - await startPercyWith({ maxCacheRam: 25 }); + it('calls diskStore.destroy and clears the key in the queue end handler', async () => { + await startWith({ maxCacheRam: 25 }); + const disk = percy[DISK_SPILL_KEY]; + const destroySpy = spyOn(disk, 'destroy').and.callThrough(); + await percy.stop(); + expect(destroySpy).toHaveBeenCalled(); + expect(percy[DISK_SPILL_KEY]).toBeUndefined(); + }); + + it('gracefully handles a DiskSpillStore that fails to init', async () => { + spyOn(fs, 'mkdirSync').and.throwError(new Error('EACCES')); + await startWith({ maxCacheRam: 25 }); + const disk = percy[DISK_SPILL_KEY]; + expect(disk.ready).toBe(false); const cache = percy[RESOURCE_CACHE_KEY]; - // Inject a synthetic oversized resource directly — the real network - // path caps individual resources at 25MB so this simulates the edge - // case where entrySize exceeds the cap. - const saved = cache.set('http://x/huge', { content: Buffer.alloc(26_000_001) }, 26_000_001 + 512); - expect(saved).toBe(false); - expect(cache.calculatedSize).toEqual(0); + const chunk = Math.floor(25_000_000 / 3); + expect(() => { + cache.set('http://x/a', { url: 'http://x/a', content: Buffer.alloc(chunk) }, chunk + 512); + cache.set('http://x/b', { url: 'http://x/b', content: Buffer.alloc(chunk) }, chunk + 512); + cache.set('http://x/c', { url: 'http://x/c', content: Buffer.alloc(chunk) }, chunk + 512); + cache.set('http://x/d', { url: 'http://x/d', content: Buffer.alloc(chunk) }, chunk + 512); + }).not.toThrow(); + expect(cache.has('http://x/a')).toBe(false); + expect(disk.has('http://x/a')).toBe(false); }); }); @@ -2551,6 +2664,69 @@ describe('Discovery', () => { })); }); + it('sendCacheSummary reports disk-tier stats when a DiskSpillStore is present', async () => { + percy.build = { id: '123' }; + percy[RESOURCE_CACHE_KEY] = new Map(); + percy[CACHE_STATS_KEY] = { + effectiveMaxCacheRamMB: 25, + oversizeSkipped: 0, + unsetModeBytes: 0 + }; + // destroy is a no-op so the afterEach percy.stop(true) 'end' handler + // does not TypeError when it reaches diskStore.destroy(). + percy[DISK_SPILL_KEY] = { + ready: true, + destroy: () => {}, + stats: { + spilled: 3, + restored: 2, + spillFailures: 1, + readFailures: 0, + currentBytes: 4096, + peakBytes: 8192, + entries: 2 + } + }; + const spy = spyOn(percy.client, 'sendBuildEvents').and.resolveTo(); + await percy.sendCacheSummary(); + expect(spy).toHaveBeenCalledWith('123', jasmine.objectContaining({ + extra: jasmine.objectContaining({ + disk_spill_enabled: true, + disk_spilled_count: 3, + disk_restored_count: 2, + disk_spill_failures: 1, + disk_read_failures: 0, + disk_peak_bytes: 8192, + disk_final_bytes: 4096, + disk_final_entries: 2 + }) + })); + }); + + it('sendCacheSummary reports zeroed disk-tier fields when no DiskSpillStore is present', async () => { + percy.build = { id: '123' }; + percy[RESOURCE_CACHE_KEY] = new Map(); + percy[CACHE_STATS_KEY] = { + effectiveMaxCacheRamMB: null, + oversizeSkipped: 0, + unsetModeBytes: 0 + }; + const spy = spyOn(percy.client, 'sendBuildEvents').and.resolveTo(); + await percy.sendCacheSummary(); + expect(spy).toHaveBeenCalledWith('123', jasmine.objectContaining({ + extra: jasmine.objectContaining({ + disk_spill_enabled: false, + disk_spilled_count: 0, + disk_restored_count: 0, + disk_spill_failures: 0, + disk_read_failures: 0, + disk_peak_bytes: 0, + disk_final_bytes: 0, + disk_final_entries: 0 + }) + })); + }); + it('keeps incrementing unsetModeBytes after the warning has fired', async () => { // Regression: the byte counter used to freeze as soon as the warning // flag flipped, so cache_summary.peak_bytes was always pinned to the diff --git a/packages/core/test/unit/byte-lru.test.js b/packages/core/test/unit/byte-lru.test.js index 6d3aead6c..eb927b2e9 100644 --- a/packages/core/test/unit/byte-lru.test.js +++ b/packages/core/test/unit/byte-lru.test.js @@ -1,4 +1,13 @@ -import { ByteLRU, entrySize } from '../../src/cache/byte-lru.js'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { + ByteLRU, + entrySize, + DiskSpillStore, + createSpillDir +} from '../../src/cache/byte-lru.js'; +import { DISK_SPILL_KEY, lookupCacheResource } from '../../src/discovery.js'; describe('Unit / ByteLRU', () => { describe('unbounded mode (no cap)', () => { @@ -75,13 +84,6 @@ describe('Unit / ByteLRU', () => { expect(c.calculatedSize).toEqual(50); }); - it('onEvict fires with reason "too-big" on oversize', () => { - const reasons = []; - const c = new ByteLRU(100, { onEvict: (k, r) => reasons.push({ k, r }) }); - c.set('huge', 'HUGE', 200); - expect(reasons).toEqual([{ k: 'huge', r: 'too-big' }]); - }); - it('oversized re-set of an existing key leaves the prior entry intact', () => { const c = new ByteLRU(100); c.set('k', 'small', 50); @@ -93,8 +95,30 @@ describe('Unit / ByteLRU', () => { }); }); + describe('onEvict', () => { + it('fires with reason "too-big" and the value on oversize', () => { + const evicted = []; + const c = new ByteLRU(100, { + onEvict: (k, r, v) => evicted.push({ k, r, v }) + }); + c.set('huge', { body: 'HUGE' }, 200); + expect(evicted).toEqual([{ k: 'huge', r: 'too-big', v: { body: 'HUGE' } }]); + }); + + it('fires with reason "lru" and the evicted value when over budget', () => { + const evicted = []; + const c = new ByteLRU(200, { + onEvict: (k, r, v) => evicted.push({ k, r, v }) + }); + c.set('a', { body: 'A' }, 100); + c.set('b', { body: 'B' }, 100); + c.set('c', { body: 'C' }, 100); + expect(evicted).toEqual([{ k: 'a', r: 'lru', v: { body: 'A' } }]); + }); + }); + describe('.values()', () => { - it('iterates yielding plain values (Percy discovery.test.js call-site shape)', () => { + it('iterates yielding plain values', () => { const c = new ByteLRU(); c.set('a', { root: true }, 100); c.set('b', { root: false }, 100); @@ -132,18 +156,17 @@ describe('Unit / ByteLRU', () => { const c = new ByteLRU(1000); c.set('a', 'A', 100); expect(c.delete('missing')).toBe(false); - // existing entry untouched expect(c.calculatedSize).toEqual(100); }); }); describe('stats', () => { - it('peakBytes captures transient high-water before eviction (honest reporting)', () => { + it('peakBytes captures transient high-water before eviction', () => { const c = new ByteLRU(300); c.set('a', 'A', 100); c.set('b', 'B', 100); c.set('c', 'C', 100); - c.set('d', 'D', 100); // transient 400, then evict → 300 + c.set('d', 'D', 100); c.delete('b'); c.delete('c'); c.delete('d'); @@ -210,11 +233,10 @@ describe('Unit / entrySize', () => { }); it('tolerates null entries and missing content fields inside an array', () => { - // Covers the optional-chain branches in the reduce callback. const arr = [ null, - {}, // no content - { content: Buffer.alloc(100) } // populated + {}, + { content: Buffer.alloc(100) } ]; expect(entrySize(arr)).toEqual(100 + 3 * 512); }); @@ -223,3 +245,330 @@ describe('Unit / entrySize', () => { expect(entrySize({ content: Buffer.alloc(100) }, 0)).toEqual(100); }); }); + +describe('Unit / DiskSpillStore', () => { + function makeResource(url, content, extra = {}) { + return { + url, + sha: 'deadbeef', + mimetype: 'text/css', + content: Buffer.isBuffer(content) ? content : Buffer.from(content), + ...extra + }; + } + + function makeLog() { + const calls = []; + return { calls, debug: (m) => calls.push(m) }; + } + + function freshDir() { + return path.join( + os.tmpdir(), + `disk-spill-test-${process.pid}-${Math.random().toString(36).slice(2, 10)}` + ); + } + + let dir; + let store; + + afterEach(() => { + store?.destroy(); + if (dir && fs.existsSync(dir)) fs.rmSync(dir, { recursive: true, force: true }); + }); + + describe('construction', () => { + it('creates the target directory', () => { + dir = freshDir(); + store = new DiskSpillStore(dir); + expect(fs.existsSync(dir)).toBe(true); + expect(store.ready).toBe(true); + }); + + it('swallows mkdir failure and marks itself not-ready', () => { + const log = makeLog(); + store = new DiskSpillStore('/dev/null/cannot-mkdir-here', { log }); + expect(store.ready).toBe(false); + expect(log.calls.some(m => m.includes('init failed'))).toBe(true); + }); + + it('short-circuits set() when not ready', () => { + store = new DiskSpillStore('/dev/null/cannot-mkdir-here'); + expect(store.set('http://x/a', makeResource('http://x/a', 'A'))).toBe(false); + expect(store.stats.spillFailures).toEqual(0); + }); + + it('works without a log option', () => { + // Covers the optional-chain branches on this.log?.debug?.() calls. + const badDir = '/dev/null/cannot-mkdir-here'; + const silent = new DiskSpillStore(badDir); + expect(silent.ready).toBe(false); + expect(silent.set('http://x/a', makeResource('http://x/a', 'A'))).toBe(false); + }); + }); + + describe('set + get round-trip', () => { + beforeEach(() => { + dir = freshDir(); + store = new DiskSpillStore(dir); + }); + + it('preserves binary content byte-for-byte', () => { + const bin = Buffer.from([0, 1, 2, 253, 254, 255, 0, 127]); + store.set('http://x/bin', makeResource('http://x/bin', bin, { mimetype: 'image/png' })); + const got = store.get('http://x/bin'); + expect(got.content.equals(bin)).toBe(true); + expect(got.mimetype).toEqual('image/png'); + expect(got.url).toEqual('http://x/bin'); + }); + + it('coerces non-Buffer content via Buffer.from', () => { + store.set('http://x/str', { url: 'http://x/str', mimetype: 'text/html', content: 'hello' }); + expect(store.get('http://x/str').content.toString()).toEqual('hello'); + }); + + it('returns false when Buffer.from coercion throws', () => { + // A symbol cannot be coerced to a Buffer. + const badContent = Symbol('x'); + expect(store.set('http://x/bad', { content: badContent })).toBe(false); + }); + + it('returns undefined for unknown urls', () => { + expect(store.get('http://x/missing')).toBeUndefined(); + }); + + it('returns false when content is null/undefined', () => { + expect(store.set('http://x/nil', { url: 'http://x/nil' })).toBe(false); + expect(store.set('http://x/nil2', null)).toBe(false); + }); + + it('carries resource metadata through the round-trip', () => { + store.set( + 'http://x/root', + makeResource('http://x/root', 'root-html', { root: true, widths: [1280], sha: 'abc123' }) + ); + const got = store.get('http://x/root'); + expect(got.root).toBe(true); + expect(got.widths).toEqual([1280]); + expect(got.sha).toEqual('abc123'); + }); + + it('increments spilled/restored counters', () => { + store.set('http://x/a', makeResource('http://x/a', 'A')); + store.set('http://x/b', makeResource('http://x/b', 'B')); + store.get('http://x/a'); + store.get('http://x/a'); + store.get('http://x/missing'); + expect(store.stats.spilled).toEqual(2); + expect(store.stats.restored).toEqual(2); + }); + }); + + describe('accounting', () => { + beforeEach(() => { + dir = freshDir(); + store = new DiskSpillStore(dir); + }); + + it('tracks bytes and peak', () => { + store.set('http://x/a', makeResource('http://x/a', Buffer.alloc(1000))); + store.set('http://x/b', makeResource('http://x/b', Buffer.alloc(2000))); + expect(store.bytes).toEqual(3000); + expect(store.stats.peakBytes).toEqual(3000); + store.delete('http://x/a'); + expect(store.bytes).toEqual(2000); + expect(store.stats.peakBytes).toEqual(3000); + }); + + it('replaces an existing URL and fixes up byte accounting', () => { + store.set('http://x/a', makeResource('http://x/a', Buffer.alloc(1000))); + store.set('http://x/a', makeResource('http://x/a', Buffer.alloc(500))); + expect(store.bytes).toEqual(500); + expect(store.size).toEqual(1); + expect(store.get('http://x/a').content.length).toEqual(500); + }); + + it('silently tolerates unlinkSync errors during overwrite', () => { + // Covers the best-effort unlink branch in the overwrite path. + store.set('http://x/a', makeResource('http://x/a', 'A')); + const spy = spyOn(fs, 'unlinkSync').and.throwError(new Error('EBUSY')); + try { + expect(() => store.set('http://x/a', makeResource('http://x/a', 'B'))).not.toThrow(); + } finally { spy.and.callThrough(); } + expect(store.get('http://x/a').content.toString()).toEqual('B'); + }); + }); + + describe('failure handling', () => { + beforeEach(() => { + dir = freshDir(); + store = new DiskSpillStore(dir); + }); + + it('returns false and increments spillFailures on write error', () => { + const log = makeLog(); + const localStore = new DiskSpillStore(dir, { log }); + const spy = spyOn(fs, 'writeFileSync').and.throwError(new Error('EACCES')); + try { + const ok = localStore.set('http://x/a', makeResource('http://x/a', 'A')); + expect(ok).toBe(false); + expect(localStore.stats.spillFailures).toEqual(1); + expect(log.calls.some(m => m.includes('write failed'))).toBe(true); + } finally { spy.and.callThrough(); } + }); + + it('self-heals the index on read failure', () => { + const log = makeLog(); + const localStore = new DiskSpillStore(dir, { log }); + localStore.set('http://x/a', makeResource('http://x/a', 'A')); + expect(localStore.has('http://x/a')).toBe(true); + + const spy = spyOn(fs, 'readFileSync').and.throwError(new Error('ENOENT')); + try { + const got = localStore.get('http://x/a'); + expect(got).toBeUndefined(); + expect(localStore.has('http://x/a')).toBe(false); + expect(localStore.stats.readFailures).toEqual(1); + expect(log.calls.some(m => m.includes('read failed'))).toBe(true); + } finally { spy.and.callThrough(); } + }); + }); + + describe('delete + destroy', () => { + beforeEach(() => { + dir = freshDir(); + store = new DiskSpillStore(dir); + }); + + it('delete removes both file and index entry, is idempotent', () => { + store.set('http://x/a', makeResource('http://x/a', 'A')); + expect(fs.readdirSync(dir).length).toEqual(1); + expect(store.delete('http://x/a')).toBe(true); + expect(fs.readdirSync(dir).length).toEqual(0); + expect(store.has('http://x/a')).toBe(false); + expect(store.delete('http://x/a')).toBe(false); + }); + + it('delete silently tolerates unlinkSync errors', () => { + store.set('http://x/a', makeResource('http://x/a', 'A')); + const spy = spyOn(fs, 'unlinkSync').and.throwError(new Error('EBUSY')); + try { + expect(() => store.delete('http://x/a')).not.toThrow(); + } finally { spy.and.callThrough(); } + }); + + it('destroy removes the entire dir and clears index', () => { + store.set('http://x/a', makeResource('http://x/a', 'A')); + store.set('http://x/b', makeResource('http://x/b', 'B')); + store.destroy(); + expect(fs.existsSync(dir)).toBe(false); + expect(store.size).toEqual(0); + expect(store.ready).toBe(false); + }); + + it('destroy swallows rm errors', () => { + const log = makeLog(); + const localStore = new DiskSpillStore(dir, { log }); + const spy = spyOn(fs, 'rmSync').and.throwError(new Error('EBUSY')); + try { + expect(() => localStore.destroy()).not.toThrow(); + expect(log.calls.some(m => m.includes('cleanup failed'))).toBe(true); + } finally { spy.and.callThrough(); } + }); + + it('destroy is a no-op when the store was not ready', () => { + const notReady = new DiskSpillStore('/dev/null/cannot-mkdir-here'); + const rmSpy = spyOn(fs, 'rmSync'); + try { + notReady.destroy(); + expect(rmSpy).not.toHaveBeenCalled(); + } finally { rmSpy.and.callThrough(); } + }); + }); + + describe('createSpillDir', () => { + it('returns a unique path under os.tmpdir() with a percy-cache prefix', () => { + const a = createSpillDir(); + const b = createSpillDir(); + expect(a).not.toEqual(b); + expect(a.startsWith(os.tmpdir())).toBe(true); + expect(path.basename(a).startsWith('percy-cache-')).toBe(true); + }); + }); +}); + +describe('Unit / lookupCacheResource', () => { + function makePercy(disk) { + const logs = []; + const percy = { + log: { debug: (m) => logs.push(m) }, + [DISK_SPILL_KEY]: disk + }; + return { percy, logs }; + } + + it('returns a snapshot-local resource first', () => { + const { percy } = makePercy(undefined); + const local = { url: 'a', mimetype: 'text/css', content: Buffer.from('L') }; + const snapshotResources = new Map([['a', local]]); + const cache = new ByteLRU(); + expect(lookupCacheResource(percy, snapshotResources, cache, 'a')).toBe(local); + }); + + it('falls through to RAM cache when snapshot has no entry', () => { + const { percy } = makePercy(undefined); + const cache = new ByteLRU(); + const cached = { url: 'a', content: Buffer.from('C') }; + cache.set('a', cached, 100); + expect(lookupCacheResource(percy, new Map(), cache, 'a')).toBe(cached); + }); + + it('falls through to disk when both snapshot and RAM miss', () => { + const dir = path.join(os.tmpdir(), `lookup-test-${process.pid}-${Math.random().toString(36).slice(2, 8)}`); + const disk = new DiskSpillStore(dir); + try { + disk.set('a', { url: 'a', mimetype: 'text/css', content: Buffer.from('DISK') }); + const { percy, logs } = makePercy(disk); + const got = lookupCacheResource(percy, new Map(), new ByteLRU(), 'a'); + expect(got.content.toString()).toEqual('DISK'); + expect(logs.some(m => m.includes('cache disk-hit: a'))).toBe(true); + } finally { disk.destroy(); } + }); + + it('returns undefined on full miss', () => { + const { percy } = makePercy(undefined); + expect(lookupCacheResource(percy, new Map(), new ByteLRU(), 'missing')).toBeUndefined(); + }); + + it('returns undefined when disk is present but url is absent', () => { + const dir = path.join(os.tmpdir(), `lookup-test-${process.pid}-${Math.random().toString(36).slice(2, 8)}`); + const disk = new DiskSpillStore(dir); + try { + const { percy, logs } = makePercy(disk); + expect(lookupCacheResource(percy, new Map(), new ByteLRU(), 'missing')).toBeUndefined(); + expect(logs.length).toEqual(0); + } finally { disk.destroy(); } + }); + + it('picks the width-matching entry from an array-valued root resource', () => { + const { percy } = makePercy(undefined); + const arr = [ + { root: true, widths: [375], content: Buffer.from('small') }, + { root: true, widths: [1280], content: Buffer.from('wide') } + ]; + const snapshotResources = new Map([['root', arr]]); + const cache = new ByteLRU(); + const got = lookupCacheResource(percy, snapshotResources, cache, 'root', 1280); + expect(got.content.toString()).toEqual('wide'); + }); + + it('falls back to the first array entry when no width matches', () => { + const { percy } = makePercy(undefined); + const arr = [ + { root: true, widths: [375], content: Buffer.from('A') }, + { root: true, widths: [1280], content: Buffer.from('B') } + ]; + const got = lookupCacheResource(percy, new Map([['root', arr]]), new ByteLRU(), 'root', 9999); + expect(got.content.toString()).toEqual('A'); + }); +}); From 2422b01d7335cd4eb7415907104b28e431834063 Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Fri, 24 Apr 2026 05:58:34 +0530 Subject: [PATCH 13/14] 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. --- packages/core/src/discovery.js | 6 +++ packages/core/src/percy.js | 22 +++++---- packages/core/test/discovery.test.js | 73 ++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 10 deletions(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 90b69e016..d1e40f9be 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -547,6 +547,12 @@ export function createDiscoveryQueue(percy) { await percy.browser.close(); const diskStore = percy[DISK_SPILL_KEY]; if (diskStore) { + // Snapshot final stats before destroy() clears them — sendCacheSummary + // runs after this handler, and reads from stats.finalDiskStats. + const stats = percy[CACHE_STATS_KEY]; + if (stats) { + stats.finalDiskStats = { ...diskStore.stats, ready: diskStore.ready }; + } diskStore.destroy(); delete percy[DISK_SPILL_KEY]; } diff --git a/packages/core/src/percy.js b/packages/core/src/percy.js index face5e708..b1f96b338 100644 --- a/packages/core/src/percy.js +++ b/packages/core/src/percy.js @@ -396,9 +396,11 @@ export class Percy { if (!cache || !stats) return; const cacheStats = typeof cache.stats === 'object' ? cache.stats : null; + // diskStore is destroyed by discovery 'end' before this runs, so fall + // back to the snapshot captured in stats.finalDiskStats. const diskStore = this[DISK_SPILL_KEY]; - const diskStats = diskStore?.stats; - + const diskSnap = diskStore?.stats ?? stats.finalDiskStats; + const diskReady = diskStore ? diskStore.ready : !!stats.finalDiskStats?.ready; await this.client.sendBuildEvents(this.build.id, { message: 'cache_summary', cliVersion: this.client.cliVersion, @@ -412,14 +414,14 @@ export class Percy { final_bytes: cache.calculatedSize ?? stats.unsetModeBytes, entry_count: cache.size ?? 0, oversize_skipped: stats.oversizeSkipped, - disk_spill_enabled: !!diskStore?.ready, - disk_spilled_count: diskStats?.spilled ?? 0, - disk_restored_count: diskStats?.restored ?? 0, - disk_spill_failures: diskStats?.spillFailures ?? 0, - disk_read_failures: diskStats?.readFailures ?? 0, - disk_peak_bytes: diskStats?.peakBytes ?? 0, - disk_final_bytes: diskStats?.currentBytes ?? 0, - disk_final_entries: diskStats?.entries ?? 0 + disk_spill_enabled: diskReady, + disk_spilled_count: diskSnap?.spilled ?? 0, + disk_restored_count: diskSnap?.restored ?? 0, + disk_spill_failures: diskSnap?.spillFailures ?? 0, + disk_read_failures: diskSnap?.readFailures ?? 0, + disk_peak_bytes: diskSnap?.peakBytes ?? 0, + disk_final_bytes: diskSnap?.currentBytes ?? 0, + disk_final_entries: diskSnap?.entries ?? 0 } }); } catch (err) { diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 07e65255c..daca8243a 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -2703,6 +2703,79 @@ describe('Discovery', () => { })); }); + it('sendCacheSummary falls back to stats.finalDiskStats after discovery.end destroys the diskStore', async () => { + // Real-build ordering: discovery 'end' destroys the diskStore and + // deletes DISK_SPILL_KEY before sendCacheSummary runs. The discovery + // 'end' handler is responsible for snapshotting the disk stats onto + // stats.finalDiskStats so sendCacheSummary can still populate the + // telemetry payload. + percy.build = { id: '123' }; + percy[RESOURCE_CACHE_KEY] = new Map(); + percy[CACHE_STATS_KEY] = { + effectiveMaxCacheRamMB: 25, + oversizeSkipped: 0, + unsetModeBytes: 0, + finalDiskStats: { + ready: true, + spilled: 97, + restored: 96, + spillFailures: 0, + readFailures: 0, + currentBytes: 0, + peakBytes: 36003060, + entries: 0 + } + }; + // DISK_SPILL_KEY intentionally unset — simulates post-destroy state. + const spy = spyOn(percy.client, 'sendBuildEvents').and.resolveTo(); + await percy.sendCacheSummary(); + expect(spy).toHaveBeenCalledWith('123', jasmine.objectContaining({ + extra: jasmine.objectContaining({ + disk_spill_enabled: true, + disk_spilled_count: 97, + disk_restored_count: 96, + disk_peak_bytes: 36003060, + disk_final_bytes: 0, + disk_final_entries: 0 + }) + })); + }); + + it('discovery end handler snapshots diskStore.stats onto stats.finalDiskStats before destroy', async () => { + // Ensures the fix covering the sendCacheSummary ordering is wired into + // the discovery queue's 'end' handler so real builds preserve the data. + await percy.stop(true); + percy = await Percy.start({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { concurrency: 1, maxCacheRam: 25 } + }); + const stats = percy[CACHE_STATS_KEY]; + const disk = percy[DISK_SPILL_KEY]; + spyOn(disk, 'destroy').and.callThrough(); + // Seed fake stats onto the disk store so we can verify snapshot copies them. + disk._testStats = { spilled: 5, restored: 3, peakBytes: 1234 }; + Object.defineProperty(disk, 'stats', { + configurable: true, + get: () => ({ + ...disk._testStats, + spillFailures: 0, + readFailures: 0, + currentBytes: 0, + entries: 0 + }) + }); + await percy.stop(); + expect(stats.finalDiskStats).toEqual(jasmine.objectContaining({ + ready: true, + spilled: 5, + restored: 3, + peakBytes: 1234 + })); + expect(disk.destroy).toHaveBeenCalled(); + expect(percy[DISK_SPILL_KEY]).toBeUndefined(); + }); + it('sendCacheSummary reports zeroed disk-tier fields when no DiskSpillStore is present', async () => { percy.build = { id: '123' }; percy[RESOURCE_CACHE_KEY] = new Map(); From 8a8d2ca87f2bf220fb0ab6684ca5595891dda96a Mon Sep 17 00:00:00 2001 From: Pranav Zinzurde Date: Fri, 24 Apr 2026 08:51:35 +0530 Subject: [PATCH 14/14] fix(core): drop dead stats guard in disk-spill end handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/core/src/discovery.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index d1e40f9be..173b5ce4b 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -549,10 +549,11 @@ export function createDiscoveryQueue(percy) { if (diskStore) { // Snapshot final stats before destroy() clears them — sendCacheSummary // runs after this handler, and reads from stats.finalDiskStats. - const stats = percy[CACHE_STATS_KEY]; - if (stats) { - stats.finalDiskStats = { ...diskStore.stats, ready: diskStore.ready }; - } + // CACHE_STATS_KEY is always set alongside DISK_SPILL_KEY in 'start'. + percy[CACHE_STATS_KEY].finalDiskStats = { + ...diskStore.stats, + ready: diskStore.ready + }; diskStore.destroy(); delete percy[DISK_SPILL_KEY]; }