From fb6efcd24b99fb450727f0055d37299d430b5571 Mon Sep 17 00:00:00 2001 From: Aryan Kumar Date: Tue, 11 Aug 2026 01:17:16 +0530 Subject: [PATCH] fix(core): customElements wait read its timeout from the wrong argument, hanging the CLI forever (PER-10405) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `percy snapshot` hung indefinitely — no error, no timeout, no output — on any page holding a custom-element tag that is never registered. Reported against every live Next.js `next start` server: the app router mounts ``, which Next never passes to `customElements.define()`. Regression in 1.32.0 (1.31.13 predates the wait); 1.32.0/1.32.3/1.32.6 all hang. Root cause ---------- `serializeFunction()` wraps a string body as `(async function eval(){ })(percyHelpers, ...args)`, so `arguments[0]` is the injected Percy helpers object and the caller's first argument lands at `arguments[1]`. `WAIT_FOR_CUSTOM_ELEMENTS_BODY` read `arguments[0]`, so: Date.now() + (arguments[0] || 500) => 1786389350594 + {config,snapshot,...} => "1786389350594[object Object]" // a String `Date.now() >= deadline` then compared a number against that string, coerced to NaN, and was permanently false. With `:not(:defined)` never clearing either, the poll had no exit: it re-ticked every 100ms forever and the returned promise never settled. The body is awaited over CDP with `awaitPromise: true`, which has no protocol-level timeout, so `page.snapshot()` blocked forever. Nothing downstream could report it — the run never even reached build creation, so no build appears in Percy at all. Fix --- - Read the timeout from `arguments[1]`, validated as a finite positive number. - Add a `settled` guard plus an absolute `setTimeout(finish, timeoutMs)` so the promise cannot fail to settle even if the tick chain breaks. This is awaited over CDP with no timeout of its own, so not settling must be structurally impossible rather than merely unlikely. - Route the tick chain's rejection path to `finish` and guard `whenDefined()`, which throws SyntaxError on an invalid custom element name. Verified against a bare `create-next-app` (next 16.3.0, node 22) served by `next start`: 1.32.6 hangs past 120s and creates no build; patched, the same run finalizes in 23s. Co-Authored-By: Claude Opus 5 (1M context) --- packages/core/src/page.js | 28 +++++++++++++++++++++++----- packages/core/test/percy.test.js | 31 ++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/packages/core/src/page.js b/packages/core/src/page.js index 8920b3a4a..1a5db7aac 100644 --- a/packages/core/src/page.js +++ b/packages/core/src/page.js @@ -25,22 +25,40 @@ export const DEFAULT_WAIT_FOR_CUSTOM_ELEMENTS_TIMEOUT = 500; // IMPORTANT: this body is intentionally ES5 — it is evaluated in the // page's realm and must work in any browser the page targets. Don't // "modernize" with arrow functions, let/const, or optional chaining. +// +// serializeFunction() injects the Percy helpers object as `arguments[0]`, so +// the caller's first argument is `arguments[1]`. export const WAIT_FOR_CUSTOM_ELEMENTS_BODY = ` - var deadline = Date.now() + (arguments[0] || 500); + var timeoutMs = typeof arguments[1] === 'number' && isFinite(arguments[1]) && arguments[1] > 0 + ? arguments[1] + : 500; + var deadline = Date.now() + timeoutMs; return new Promise(function(resolve) { + var settled = false; + function finish() { + if (settled) return; + settled = true; + resolve(); + } + setTimeout(finish, timeoutMs); function tick() { + if (settled) return; var undef = document.querySelectorAll(":not(:defined)"); - if (!undef.length) return resolve(); - if (Date.now() >= deadline) return resolve(); + if (!undef.length) return finish(); + if (Date.now() >= deadline) return finish(); var names = {}; for (var i = 0; i < undef.length; i++) names[undef[i].localName] = true; var promises = Object.keys(names).map(function(n) { - return window.customElements.whenDefined(n).catch(function(){}); + try { + return window.customElements.whenDefined(n).catch(function(){}); + } catch (e) { + return Promise.resolve(); + } }); Promise.race([ Promise.all(promises), new Promise(function(r) { setTimeout(r, 100); }) - ]).then(tick); + ]).then(tick, finish); } tick(); }); diff --git a/packages/core/test/percy.test.js b/packages/core/test/percy.test.js index 0b93c1be3..992d1b0ab 100644 --- a/packages/core/test/percy.test.js +++ b/packages/core/test/percy.test.js @@ -4,7 +4,11 @@ import Percy from '@percy/core'; import Pako from 'pako'; import DetectProxy from '@percy/client/detect-proxy'; import { validateSnapshotOptions } from '../src/snapshot.js'; -import { Page, WAIT_FOR_CUSTOM_ELEMENTS_BODY } from '../src/page.js'; +import { + Page, + WAIT_FOR_CUSTOM_ELEMENTS_BODY, + DEFAULT_WAIT_FOR_CUSTOM_ELEMENTS_TIMEOUT +} from '../src/page.js'; describe('Percy', () => { let percy, server; @@ -216,6 +220,31 @@ describe('Percy', () => { ])); }); + it('does not hang on a page holding a never-defined custom element', async () => { + // PER-10405: an unregistered tag matches `:not(:defined)` forever and its + // `customElements.whenDefined()` never resolves, so the wait must fall back + // on its own ceiling. + server.reply('/', () => [200, 'text/html', ( + '

hi

' + )]); + await percy.browser.launch(); + let page = await percy.browser.page(); + await page.goto('http://localhost:8000'); + + let undefinedCount = await page.eval( + () => document.querySelectorAll(':not(:defined)').length); + expect(undefinedCount).toBe(1); + + let start = Date.now(); + await expectAsync(page.eval( + WAIT_FOR_CUSTOM_ELEMENTS_BODY, + DEFAULT_WAIT_FOR_CUSTOM_ELEMENTS_TIMEOUT + )).toBeResolved(); + let elapsed = Date.now() - start; + + expect(elapsed).toBeLessThan(5000); + }); + it('runs readiness check before serializing when readiness option is set', async () => { server.reply('/', () => [200, 'text/html', '

Hello Percy!

']);