fix(sdk-utils): bound the readiness eval on the Node side

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
 };
 

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);
   }
 }
 

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)', () => {