From d29b5c69a9b02bf5098d589ac8adbcdda6d6ace0 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Wed, 7 Sep 2022 16:24:00 -0400 Subject: [PATCH] chore(webkit): suppress ECONNRESET... --- packages/server/index.js | 2 +- packages/server/lib/browsers/webkit.ts | 48 +++++++++++++++------ packages/server/lib/unhandled_exceptions.js | 9 ---- packages/server/lib/unhandled_exceptions.ts | 26 +++++++++++ 4 files changed, 63 insertions(+), 22 deletions(-) delete mode 100644 packages/server/lib/unhandled_exceptions.js create mode 100644 packages/server/lib/unhandled_exceptions.ts diff --git a/packages/server/index.js b/packages/server/index.js index f6863a2d2074..4fe98b661354 100644 --- a/packages/server/index.js +++ b/packages/server/index.js @@ -23,7 +23,7 @@ if (process.env.CY_NET_PROFILE && isRunningElectron) { process.stdout.write(`Network profiler writing to ${netProfiler.logPath}\n`) } -require('./lib/unhandled_exceptions') +require('./lib/unhandled_exceptions').handle() process.env.UV_THREADPOOL_SIZE = 128 diff --git a/packages/server/lib/browsers/webkit.ts b/packages/server/lib/browsers/webkit.ts index ac359b40a44c..58efbd69f96b 100644 --- a/packages/server/lib/browsers/webkit.ts +++ b/packages/server/lib/browsers/webkit.ts @@ -4,6 +4,7 @@ import type playwright from 'playwright-webkit' import type { Browser, BrowserInstance } from './types' import type { Automation } from '../automation' import { WebKitAutomation } from './webkit-automation' +import * as unhandledExceptions from '../unhandled_exceptions' import type { BrowserLaunchOpts, BrowserNewTabOpts } from '@packages/types' import utils from './utils' @@ -28,7 +29,26 @@ export function connectToExisting () { throw new Error('Cypress-in-Cypress is not supported for WebKit.') } +/** + * Playwright adds an `exit` event listener to run a cleanup process. It tries to use the current binary to run a Node script by passing it as argv[1]. + * However, the Electron binary does not support an entrypoint, leading Cypress to think it's being opened in global mode (no args) when this fn is called. + * Solution is to filter out the problematic function. + * TODO(webkit): do we want to run this cleanup script another way? + * @see https://github.com/microsoft/playwright/blob/7e2aec7454f596af452b51a2866e86370291ac8b/packages/playwright-core/src/utils/processLauncher.ts#L191-L203 + */ +function removeBadExitListener () { + const killProcessAndCleanup = process.rawListeners('exit').find((fn) => fn.name === 'killProcessAndCleanup') + + // @ts-expect-error Electron's Process types override those of @types/node, leading to `exit` not being recognized as an event + if (killProcessAndCleanup) process.removeListener('exit', killProcessAndCleanup) + else debug('did not find killProcessAndCleanup, which may cause interactive mode to unexpectedly open') +} + export async function open (browser: Browser, url: string, options: BrowserLaunchOpts, automation: Automation): Promise { + if (!options.experimentalWebKitSupport) { + throw new Error('WebKit was launched, but the experimental feature was not enabled. Please add `experimentalWebKitSupport: true` to your config file to launch WebKit.') + } + // resolve pw from user's project path const pwModulePath = require.resolve('playwright-webkit', { paths: [process.cwd()] }) const pw = await import(pwModulePath) as typeof playwright @@ -52,18 +72,7 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc const pwServer = await pw.webkit.launchServer(launchOptions.preferences) - /** - * Playwright adds an `exit` event listener to run a cleanup process. It tries to use the current binary to run a Node script by passing it as argv[1]. - * However, the Electron binary does not support an entrypoint, leading Cypress to think it's being opened in global mode (no args) when this fn is called. - * Solution is to filter out the problematic function. - * TODO(webkit): do we want to run this cleanup script another way? - * @see https://github.com/microsoft/playwright/blob/7e2aec7454f596af452b51a2866e86370291ac8b/packages/playwright-core/src/utils/processLauncher.ts#L191-L203 - */ - const killProcessAndCleanup = process.rawListeners('exit').find((fn) => fn.name === 'killProcessAndCleanup') - - // @ts-expect-error Electron's Process types override those of @types/node, leading to `exit` not being recognized as an event - if (killProcessAndCleanup) process.removeListener('exit', killProcessAndCleanup) - else debug('did not find killProcessAndCleanup, which may cause interactive mode to unexpectedly open') + removeBadExitListener() const pwBrowser = await pw.webkit.connect(pwServer.wsEndpoint()) @@ -80,6 +89,8 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc debug('pwBrowser disconnected') this.emit('exit') }) + + this.suppressUnhandledEconnreset() } async kill () { @@ -87,6 +98,19 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc await pwBrowser.close() wkAutomation = undefined } + + /** + * An unhandled `read ECONNRESET` in the depths of `playwright-webkit` is causing the process to crash when running kitchensink on Linux. Absent a + * way to attach to the `error` event, this replaces the global `unhandledException` handler with one that will not exit the process on ECONNRESET. + */ + private suppressUnhandledEconnreset () { + unhandledExceptions.handle((err: NodeJS.ErrnoException) => { + return err.code === 'ECONNRESET' + }) + + // restore normal exception handling behavior + this.once('exit', () => unhandledExceptions.handle()) + } } return new WkInstance() diff --git a/packages/server/lib/unhandled_exceptions.js b/packages/server/lib/unhandled_exceptions.js deleted file mode 100644 index cff03d762ab4..000000000000 --- a/packages/server/lib/unhandled_exceptions.js +++ /dev/null @@ -1,9 +0,0 @@ -const globalExceptionHandler = async (err) => { - await require('./errors').logException(err) - process.exit(1) -} - -process.removeAllListeners('unhandledRejection') -process.once('unhandledRejection', globalExceptionHandler) -process.removeAllListeners('uncaughtException') -process.once('uncaughtException', globalExceptionHandler) diff --git a/packages/server/lib/unhandled_exceptions.ts b/packages/server/lib/unhandled_exceptions.ts new file mode 100644 index 000000000000..ce940b8cb0f2 --- /dev/null +++ b/packages/server/lib/unhandled_exceptions.ts @@ -0,0 +1,26 @@ +import Debug from 'debug' +const debug = Debug('cypress:server:unhandled_exceptions') + +export function handle (shouldExitCb?: (err: Error) => boolean) { + function globalExceptionHandler (err: Error) { + if (!shouldExitCb?.(err)) { + debug('suppressing unhandled exception, not exiting %o', { err }) + handle(shouldExitCb) + + return + } + + process.exitCode = 1 + + return require('./errors').logException(err) + .then(() => { + process.exit(1) + }) + } + + process.removeAllListeners('unhandledRejection') + // @ts-expect-error missing unhandledRejection here + process.once('unhandledRejection', globalExceptionHandler) + process.removeAllListeners('uncaughtException') + process.once('uncaughtException', globalExceptionHandler) +}