chore: ignore SIGINT inside driver process (#23028)

https://github.com/microsoft/playwright-python/issues/1843

Almost reverts https://github.com/microsoft/playwright/pull/11826 since
this is not used by VSCode anymore and was legacy.

packages/playwright-core/src/cli/driver.ts

@@ -21,7 +21,7 @@ import * as playwright from '../..';
 import type { BrowserType } from '../client/browserType';
 import type { LaunchServerOptions } from '../client/types';
 import { createPlaywright, DispatcherConnection, RootDispatcher, PlaywrightDispatcher } from '../server';
-import { IpcTransport, PipeTransport } from '../protocol/transport';
+import { PipeTransport } from '../protocol/transport';
 import { PlaywrightServer } from '../remote/playwrightServer';
 import { gracefullyCloseAll } from '../utils/processLauncher';
 
@@ -36,14 +36,19 @@ export function runDriver() {
     const playwright = createPlaywright(sdkLanguage);
     return new PlaywrightDispatcher(rootScope, playwright);
   });
-  const transport = process.send ? new IpcTransport(process) : new PipeTransport(process.stdout, process.stdin);
+  const transport = new PipeTransport(process.stdout, process.stdin);
-  transport.onmessage = message => dispatcherConnection.dispatch(JSON.parse(message));
+  transport.onmessage = (message: string) => dispatcherConnection.dispatch(JSON.parse(message));
   dispatcherConnection.onmessage = message => transport.send(JSON.stringify(message));
   transport.onclose = () => {
     // Drop any messages during shutdown on the floor.
     dispatcherConnection.onmessage = () => {};
     selfDestruct();
   };
+  // Ignore the SIGINT signal in the driver process so the parent can gracefully close the connection.
+  // We still will destruct everything (close browsers and exit) when the transport pipe closes.
+  process.on('SIGINT', () => {
+    // Keep the process running.
+  });
 }
 
 export type RunServerOptions = {

packages/playwright-core/src/outofprocess.ts

@@ -15,7 +15,7 @@
  */
 
 import { Connection } from './client/connection';
-import { IpcTransport } from './protocol/transport';
+import { PipeTransport } from './protocol/transport';
 import type { Playwright } from './client/playwright';
 import * as childProcess from 'child_process';
 import * as path from 'path';
@@ -32,12 +32,10 @@ class PlaywrightClient {
   _playwright: Promise<Playwright>;
   _driverProcess: childProcess.ChildProcess;
   private _closePromise = new ManualPromise<void>();
-  private _transport: IpcTransport;
-  private _stopped = false;
 
   constructor(env: any) {
     this._driverProcess = childProcess.fork(path.join(__dirname, 'cli', 'cli.js'), ['run-driver'], {
-      stdio: ['inherit', 'inherit', 'inherit', 'ipc'],
+      stdio: 'pipe',
       detached: true,
       env: {
         ...process.env,
@@ -49,25 +47,23 @@ class PlaywrightClient {
 
     const connection = new Connection();
     connection.markAsRemote();
-    this._transport = new IpcTransport(this._driverProcess);
+    const transport = new PipeTransport(this._driverProcess.stdin!, this._driverProcess.stdout!);
-    connection.onmessage = message => this._transport.send(JSON.stringify(message));
+    connection.onmessage = message => transport.send(JSON.stringify(message));
-    this._transport.onmessage = message => connection.dispatch(JSON.parse(message));
+    transport.onmessage = message => connection.dispatch(JSON.parse(message));
-    this._transport.onclose = () => this._closePromise.resolve();
+    transport.onclose = () => this._closePromise.resolve();
 
     this._playwright = connection.initializePlaywright();
   }
 
   async stop() {
-    this._stopped = true;
+    this._driverProcess.removeListener('exit', this._onExit);
-    this._transport.close();
+    this._driverProcess.stdin!.destroy();
+    this._driverProcess.stdout!.destroy();
+    this._driverProcess.stderr!.destroy();
     await this._closePromise;
   }
 
   private _onExit(exitCode: number | null, signal: string | null) {
-    if (this._stopped)
+    throw new Error(`Server closed with exitCode=${exitCode} signal=${signal}`);
-      this._closePromise.resolve();
-    else
-      throw new Error(`Server closed with exitCode=${exitCode} signal=${signal}`);
   }
-
 }

packages/playwright-core/src/protocol/transport.ts

@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 
-import type { ChildProcess } from 'child_process';
 import { makeWaitForNextTask } from '../utils';
 
 export interface WritableStream {
@@ -103,28 +102,3 @@ export class PipeTransport {
     }
   }
 }
-
-export class IpcTransport {
-  private _process: NodeJS.Process | ChildProcess;
-  private _waitForNextTask = makeWaitForNextTask();
-  onmessage?: (message: string) => void;
-  onclose?: () => void;
-
-  constructor(process: NodeJS.Process | ChildProcess) {
-    this._process = process;
-    this._process.on('message', message => this._waitForNextTask(() => {
-      if (message === '<eof>')
-        this.onclose?.();
-      else
-        this.onmessage?.(message);
-    }));
-  }
-
-  send(message: string) {
-    this._process.send!(message);
-  }
-
-  close() {
-    this._process.send!('<eof>');
-  }
-}

tests/installation/driver-should-work.spec.ts

@@ -13,10 +13,29 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { test } from './npmTest';
+import { test, expect } from './npmTest';
 
 test('driver should work', async ({ exec }) => {
   await exec('npm i --foreground-scripts playwright', { env: { PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: '1' } });
   await exec('npx playwright install');
   await exec('node driver-client.js');
 });
+
+test('driver should ignore SIGINT', async ({ exec }) => {
+  test.skip(process.platform === 'win32', 'Only relevant for POSIX');
+  test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright-python/issues/1843' });
+  await exec('npm i --foreground-scripts playwright', { env: { PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD: '1' } });
+  await exec('npx playwright install chromium');
+  const output = await exec('node driver-client-sigint.js');
+  const lines = output.split('\n').filter(l => l.trim());
+  expect(lines).toEqual([
+    'launching driver',
+    'launched driver',
+    'closing gracefully',
+    'closed page',
+    'closed context',
+    'closed browser',
+    'stopped driver',
+    'SUCCESS',
+  ]);
+});

tests/installation/fixture-scripts/driver-client-sigint.js

@@ -0,0 +1,54 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+// @ts-check
+
+const pw = require.resolve('playwright');
+const oop = require.resolve('playwright-core/lib/outofprocess', { paths: [pw] });
+const { start } = require(oop);
+
+(async () => {
+  console.log('launching driver')
+  const { playwright, stop } = await start();
+  console.log('launched driver')
+  try {
+    const browser = await playwright.chromium.launch({ handleSIGINT: false });
+    const context = await browser.newContext();
+    const page = await context.newPage();
+    // let things settle down
+    await page.waitForTimeout(100);
+    // send SIGINT to driver
+    process.kill(playwright.driverProcess.pid, 'SIGINT');
+    // wait and see if driver exits
+    await page.waitForTimeout(100);
+    console.log(`closing gracefully`)
+    await page.close();
+    console.log('closed page');
+    await context.close();
+    console.log('closed context');
+    await browser.close();
+    console.log('closed browser');
+    await stop();
+    console.log('stopped driver');
+  } catch (e) {
+    console.error(`Should be able to launch from ${process.cwd()}`);
+    console.error(e);
+    process.exit(1);
+  }
+  console.log(`SUCCESS`);
+})().catch(err => {
+  console.error(err);
+  process.exit(1);
+});