From d29b5c69a9b02bf5098d589ac8adbcdda6d6ace0 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Wed, 7 Sep 2022 16:24:00 -0400 Subject: [PATCH 01/20] 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) +} From 9a9bf4023259ea929b2c8da6e4b06acab9c5d46e Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Wed, 7 Sep 2022 17:13:57 -0400 Subject: [PATCH 02/20] feat: gate WebKit behind `experimentalWebKitSupport` in prod --- cli/types/cypress.d.ts | 7 +++++- .../config/__snapshots__/index.spec.ts.js | 3 +++ packages/config/src/options.ts | 6 +++++ packages/config/test/project/utils.spec.ts | 2 ++ .../src/data/ProjectConfigManager.ts | 18 +++++++++++---- .../CHROME_WEB_SECURITY_NOT_SUPPORTED.html | 4 ++-- packages/errors/src/errors.ts | 2 +- .../cypress/fixtures/config.json | 8 +++++-- .../frontend-shared/src/locales/en-US.json | 4 ++++ .../schemaTypes/objectTypes/gql-Browser.ts | 2 +- .../launchpad/src/setup/OpenBrowserList.vue | 7 ++++-- packages/server/lib/browsers/index.ts | 12 ++-------- packages/server/lib/browsers/utils.ts | 23 ++++++++----------- packages/server/lib/experiments.ts | 2 ++ packages/server/lib/modes/run.ts | 2 +- packages/server/lib/open_project.ts | 3 ++- packages/server/test/unit/project_spec.js | 2 +- packages/types/src/browser.ts | 3 +-- packages/types/src/config.ts | 2 +- packages/types/src/server.ts | 2 +- .../__snapshots__/web_security_spec.js | 2 +- system-tests/test/web_security_spec.js | 2 +- 22 files changed, 72 insertions(+), 46 deletions(-) diff --git a/cli/types/cypress.d.ts b/cli/types/cypress.d.ts index a9c7e65d5b92..2eceb04bb3f9 100644 --- a/cli/types/cypress.d.ts +++ b/cli/types/cypress.d.ts @@ -82,7 +82,7 @@ declare namespace Cypress { type BrowserChannel = 'stable' | 'canary' | 'beta' | 'dev' | 'nightly' | string - type BrowserFamily = 'chromium' | 'firefox' + type BrowserFamily = 'chromium' | 'firefox' | 'webkit' /** * Describes a browser Cypress can control @@ -2876,6 +2876,11 @@ declare namespace Cypress { * @default false */ experimentalStudio: boolean + /** + * Adds support for testing in the WebKit browser engine used by Safari. See https://on.cypress.io/webkit for more information. + * @default false + */ + experimentalWebKitSupport: boolean /** * Number of times to retry a failed test. * If a number is set, tests will retry in both runMode and openMode. diff --git a/packages/config/__snapshots__/index.spec.ts.js b/packages/config/__snapshots__/index.spec.ts.js index 260e9ba18c67..0a367f0dbcc1 100644 --- a/packages/config/__snapshots__/index.spec.ts.js +++ b/packages/config/__snapshots__/index.spec.ts.js @@ -40,6 +40,7 @@ exports['config/src/index .getDefaultValues returns list of public config keys 1 "experimentalSourceRewriting": false, "experimentalSingleTabRunMode": false, "experimentalStudio": false, + "experimentalWebKitSupport": false, "fileServerFolder": "", "fixturesFolder": "cypress/fixtures", "excludeSpecPattern": "*.hot-update.js", @@ -123,6 +124,7 @@ exports['config/src/index .getDefaultValues returns list of public config keys f "experimentalSourceRewriting": false, "experimentalSingleTabRunMode": false, "experimentalStudio": false, + "experimentalWebKitSupport": false, "fileServerFolder": "", "fixturesFolder": "cypress/fixtures", "excludeSpecPattern": "*.hot-update.js", @@ -202,6 +204,7 @@ exports['config/src/index .getPublicConfigKeys returns list of public config key "experimentalSourceRewriting", "experimentalSingleTabRunMode", "experimentalStudio", + "experimentalWebKitSupport", "fileServerFolder", "fixturesFolder", "excludeSpecPattern", diff --git a/packages/config/src/options.ts b/packages/config/src/options.ts index 843fb6e4ac71..dd39341ae123 100644 --- a/packages/config/src/options.ts +++ b/packages/config/src/options.ts @@ -227,6 +227,12 @@ const driverConfigOptions: Array = [ validation: validate.isBoolean, isExperimental: true, requireRestartOnChange: 'browser', + }, { + name: 'experimentalWebKitSupport', + defaultValue: false, + validation: validate.isBoolean, + isExperimental: true, + requireRestartOnChange: 'server', }, { name: 'fileServerFolder', defaultValue: '', diff --git a/packages/config/test/project/utils.spec.ts b/packages/config/test/project/utils.spec.ts index 2af4ff0b36be..843b5069d358 100644 --- a/packages/config/test/project/utils.spec.ts +++ b/packages/config/test/project/utils.spec.ts @@ -1044,6 +1044,7 @@ describe('config/src/project/utils', () => { experimentalSingleTabRunMode: { value: false, from: 'default' }, experimentalStudio: { value: false, from: 'default' }, experimentalSourceRewriting: { value: false, from: 'default' }, + experimentalWebKitSupport: { value: false, from: 'default' }, fileServerFolder: { value: '', from: 'default' }, fixturesFolder: { value: 'cypress/fixtures', from: 'default' }, hosts: { value: null, from: 'default' }, @@ -1137,6 +1138,7 @@ describe('config/src/project/utils', () => { experimentalSingleTabRunMode: { value: false, from: 'default' }, experimentalStudio: { value: false, from: 'default' }, experimentalSourceRewriting: { value: false, from: 'default' }, + experimentalWebKitSupport: { value: false, from: 'default' }, env: { foo: { value: 'foo', diff --git a/packages/data-context/src/data/ProjectConfigManager.ts b/packages/data-context/src/data/ProjectConfigManager.ts index 2717bedc4fba..7b2bbe4e9b12 100644 --- a/packages/data-context/src/data/ProjectConfigManager.ts +++ b/packages/data-context/src/data/ProjectConfigManager.ts @@ -500,14 +500,22 @@ export class ProjectConfigManager { } fullConfig.browsers = fullConfig.browsers?.map((browser) => { - if (browser.family === 'chromium' || fullConfig.chromeWebSecurity) { - return browser + if (browser.family === 'webkit' && !fullConfig.experimentalWebKitSupport) { + return { + ...browser, + disabled: true, + warning: '`playwright-webkit` is installed and WebKit is detected, but `experimentalWebKitSupport` is not enabled in your Cypress config. Set it to `true` to use WebKit.', + } } - return { - ...browser, - warning: browser.warning || getError('CHROME_WEB_SECURITY_NOT_SUPPORTED', browser.name).message, + if (browser.family !== 'chromium' && fullConfig.chromeWebSecurity) { + return { + ...browser, + warning: browser.warning || getError('CHROME_WEB_SECURITY_NOT_SUPPORTED', browser.name).message, + } } + + return browser }) // If we have withBrowsers set to false, it means we're coming from the legacy config.get API diff --git a/packages/errors/__snapshot-html__/CHROME_WEB_SECURITY_NOT_SUPPORTED.html b/packages/errors/__snapshot-html__/CHROME_WEB_SECURITY_NOT_SUPPORTED.html index 78a59080d890..e8b5cd152712 100644 --- a/packages/errors/__snapshot-html__/CHROME_WEB_SECURITY_NOT_SUPPORTED.html +++ b/packages/errors/__snapshot-html__/CHROME_WEB_SECURITY_NOT_SUPPORTED.html @@ -34,7 +34,7 @@ - 
Your project has set the configuration option: chromeWebSecurity to false
+    
Your project has set the configuration option: `chromeWebSecurity` to `false`.
 
 This option will not have an effect in Firefox. Tests that rely on web security being disabled will not run as expected.
-

\ No newline at end of file
+
diff --git a/packages/errors/src/errors.ts b/packages/errors/src/errors.ts index 2426db69c7c6..073cc28b012b 100644 --- a/packages/errors/src/errors.ts +++ b/packages/errors/src/errors.ts @@ -86,7 +86,7 @@ export const AllCypressErrors = { }, CHROME_WEB_SECURITY_NOT_SUPPORTED: (browser: string) => { return errTemplate`\ - Your project has set the configuration option: ${fmt.highlight(`chromeWebSecurity`)} to ${fmt.highlightTertiary(`false`)} + Your project has set the configuration option: \`chromeWebSecurity\` to \`false\`. This option will not have an effect in ${fmt.off(_.capitalize(browser))}. Tests that rely on web security being disabled will not run as expected.` }, diff --git a/packages/frontend-shared/cypress/fixtures/config.json b/packages/frontend-shared/cypress/fixtures/config.json index 19221291c299..0b4a5cdfdd07 100644 --- a/packages/frontend-shared/cypress/fixtures/config.json +++ b/packages/frontend-shared/cypress/fixtures/config.json @@ -43,8 +43,7 @@ "displayName": "Electron", "version": "94.0.4606.81", "path": "", - "majorVersion": 94, - "info": "Electron is the default browser that comes with Cypress. This is the default browser that runs in headless mode. Selecting this browser is useful when debugging. The version number indicates the underlying Chromium version that Electron uses." + "majorVersion": 94 } ], "from": "default", @@ -103,6 +102,11 @@ "from": "default", "field": "experimentalSessionAndOrigin" }, + { + "value": false, + "from": "default", + "field": "experimentalWebKitSupport" + }, { "value": "", "from": "default", diff --git a/packages/frontend-shared/src/locales/en-US.json b/packages/frontend-shared/src/locales/en-US.json index d93a9b32bd9b..5859ca6698a9 100644 --- a/packages/frontend-shared/src/locales/en-US.json +++ b/packages/frontend-shared/src/locales/en-US.json @@ -507,6 +507,10 @@ "experimentalStudio": { "name": "Studio", "description": "Generate and save commands directly to your test suite by interacting with your app as an end user would." + }, + "experimentalWebKitSupport": { + "name": "WebKit Support", + "description": "Adds support for testing in the WebKit browser engine used by Safari. See https://on.cypress.io/webkit for more information." } }, "device": { diff --git a/packages/graphql/src/schemaTypes/objectTypes/gql-Browser.ts b/packages/graphql/src/schemaTypes/objectTypes/gql-Browser.ts index 160d4c7b34cb..b243ed326f89 100644 --- a/packages/graphql/src/schemaTypes/objectTypes/gql-Browser.ts +++ b/packages/graphql/src/schemaTypes/objectTypes/gql-Browser.ts @@ -8,7 +8,7 @@ export const Browser = objectType({ definition (t) { t.nonNull.string('channel') t.nonNull.boolean('disabled', { - resolve: () => false, + resolve: (source) => source.disabled || false, }) t.nonNull.boolean('isSelected', { diff --git a/packages/launchpad/src/setup/OpenBrowserList.vue b/packages/launchpad/src/setup/OpenBrowserList.vue index e762af6c15a5..e7215d573d4e 100644 --- a/packages/launchpad/src/setup/OpenBrowserList.vue +++ b/packages/launchpad/src/setup/OpenBrowserList.vue @@ -27,7 +27,7 @@ }" >