fix: handle worker process start failure (#27249)

Worker process start failure is reported as a test error and skips other
tests from the group.
If happened during stop (e.g. from a Ctrl+C) before worker has fully
initialized, this error is ignored.

Drive-by: send SIGINT in tests to the whole tree, to better emulate
Ctrl+C behavior.

packages/playwright/src/runner/dispatcher.ts

@@ -117,21 +117,27 @@ export class Dispatcher {
         return;
     }
 
+    const stopCallback = () => this.stop().catch(() => {});
+    const jobDispatcher = new JobDispatcher(job, this._reporter, this._failureTracker, stopCallback);
+    this._workerSlots[index].jobDispatcher = jobDispatcher;
+
     // 2. Start the worker if it is down.
+    let startError;
     if (!worker) {
       worker = this._createWorker(job, index, serializeConfig(this._config));
       this._workerSlots[index].worker = worker;
       worker.on('exit', () => this._workerSlots[index].worker = undefined);
-      await worker.start();
+      startError = await worker.start();
       if (this._isStopped) // Check stopped signal after async hop.
         return;
     }
 
     // 3. Run the job.
-    const stopCallback = () => this.stop().catch(() => {});
-    const jobDispatcher = new JobDispatcher(job, this._reporter, this._failureTracker, stopCallback);
-    this._workerSlots[index].jobDispatcher = jobDispatcher;
-    const result = await jobDispatcher.runInWorker(worker);
+    if (startError)
+      jobDispatcher.onExit(startError);
+    else
+      jobDispatcher.runInWorker(worker);
+    const result = await jobDispatcher.jobResult;
     this._workerSlots[index].jobDispatcher = undefined;
     this._updateCounterForWorkerHash(job.workerHash, -1);
 
@@ -265,7 +271,8 @@ export class Dispatcher {
 }
 
 class JobDispatcher {
-  private _jobResult = new ManualPromise<{ newJob?: TestGroup, didFail: boolean }>();
+  jobResult = new ManualPromise<{ newJob?: TestGroup, didFail: boolean }>();
+
   private _listeners: RegisteredListener[] = [];
   private _failedTests = new Set<TestCase>();
   private _remainingByTestId = new Map<string, TestCase>();
@@ -498,19 +505,19 @@ class JobDispatcher {
     this._finished({ didFail: true, newJob });
   }
 
-  private _onExit(data: ProcessExitData) {
+  onExit(data: ProcessExitData) {
     const unexpectedExitError: TestError | undefined = data.unexpectedly ? {
-      message: `Internal error: worker process exited unexpectedly (code=${data.code}, signal=${data.signal})`
+      message: `Error: worker process exited unexpectedly (code=${data.code}, signal=${data.signal})`
     } : undefined;
     this._onDone({ skipTestsDueToSetupFailure: [], fatalErrors: [], unexpectedExitError });
   }
 
   private _finished(result: { newJob?: TestGroup, didFail: boolean }) {
     eventsHelper.removeEventListeners(this._listeners);
-    this._jobResult.resolve(result);
+    this.jobResult.resolve(result);
   }
 
-  async runInWorker(worker: WorkerHost): Promise<{ newJob?: TestGroup, didFail: boolean }> {
+  runInWorker(worker: WorkerHost) {
     this._parallelIndex = worker.parallelIndex;
     this._workerIndex = worker.workerIndex;
 
@@ -529,9 +536,8 @@ class JobDispatcher {
       eventsHelper.addEventListener(worker, 'stepEnd', this._onStepEnd.bind(this)),
       eventsHelper.addEventListener(worker, 'attach', this._onAttach.bind(this)),
       eventsHelper.addEventListener(worker, 'done', this._onDone.bind(this)),
-      eventsHelper.addEventListener(worker, 'exit', this._onExit.bind(this)),
+      eventsHelper.addEventListener(worker, 'exit', this.onExit.bind(this)),
     ];
-    return this._jobResult;
   }
 
   currentlyRunning() {

packages/playwright/src/runner/loadUtils.ts

@@ -90,13 +90,14 @@ export async function loadFileSuites(testRun: TestRun, mode: 'out-of-process' |
   // Load test files.
   const fileSuiteByFile = new Map<string, Suite>();
   const loaderHost = mode === 'out-of-process' ? new OutOfProcessLoaderHost(config) : new InProcessLoaderHost(config);
-  await loaderHost.start();
-  for (const file of allTestFiles) {
-    const fileSuite = await loaderHost.loadTestFile(file, errors);
-    fileSuiteByFile.set(file, fileSuite);
-    errors.push(...createDuplicateTitlesErrors(config, fileSuite));
+  if (await loaderHost.start(errors)) {
+    for (const file of allTestFiles) {
+      const fileSuite = await loaderHost.loadTestFile(file, errors);
+      fileSuiteByFile.set(file, fileSuite);
+      errors.push(...createDuplicateTitlesErrors(config, fileSuite));
+    }
+    await loaderHost.stop();
   }
-  await loaderHost.stop();
 
   // Check that no test file imports another test file.
   // Loader must be stopped first, since it popuplates the dependency tree.

packages/playwright/src/runner/loaderHost.ts

@@ -33,8 +33,9 @@ export class InProcessLoaderHost {
     this._poolBuilder = PoolBuilder.createForLoader();
   }
 
-  async start() {
+  async start(errors: TestError[]) {
     await initializeEsmLoader();
+    return true;
   }
 
   async loadTestFile(file: string, testErrors: TestError[]): Promise<Suite> {
@@ -57,8 +58,15 @@ export class OutOfProcessLoaderHost {
     this._processHost = new ProcessHost(require.resolve('../loader/loaderMain.js'), 'loader', {});
   }
 
-  async start() {
-    await this._processHost.startRunner(serializeConfig(this._config));
+  async start(errors: TestError[]) {
+    const startError = await this._processHost.startRunner(serializeConfig(this._config));
+    if (startError) {
+      errors.push({
+        message: `Test loader process failed to start with code "${startError.code}" and signal "${startError.signal}"`,
+      });
+      return false;
+    }
+    return true;
   }
 
   async loadTestFile(file: string, testErrors: TestError[]): Promise<Suite> {

packages/playwright/src/runner/processHost.ts

@@ -45,7 +45,7 @@ export class ProcessHost extends EventEmitter {
     this._extraEnv = env;
   }
 
-  async startRunner(runnerParams: any, options: { onStdOut?: (chunk: Buffer | string) => void, onStdErr?: (chunk: Buffer | string) => void } = {}) {
+  async startRunner(runnerParams: any, options: { onStdOut?: (chunk: Buffer | string) => void, onStdErr?: (chunk: Buffer | string) => void } = {}): Promise<ProcessExitData | undefined> {
     this.process = child_process.fork(require.resolve('../common/process'), {
       detached: false,
       env: { ...process.env, ...this._extraEnv },
@@ -93,11 +93,14 @@ export class ProcessHost extends EventEmitter {
     if (options.onStdErr)
       this.process.stderr?.on('data', options.onStdErr);
 
-    await new Promise<void>((resolve, reject) => {
-      this.process!.once('exit', (code, signal) => reject(new Error(`process exited with code "${code}" and signal "${signal}" before it became ready`)));
-      this.once('ready', () => resolve());
+    const error = await new Promise<ProcessExitData | undefined>(resolve => {
+      this.process!.once('exit', (code, signal) => resolve({ unexpectedly: true, code, signal }));
+      this.once('ready', () => resolve(undefined));
     });
 
+    if (error)
+      return error;
+
     const processParams: ProcessInitParams = {
       stdoutParams: {
         rows: process.stdout.rows,

packages/playwright/src/runner/workerHost.ts

@@ -55,7 +55,7 @@ export class WorkerHost extends ProcessHost {
 
   async start() {
     await fs.promises.mkdir(this._params.artifactsDir, { recursive: true });
-    await this.startRunner(this._params, {
+    return await this.startRunner(this._params, {
       onStdOut: chunk => this.emit('stdOut', stdioChunkToParams(chunk)),
       onStdErr: chunk => this.emit('stdErr', stdioChunkToParams(chunk)),
     });

tests/playwright-test/expect.spec.ts

@@ -649,7 +649,7 @@ test('should print expected/received on Ctrl+C', async ({ interactWithTestRunner
       `,
   }, { workers: 1 });
   await testProcess.waitForOutput('%%SEND-SIGINT%%');
-  process.kill(testProcess.process.pid!, 'SIGINT');
+  process.kill(-testProcess.process.pid!, 'SIGINT');
   const { exitCode } = await testProcess.exited;
   expect(exitCode).toBe(130);
 

tests/playwright-test/hooks.spec.ts

@@ -660,7 +660,7 @@ test('should not hang and report results when worker process suddenly exits duri
   expect(result.exitCode).toBe(1);
   expect(result.passed).toBe(0);
   expect(result.failed).toBe(1);
-  expect(result.output).toContain('Internal error: worker process exited unexpectedly');
+  expect(result.output).toContain('Error: worker process exited unexpectedly');
   expect(result.output).toContain('[1/1] a.spec.js:3:11 › failing due to afterall');
 });
 

tests/playwright-test/playwright.spec.ts

@@ -447,7 +447,7 @@ test('should report click error on sigint', async ({ interactWithTestRunner }) =
     `,
   }, { workers: 1 });
   await testProcess.waitForOutput('%%SEND-SIGINT%%');
-  process.kill(testProcess.process.pid!, 'SIGINT');
+  process.kill(-testProcess.process.pid!, 'SIGINT');
   const { exitCode } = await testProcess.exited;
   expect(exitCode).toBe(130);
 

tests/playwright-test/runner.spec.ts

@@ -92,7 +92,66 @@ test('should continue with other tests after worker process suddenly exits', asy
   expect(result.passed).toBe(4);
   expect(result.failed).toBe(1);
   expect(result.skipped).toBe(0);
-  expect(result.output).toContain('Internal error: worker process exited unexpectedly');
+  expect(result.output).toContain('Error: worker process exited unexpectedly');
+});
+
+test('should report subprocess creation error', async ({ runInlineTest }, testInfo) => {
+  const result = await runInlineTest({
+    'preload.js': `
+      process.exit(42);
+    `,
+    'a.spec.js': `
+      import { test, expect } from '@playwright/test';
+      test('fails', () => {});
+      test('skipped', () => {});
+      // Infect subprocesses to immediately exit when spawning a worker.
+      process.env.NODE_OPTIONS = '--require ${JSON.stringify(testInfo.outputPath('preload.js').replace(/\\/g, '\\\\'))}';
+    `
+  });
+  expect(result.exitCode).toBe(1);
+  expect(result.passed).toBe(0);
+  expect(result.failed).toBe(1);
+  expect(result.skipped).toBe(1);
+  expect(result.output).toContain('Error: worker process exited unexpectedly (code=42, signal=null)');
+});
+
+test('should ignore subprocess creation error because of SIGINT', async ({ interactWithTestRunner }, testInfo) => {
+  test.skip(process.platform === 'win32', 'No sending SIGINT on Windows');
+
+  const readyFile = testInfo.outputPath('ready.txt');
+  const testProcess = await interactWithTestRunner({
+    'hang.js': `
+      require('fs').writeFileSync(${JSON.stringify(readyFile)}, 'ready');
+      setInterval(() => {}, 1000);
+    `,
+    'preload.js': `
+      require('child_process').spawnSync(
+        process.argv[0],
+        [require('path').resolve('./hang.js')],
+        { env: { ...process.env, NODE_OPTIONS: '' } },
+      );
+    `,
+    'a.spec.js': `
+      import { test, expect } from '@playwright/test';
+      test('fails', () => {});
+      test('skipped', () => {});
+      // Infect subprocesses to immediately hang when spawning a worker.
+      process.env.NODE_OPTIONS = '--require ${JSON.stringify(testInfo.outputPath('preload.js'))}';
+    `
+  });
+
+  while (!fs.existsSync(readyFile))
+    await new Promise(f => setTimeout(f, 100));
+  process.kill(-testProcess.process.pid!, 'SIGINT');
+
+  const { exitCode } = await testProcess.exited;
+  expect(exitCode).toBe(130);
+
+  const result = parseTestRunnerOutput(testProcess.output);
+  expect(result.passed).toBe(0);
+  expect(result.failed).toBe(0);
+  expect(result.skipped).toBe(2);
+  expect(result.output).not.toContain('worker process exited unexpectedly');
 });
 
 test('sigint should stop workers', async ({ interactWithTestRunner }) => {
@@ -124,7 +183,7 @@ test('sigint should stop workers', async ({ interactWithTestRunner }) => {
     PLAYWRIGHT_JSON_OUTPUT_NAME: 'report.json',
   });
   await testProcess.waitForOutput('%%SEND-SIGINT%%', 2);
-  process.kill(testProcess.process.pid!, 'SIGINT');
+  process.kill(-testProcess.process.pid!, 'SIGINT');
   const { exitCode } = await testProcess.exited;
   expect(exitCode).toBe(130);
 
@@ -200,7 +259,7 @@ test('worker interrupt should report errors', async ({ interactWithTestRunner })
     `,
   });
   await testProcess.waitForOutput('%%SEND-SIGINT%%');
-  process.kill(testProcess.process.pid!, 'SIGINT');
+  process.kill(-testProcess.process.pid!, 'SIGINT');
   const { exitCode } = await testProcess.exited;
   expect(exitCode).toBe(130);
 
@@ -378,7 +437,7 @@ test('sigint should stop global setup', async ({ interactWithTestRunner }) => {
     `,
   }, { 'workers': 1 });
   await testProcess.waitForOutput('%%SEND-SIGINT%%');
-  process.kill(testProcess.process.pid!, 'SIGINT');
+  process.kill(-testProcess.process.pid!, 'SIGINT');
   const { exitCode } = await testProcess.exited;
   expect(exitCode).toBe(130);
 
@@ -425,7 +484,7 @@ test('sigint should stop plugins', async ({ interactWithTestRunner }) => {
     `,
   }, { 'workers': 1 });
   await testProcess.waitForOutput('%%SEND-SIGINT%%');
-  process.kill(testProcess.process.pid!, 'SIGINT');
+  process.kill(-testProcess.process.pid!, 'SIGINT');
   const { exitCode } = await testProcess.exited;
   expect(exitCode).toBe(130);
 
@@ -473,7 +532,7 @@ test('sigint should stop plugins 2', async ({ interactWithTestRunner }) => {
     `,
   }, { 'workers': 1 });
   await testProcess.waitForOutput('%%SEND-SIGINT%%');
-  process.kill(testProcess.process.pid!, 'SIGINT');
+  process.kill(-testProcess.process.pid!, 'SIGINT');
   const { exitCode } = await testProcess.exited;
   expect(exitCode).toBe(130);
 
@@ -578,7 +637,7 @@ test('should not hang on worker error in test file', async ({ runInlineTest }) =
   }, { 'timeout': 3000 });
   expect(result.exitCode).toBe(1);
   expect(result.results[0].status).toBe('failed');
-  expect(result.results[0].error.message).toContain('Internal error: worker process exited unexpectedly');
+  expect(result.results[0].error.message).toContain('Error: worker process exited unexpectedly');
   expect(result.results[1].status).toBe('skipped');
 });
 
@@ -606,8 +665,8 @@ test('fast double SIGINT should be ignored', async ({ interactWithTestRunner })
   });
   await testProcess.waitForOutput('%%SEND-SIGINT%%');
   // Send SIGINT twice in quick succession.
-  process.kill(testProcess.process.pid!, 'SIGINT');
-  process.kill(testProcess.process.pid!, 'SIGINT');
+  process.kill(-testProcess.process.pid!, 'SIGINT');
+  process.kill(-testProcess.process.pid!, 'SIGINT');
   const { exitCode } = await testProcess.exited;
   expect(exitCode).toBe(130);
 
@@ -639,9 +698,9 @@ test('slow double SIGINT should be respected', async ({ interactWithTestRunner }
     `,
   });
   await testProcess.waitForOutput('%%SEND-SIGINT%%');
-  process.kill(testProcess.process.pid!, 'SIGINT');
+  process.kill(-testProcess.process.pid!, 'SIGINT');
   await new Promise(f => setTimeout(f, 2000));
-  process.kill(testProcess.process.pid!, 'SIGINT');
+  process.kill(-testProcess.process.pid!, 'SIGINT');
   const { exitCode } = await testProcess.exited;
   expect(exitCode).toBe(130);
 
@@ -680,10 +739,10 @@ test('slow double SIGINT should be respected in reporter.onExit', async ({ inter
     `,
   }, { reporter: '' });
   await testProcess.waitForOutput('%%SEND-SIGINT%%');
-  process.kill(testProcess.process.pid!, 'SIGINT');
+  process.kill(-testProcess.process.pid!, 'SIGINT');
   await new Promise(f => setTimeout(f, 2000));
   await testProcess.waitForOutput('MyReporter.onExit started');
-  process.kill(testProcess.process.pid!, 'SIGINT');
+  process.kill(-testProcess.process.pid!, 'SIGINT');
   const { exitCode, signal } = await testProcess.exited;
   expect(exitCode).toBe(null);
   expect(signal).toBe('SIGINT');  // Default handler should report the signal.

tests/playwright-test/web-server.spec.ts

@@ -735,7 +735,7 @@ test('should forward stdout when set to "pipe" before server is ready', async ({
     `,
   }, { workers: 1 });
   await testProcess.waitForOutput('%%SEND-SIGINT%%');
-  process.kill(testProcess.process.pid!, 'SIGINT');
+  process.kill(-testProcess.process.pid!, 'SIGINT');
   await testProcess.exited;
 
   const result = parseTestRunnerOutput(testProcess.output);