diff --git a/packages/sdk-utils/src/index.js b/packages/sdk-utils/src/index.js index a4d5aec40..639b0b357 100644 --- a/packages/sdk-utils/src/index.js +++ b/packages/sdk-utils/src/index.js @@ -15,6 +15,7 @@ import { waitForReadyScript, getReadinessConfig, isReadinessDisabled, + readinessDeadlineMs, runReadinessGate } from './serialize-dom.js'; @@ -111,6 +112,7 @@ export { waitForReadyScript, getReadinessConfig, isReadinessDisabled, + readinessDeadlineMs, runReadinessGate }; diff --git a/packages/sdk-utils/src/serialize-dom.js b/packages/sdk-utils/src/serialize-dom.js index ef7a15a3e..937ea38b7 100644 --- a/packages/sdk-utils/src/serialize-dom.js +++ b/packages/sdk-utils/src/serialize-dom.js @@ -106,17 +106,99 @@ export function waitForReadyScript(readinessConfig = {}, { callback = false } = // options, // { callback: true, log } // ); +// Effective in-page timeout for each readiness preset, mirroring PRESETS in +// @percy/dom's readiness.js. Duplicated here because the presets live in the +// browser bundle, and the Node side needs the number to size its own deadline. +const PRESET_TIMEOUT_MS = { balanced: 10000, strict: 30000, fast: 5000 }; + +// Added to the in-page timeout before the Node side gives up, so a gate that is +// merely slow (eval round-trip, check teardown) is never pre-empted. +const READINESS_DEADLINE_GRACE_MS = 3000; + +// Wall-clock budget the Node side gives the readiness eval. +// +// PercyDOM.waitForReady() bounds itself with an in-page `setTimeout`, and every +// individual check settles on a `setTimeout`/`setInterval`. On a page whose +// timers are faked and paused -- Playwright's `page.clock.pauseAt()`, sinon +// `useFakeTimers`, jest fake timers -- neither the checks nor the gate's own +// timeout can ever fire, so the eval stays pending for the life of the page and +// hangs the test that called percySnapshot(). Node is the only side of that +// boundary guaranteed to have a real clock, so the backstop belongs here. +export function readinessDeadlineMs(readinessConfig = {}) { + let timeout = readinessConfig.timeoutMs ?? readinessConfig.timeout_ms ?? + PRESET_TIMEOUT_MS[readinessConfig.preset] ?? PRESET_TIMEOUT_MS.balanced; + let max = readinessConfig.maxTimeoutMs ?? readinessConfig.max_timeout_ms; + if (max != null) timeout = Math.min(timeout, max); + return timeout + READINESS_DEADLINE_GRACE_MS; +} + +// Captured once at module load, because the deadline below must not be the very +// thing a faked clock disables. jest's and sinon's fake timers replace the +// *global* `setTimeout` binding, and a bare `setTimeout(...)` call resolves that +// global at call time -- so a consumer whose Node test process has fake timers +// installed would schedule the deadline on a frozen clock and hang exactly as +// before, one layer up from the in-page freeze this gate exists to survive. +// +// The capture holds for the ordinary case: the SDK imports this module at +// require time, before a test body or beforeEach reaches `useFakeTimers()`. It +// is NOT a guarantee. Fake timers installed before this module is first +// evaluated -- jest's `fakeTimers: { enableGlobally: true }`, a `useFakeTimers()` +// call in `setupFiles`, or `resetModules()` + a fresh require under an already +// faked clock -- capture the fake, and the hang returns. Those consumers need +// `snapshot.readiness.preset: disabled`. +// +// Deliberately not `import { setTimeout } from 'node:timers'`, which would be +// immune to import order too: this file is imported statically by index.js, and +// index.js is the rollup entry for the browser bundle (see the package's +// `browser` field). Node-only code in this package is always reached through a +// lazy `await import(...)` -- `http`/`https` in request.js, `./proxy.js` and its +// `net`/`tls` imports -- precisely to keep builtins out of that graph. A static +// builtin import here would not even fail the build, since the rollup config +// suppresses MISSING_NODE_BUILTINS; it would ship a browser bundle that breaks +// at runtime, which is worse. +const nativeSetTimeout = globalThis.setTimeout; +const nativeClearTimeout = globalThis.clearTimeout; + +const READINESS_DEADLINE_HIT = Symbol('readiness-deadline'); + export async function runReadinessGate(evalScript, snapshotOptions = {}, { callback = false, log } = {}) { if (isReadinessDisabled(snapshotOptions)) return null; const config = getReadinessConfig(snapshotOptions); const script = waitForReadyScript(config, { callback }); + const deadline = readinessDeadlineMs(config); + let timer; + try { - return await evalScript(script); + const evaluation = Promise.resolve(evalScript(script)); + // Hitting the deadline abandons `evaluation`, which may still reject much + // later -- when the driver tears the page down at end of test, say. Swallow + // that here so it never surfaces as an unhandled rejection. + evaluation.catch(() => {}); + + const result = await Promise.race([ + evaluation, + new Promise(resolve => { timer = nativeSetTimeout(() => resolve(READINESS_DEADLINE_HIT), deadline); }) + ]); + + if (result === READINESS_DEADLINE_HIT) { + if (log && typeof log.debug === 'function') { + log.debug( + `waitForReady did not settle within ${deadline}ms, proceeding to serialize. ` + + 'If this page fakes or pauses timers (e.g. Playwright page.clock), disable ' + + 'the readiness gate with snapshot.readiness.preset: disabled.' + ); + } + return null; + } + + return result; } catch (err) { if (log && typeof log.debug === 'function') { log.debug(`waitForReady failed, proceeding to serialize: ${err?.message || err}`); } return null; + } finally { + nativeClearTimeout(timer); } } diff --git a/packages/sdk-utils/test/index.test.js b/packages/sdk-utils/test/index.test.js index 11ad23782..776d3ab12 100644 --- a/packages/sdk-utils/test/index.test.js +++ b/packages/sdk-utils/test/index.test.js @@ -1043,6 +1043,150 @@ describe('SDK Utils', () => { let result = await runReadinessGate(() => Promise.reject(new Error('no log')), {}); expect(result).toBe(null); }); + + // PER-10756: PercyDOM.waitForReady() enforces its own timeout with an + // in-page setTimeout, so a page with faked/paused timers (Playwright + // page.clock.pauseAt, sinon useFakeTimers) can never settle the gate. The + // Node-side deadline is what keeps percySnapshot() from hanging the test. + it('returns null when evalScript never settles, without blocking on it', async () => { + let logged; + let result = await runReadinessGate( + () => new Promise(() => {}), + { readiness: { timeoutMs: 1000 } }, + { log: { debug: (m) => { logged = m; } } } + ); + expect(result).toBe(null); + expect(logged).toContain('did not settle within 4000ms'); + expect(logged).toContain('preset: disabled'); + }); + + it('does not raise an unhandled rejection when an abandoned eval rejects later', async () => { + let rejectEval; + let result = await runReadinessGate( + () => new Promise((resolve, reject) => { rejectEval = reject; }), + { readiness: { timeoutMs: 1000 } } + ); + expect(result).toBe(null); + // The driver tearing the page down after the deadline must stay silent. + rejectEval(new Error('Target page, context or browser has been closed')); + await new Promise(resolve => setTimeout(resolve, 10)); + }); + + // Regression: the deadline must not be schedulable on a clock the consumer + // has frozen. jest/sinon fake timers replace the global setTimeout binding, + // so a bare setTimeout(...) here would land on the frozen clock and hang + // exactly as the in-page gate does -- see the module-scope capture. + it('survives Node-side fake timers replacing the global setTimeout', async () => { + let realSetTimeout = globalThis.setTimeout; + let realClearTimeout = globalThis.clearTimeout; + let scheduledOnFrozenClock = 0; + + // Stand in for useFakeTimers() with no clock advancement: replace the + // globals with timers that are recorded and never fire. + globalThis.setTimeout = () => { scheduledOnFrozenClock++; return 0; }; + globalThis.clearTimeout = () => {}; + + try { + // Bound the wait on the REAL timer. Without it, a regression here (the + // deadline scheduled on the frozen clock) never settles the await, so + // the finally below never restores the globals and every later spec + // using a bare setTimeout hangs too — one regression becomes a + // suite-wide cascade instead of one clean failure. 6000ms is above the + // real 4000ms deadline (timeoutMs 1000 + 3000ms grace) so a healthy run + // resolves on its own, and below jasmine's 10s spec timeout so this + // rejection, not the runner, is what reports the regression. + let result = await Promise.race([ + runReadinessGate( + () => new Promise(() => {}), + { readiness: { timeoutMs: 1000 } } + ), + new Promise((resolve, reject) => realSetTimeout(() => reject(new Error( + 'readiness deadline never fired: it was scheduled on the frozen global clock ' + + 'instead of the timer captured at module load' + )), 6000)) + ]); + expect(result).toBe(null); + // Nothing was handed to the frozen clock — the capture was used. + expect(scheduledOnFrozenClock).toBe(0); + } finally { + globalThis.setTimeout = realSetTimeout; + globalThis.clearTimeout = realClearTimeout; + } + }); + + it('still returns diagnostics from an eval that settles before the deadline', async () => { + let diagnostics = { passed: true, timed_out: false, preset: 'balanced' }; + let result = await runReadinessGate( + () => new Promise(resolve => setTimeout(() => resolve(diagnostics), 10)), + { readiness: { timeoutMs: 1000 } } + ); + expect(result).toEqual(diagnostics); + }); + }); + + describe('readinessDeadlineMs(readinessConfig)', () => { + let { readinessDeadlineMs } = utils; + + it('defaults to the balanced preset timeout plus grace', () => { + expect(readinessDeadlineMs()).toBe(13000); + expect(readinessDeadlineMs({})).toBe(13000); + expect(readinessDeadlineMs({ preset: 'balanced' })).toBe(13000); + }); + + it('uses the timeout of the named preset', () => { + expect(readinessDeadlineMs({ preset: 'strict' })).toBe(33000); + expect(readinessDeadlineMs({ preset: 'fast' })).toBe(8000); + }); + + it('falls back to balanced for an unknown preset', () => { + expect(readinessDeadlineMs({ preset: 'nonsense' })).toBe(13000); + }); + + it('prefers an explicit timeout over the preset, in either naming', () => { + expect(readinessDeadlineMs({ preset: 'strict', timeoutMs: 5000 })).toBe(8000); + expect(readinessDeadlineMs({ preset: 'strict', timeout_ms: 5000 })).toBe(8000); + }); + + it('clamps to maxTimeoutMs when that is lower', () => { + expect(readinessDeadlineMs({ timeoutMs: 20000, maxTimeoutMs: 6000 })).toBe(9000); + expect(readinessDeadlineMs({ timeoutMs: 20000, max_timeout_ms: 6000 })).toBe(9000); + // A higher cap leaves the configured timeout alone. + expect(readinessDeadlineMs({ timeoutMs: 4000, maxTimeoutMs: 30000 })).toBe(7000); + }); + + // Node-only, same rationale as the serialize-frames parity test above: + // PRESET_TIMEOUT_MS mirrors PRESETS in @percy/dom's readiness.js, which + // only exists in the browser bundle. Read the dom source and assert the + // timeouts still match, so drift fails loudly instead of silently + // producing a Node deadline shorter than the in-page one. + const isNodeEnv = typeof process !== 'undefined' && + typeof process.cwd === 'function' && + !!(process.versions && process.versions.node); + const itNodeEnv = isNodeEnv ? it : xit; + + itNodeEnv('stays in lockstep with the presets in @percy/dom/src/readiness.js', async () => { + const fs = await import('fs'); + const path = await import('path'); + const domSource = fs.readFileSync( + path.resolve(process.cwd(), '../dom/src/readiness.js'), + 'utf8' + ); + // Pull `timeout_ms:` out of each preset block in PRESETS. + const presets = domSource + .slice(domSource.indexOf('const PRESETS = {')) + .match(/(balanced|strict|fast):\s*\{[^}]*?timeout_ms:\s*(\d+)/g) + .reduce((acc, block) => { + const [, name, ms] = block.match(/(balanced|strict|fast):[\s\S]*timeout_ms:\s*(\d+)/); + acc[name] = Number(ms); + return acc; + }, {}); + + expect(presets).toEqual({ balanced: 10000, strict: 30000, fast: 5000 }); + // Every dom preset timeout must be the base of our deadline. + for (const [preset, timeout] of Object.entries(presets)) { + expect(readinessDeadlineMs({ preset })).toBe(timeout + 3000); + } + }); }); describe('mergeSnapshotOptions(options)', () => {