diff --git a/package-lock.json b/package-lock.json index 9c13db4841ad8..6796828c545a7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5277,9 +5277,9 @@ "dev": true }, "node_modules/fast-uri": { - "version": "3.1.0", - "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.0.tgz", - "integrity": "sha512-iPeeDKJSWf4IEOasVVrknXpaBV0IApz/gp7S2bb7Z4Lljbl2MGJRqInZiUrQwV16cpzw/D3S5j5Julj/gT52AA==", + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.2.tgz", + "integrity": "sha512-rVjf7ArG3LTk+FS6Yw81V1DLuZl1bRbNrev6Tmd/9RaroeeRRJhAt7jg/6YFxbvAQXUCavSoZhPPj6oOx+5KjQ==", "dev": true, "funding": [ { diff --git a/packages/isomorphic/stringUtils.ts b/packages/isomorphic/stringUtils.ts index 6e8a9115071ba..65dc22051fbf1 100644 --- a/packages/isomorphic/stringUtils.ts +++ b/packages/isomorphic/stringUtils.ts @@ -189,7 +189,10 @@ export function parseRegex(regex: string): RegExp { return new RegExp(source, flags); } -export const ansiRegex = new RegExp('([\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)|(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-ntqry=><~])))', 'g'); +// Semicolons removed from [[\]()#;?] to avoid polynomial backtracking +// when both that group and (?:;...)* can match runs of semicolons. +// \d{1,4} relaxed to \d{0,4} so empty params (e.g. ESC[;H) still match. +export const ansiRegex = new RegExp('([\\u001B\\u009B][[\\]()#?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)|(?:(?:\\d{0,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-ntqry=><~])))', 'g'); export function stripAnsiEscapes(str: string): string { return str.replace(ansiRegex, ''); } diff --git a/packages/playwright-core/src/tools/cli-client/program.ts b/packages/playwright-core/src/tools/cli-client/program.ts index 1c8d21574d34c..c01d0aaf70337 100644 --- a/packages/playwright-core/src/tools/cli-client/program.ts +++ b/packages/playwright-core/src/tools/cli-client/program.ts @@ -33,7 +33,6 @@ import { minimist } from './minimist'; import type { ListData, ListedBrowser, Output } from './output'; import type { ClientInfo, SessionFile } from './registry'; import type { MinimistArgs } from './minimist'; -import type { Readable } from 'stream'; type GlobalOptions = { help?: boolean; @@ -230,35 +229,46 @@ export async function program(options?: { embedderVersion?: string}) { const foreground = args.port !== undefined; const child = spawn(process.execPath, daemonArgs, { detached: !foreground, - stdio: foreground ? 'inherit' : ['ignore', 'ignore', 'ignore', 'pipe'], + stdio: foreground ? 'inherit' : ['pipe', 'pipe', 'ignore'], }); if (foreground) { await new Promise(resolve => child.on('exit', () => resolve())); return; } - const readyStream = (child.stdio as unknown as Readable[])[3]; try { await new Promise((resolve, reject) => { + let outLog = ''; const settle = (err?: Error) => { clearTimeout(timer); - readyStream.destroy(); + child.stdout!.removeAllListeners(); + child.removeAllListeners('exit'); if (err) reject(err); else resolve(); }; - const timer = setTimeout(() => settle(new Error('Dashboard daemon did not spin up within 60s, killing it')), 60_000); - readyStream.once('data', () => settle()); - readyStream.once('error', err => settle(err)); - child.once('exit', (code, signal) => settle(new Error(`Dashboard daemon exited (code=${code}, signal=${signal}) before signaling READY`))); + const timer = setTimeout(() => settle(new Error('Dashboard daemon did not spin up within 60s')), 60_000); + child.stdout!.on('data', data => { + outLog += data.toString(); + if (!outLog.includes('')) + return; + if (outLog.match(/### Success\n[\s\S]*/)) + settle(); + else + settle(new Error(outLog.trim())); + }); + child.stdout!.once('error', err => settle(err)); + child.once('exit', (code, signal) => settle(new Error(`Dashboard daemon exited (code=${code}, signal=${signal}) before signaling READY${outLog ? '\n' + outLog : ''}`))); }); } catch (err) { - if (child.exitCode === null && child.signalCode === null) { - child.kill('SIGKILL'); + child.stdin!.destroy(); + child.stdout!.destroy(); + if (child.exitCode === null && child.signalCode === null) await new Promise(resolve => child.once('exit', () => resolve())); - } throw err; } + child.stdin!.destroy(); + child.stdout!.destroy(); child.unref(); output.show(sessionName, child.pid); return; diff --git a/packages/playwright-core/src/tools/cli-client/session.ts b/packages/playwright-core/src/tools/cli-client/session.ts index ce533cbe420cc..86329934b2404 100644 --- a/packages/playwright-core/src/tools/cli-client/session.ts +++ b/packages/playwright-core/src/tools/cli-client/session.ts @@ -168,21 +168,17 @@ export class Session { outLog += data.toString(); if (!outLog.includes('')) return; - const errorMatch = outLog.match(/### Error\n([\s\S]*)/); - const error = errorMatch ? errorMatch[1].trim() : undefined; - if (error) { - const errLogContent = fs.readFileSync(errLog, 'utf-8'); - rejectWithPid(reject, error + (errLogContent ? '\n' + errLogContent : '')); - } - - const successMatch = outLog.match(/### Success\nDaemon listening on (.*)\n/); - if (successMatch) + if (outLog.match(/### Success\nDaemon listening on (.*)\n/)) { resolve(); + return; + } + const errLogContent = fs.readFileSync(errLog, 'utf-8'); + rejectWithPid(reject, outLog.trim() + (errLogContent ? '\n' + errLogContent : '')); }); child.on('close', code => { if (!signalled) { const errLogContent = fs.readFileSync(errLog, 'utf-8'); - rejectWithPid(reject, `Daemon process exited with code ${code}` + (errLogContent ? '\n' + errLogContent : '')); + rejectWithPid(reject, `Daemon process exited with code ${code}` + (outLog ? '\n' + outLog : '') + (errLogContent ? '\n' + errLogContent : '')); } }); }); diff --git a/packages/playwright-core/src/tools/dashboard/dashboardApp.ts b/packages/playwright-core/src/tools/dashboard/dashboardApp.ts index b0cec86fee003..ffa1a183e92b6 100644 --- a/packages/playwright-core/src/tools/dashboard/dashboardApp.ts +++ b/packages/playwright-core/src/tools/dashboard/dashboardApp.ts @@ -311,15 +311,44 @@ export async function openDashboardApp() { selfDestructOnParentGone(); return; } - let server: net.Server | undefined; - process.on('exit', () => server?.close()); + // Self-destruct if the parent CLI dies before we signal READY. Unregistered + // before we signal so the daemon outlives the parent. + const stopSelfDestruct = selfDestructOnParentGone(); + let server: net.Server; try { server = await acquireSingleton(options); } catch { + // Another daemon is already running; acquireSingleton forwarded our + // options to it. Signal success so the parent doesn't treat our clean + // exit as a startup failure. + stopSelfDestruct(); + // eslint-disable-next-line no-console + console.log('### Success\nDashboard already running'); + // eslint-disable-next-line no-console + console.log(''); return; } + process.on('exit', () => server.close()); + try { + await startApp(server, options); + stopSelfDestruct(); + // eslint-disable-next-line no-console + console.log('### Success\nDashboard ready'); + // eslint-disable-next-line no-console + console.log(''); + } catch (error) { + const message = (error as Error).stack || (error as Error).message; + // eslint-disable-next-line no-console + console.log(`### Error\n${message}`); + // eslint-disable-next-line no-console + console.log(''); + gracefullyProcessExitDoNotHang(1); + } +} + +async function startApp(server: net.Server, options: DashboardOptions) { const statePromise = innerOpenDashboardApp(options); - server?.on('connection', socket => { + server.on('connection', socket => { let buffer = ''; socket.on('data', data => { buffer += data.toString(); @@ -356,7 +385,6 @@ export async function openDashboardApp() { }); }); await statePromise; - try { fs.writeSync(3, '.'); fs.closeSync(3); } catch {} } export async function openDashboardForContext(context: api.BrowserContext): Promise { @@ -426,8 +454,8 @@ async function runAnnotateClient(options: DashboardOptions): Promise { console.log(text); } -function selfDestructOnParentGone() { - process.stdin.on('close', () => { - gracefullyProcessExitDoNotHang(0); - }); +function selfDestructOnParentGone(): () => void { + const onClose = () => gracefullyProcessExitDoNotHang(0); + process.stdin.on('close', onClose); + return () => process.stdin.off('close', onClose); } diff --git a/packages/playwright-core/src/tools/dashboard/dashboardController.ts b/packages/playwright-core/src/tools/dashboard/dashboardController.ts index 670eed56f6066..b11df9415dea9 100644 --- a/packages/playwright-core/src/tools/dashboard/dashboardController.ts +++ b/packages/playwright-core/src/tools/dashboard/dashboardController.ts @@ -61,7 +61,10 @@ export class DashboardConnection implements Transport { this._pushSessions(); void this._tryRevealPending(); }); - this._provider.on(SessionProviderEvent.TabsChanged, () => this._pushTabs()); + this._provider.on(SessionProviderEvent.TabsChanged, () => { + this._pushTabs(); + void this._tryRevealPending(); + }); this._provider.on(SessionProviderEvent.ContextClosed, context => { if (this._attachedPage?.page.context() === context) { this._attachedPage.dispose(); diff --git a/tests/config/utils.ts b/tests/config/utils.ts index 95d42a17a3285..f39363a61f344 100644 --- a/tests/config/utils.ts +++ b/tests/config/utils.ts @@ -231,7 +231,7 @@ export function unshift(snapshot: string): string { return lines.filter(t => t.trim()).map(line => line.substring(whitespacePrefixLength)).join('\n'); } -const ansiRegex = new RegExp('[\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)|(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-ntqry=><~]))', 'g'); +const ansiRegex = new RegExp('[\\u001B\\u009B][[\\]()#?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)|(?:(?:\\d{0,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-ntqry=><~]))', 'g'); export function stripAnsi(str: string): string { return str.replace(ansiRegex, ''); } diff --git a/tests/library/snapshot-renderer.spec.ts b/tests/library/snapshot-renderer.spec.ts index 0dd0d7900c621..f512a6933b57c 100644 --- a/tests/library/snapshot-renderer.spec.ts +++ b/tests/library/snapshot-renderer.spec.ts @@ -17,6 +17,7 @@ import { test, expect } from '@playwright/test'; import { SnapshotRenderer } from '../../packages/isomorphic/trace/snapshotRenderer'; import { LRUCache } from '../../packages/isomorphic/lruCache'; +import { stripAnsiEscapes } from '../../packages/isomorphic/stringUtils'; import type { FrameSnapshot } from '../../packages/trace/src/snapshot'; function makeSnapshot(overrides: Partial = {}): FrameSnapshot { @@ -111,3 +112,21 @@ test('snapshot renderer handles case-insensitive iframe tag names', () => { expect(html).toContain('__playwright_srcdoc__'); expect(html).toContain('__playwright_src__'); }); + +test('stripAnsiEscapes should not exhibit polynomial backtracking', () => { + // \x1b[ + 50000 semicolons + non-terminal character. + // Before the fix this took >3 seconds due to O(n^2) backtracking. + const payload = '\x1b[' + ';'.repeat(50000) + '!'; + const result = stripAnsiEscapes(payload); + // The ESC[ prefix is not a complete ANSI sequence, so it stays in the output. + expect(result).toContain('!'); +}); + +test('stripAnsiEscapes handles common ANSI sequences after fix', () => { + expect(stripAnsiEscapes('\x1b[31mred\x1b[0m')).toBe('red'); + expect(stripAnsiEscapes('\x1b[0;31mbold red\x1b[0m')).toBe('bold red'); + expect(stripAnsiEscapes('\x1b[;H')).toBe(''); + expect(stripAnsiEscapes('\x1b[2J')).toBe(''); + expect(stripAnsiEscapes('\x1b[38;2;255;0;0mcolored\x1b[0m')).toBe('colored'); + expect(stripAnsiEscapes('hello\x1b[32mworld\x1b[0m!')).toBe('helloworld!'); +}); diff --git a/utils/build/build.js b/utils/build/build.js index d565e9dd3e22f..3ef3339cd13d1 100644 --- a/utils/build/build.js +++ b/utils/build/build.js @@ -870,17 +870,14 @@ steps.push(new ProgramStep({ concurrent: true, })); -// Build/watch web packages. -// HMR: in watch mode the dashboard, html-reporter, and trace viewer (incl. UI -// mode) are served by embedded Vite dev servers, so skip their -// `vite build --watch` steps. Set PW_HMR_STATIC=1 to keep the watch builds for -// testing the bundled output. Recorder is not yet HMR'd. The trace viewer -// service worker still builds via vite.sw.config.ts above — that step is not -// in this loop. -const hmrReplacesWebBuilds = watchMode && process.env.PW_HMR_STATIC !== '1'; -const hmrHandledPackages = new Set(['dashboard', 'html-reporter', 'trace-viewer']); -const webPackages = ['html-reporter', 'recorder', 'trace-viewer', 'dashboard'] - .filter(pkg => !(hmrReplacesWebBuilds && hmrHandledPackages.has(pkg))); +// Build/watch web packages. The html-reporter, trace-viewer, and dashboard +// also have embedded Vite dev servers used when viewing reports/traces/the +// dashboard live, but their bundled output is consumed as a static artifact +// in other code paths (e.g. HtmlBuilder.build() reads lib/vite/htmlReport/ +// and lib/vite/traceViewer/), so we always keep the static build alongside +// HMR. Recorder is not yet HMR'd. The trace viewer service worker still +// builds via vite.sw.config.ts above — that step is not in this loop. +const webPackages = ['html-reporter', 'recorder', 'trace-viewer', 'dashboard']; for (const webPackage of webPackages) { steps.push(new ProgramStep({ command: 'npx',