From 05333076c66a4f623237202ad6fcc289cbf10c23 Mon Sep 17 00:00:00 2001 From: Bill Glesias Date: Fri, 18 Oct 2024 23:37:37 -0400 Subject: [PATCH] chore: gracefully handle CDP address not returning from webdriver capabilities, prompting a browser relaunch (#30393) * chore: gracefully handle CDP address not returning from webdriver capabilities, prompting a browser relaunch Update packages/server/lib/browsers/firefox.ts add inline comment to github comment thread [run ci] strongly type errors more consistently [run ci] * add test for firefox cdp error for visual snapshot [run ci] --- .../FIREFOX_CDP_FAILED_TO_CONNECT.html | 38 +++++++++++++++ packages/errors/src/errors.ts | 3 ++ .../test/unit/visualSnapshotErrors_spec.ts | 5 ++ packages/server/lib/browsers/firefox.ts | 35 ++++++++++++-- packages/server/lib/modes/run.ts | 48 ++++++++++++++----- .../server/test/unit/browsers/firefox_spec.ts | 16 +++++++ 6 files changed, 128 insertions(+), 17 deletions(-) create mode 100644 packages/errors/__snapshot-html__/FIREFOX_CDP_FAILED_TO_CONNECT.html diff --git a/packages/errors/__snapshot-html__/FIREFOX_CDP_FAILED_TO_CONNECT.html b/packages/errors/__snapshot-html__/FIREFOX_CDP_FAILED_TO_CONNECT.html new file mode 100644 index 000000000000..319c7ee73765 --- /dev/null +++ b/packages/errors/__snapshot-html__/FIREFOX_CDP_FAILED_TO_CONNECT.html @@ -0,0 +1,38 @@ + + + + + + + + + + + + 
Failed to spawn CDP with Firefox. Retrying...
+
\ No newline at end of file diff --git a/packages/errors/src/errors.ts b/packages/errors/src/errors.ts index f297495cba6a..589353504a4e 100644 --- a/packages/errors/src/errors.ts +++ b/packages/errors/src/errors.ts @@ -146,6 +146,9 @@ export const AllCypressErrors = { TESTS_DID_NOT_START_RETRYING: (arg1: string) => { return errTemplate`Timed out waiting for the browser to connect. ${fmt.off(arg1)}` }, + FIREFOX_CDP_FAILED_TO_CONNECT: (arg1: string) => { + return errTemplate`Failed to spawn CDP with Firefox. ${fmt.off(arg1)}` + }, TESTS_DID_NOT_START_FAILED: () => { return errTemplate`The browser never connected. Something is wrong. The tests cannot run. Aborting...` }, diff --git a/packages/errors/test/unit/visualSnapshotErrors_spec.ts b/packages/errors/test/unit/visualSnapshotErrors_spec.ts index e4ab2205dfc8..77b534bf7701 100644 --- a/packages/errors/test/unit/visualSnapshotErrors_spec.ts +++ b/packages/errors/test/unit/visualSnapshotErrors_spec.ts @@ -375,6 +375,11 @@ describe('visual error templates', () => { retryingAgain: ['Retrying again...'], } }, + FIREFOX_CDP_FAILED_TO_CONNECT: () => { + return { + default: ['Retrying...'], + } + }, TESTS_DID_NOT_START_FAILED: () => { return { default: [], diff --git a/packages/server/lib/browsers/firefox.ts b/packages/server/lib/browsers/firefox.ts index 6f0a85efcc03..cc5fc6ffdab2 100644 --- a/packages/server/lib/browsers/firefox.ts +++ b/packages/server/lib/browsers/firefox.ts @@ -17,12 +17,24 @@ import mimeDb from 'mime-db' import type { BrowserCriClient } from './browser-cri-client' import type { Automation } from '../automation' import { getCtx } from '@packages/data-context' -import { getError } from '@packages/errors' +import { getError, SerializedError } from '@packages/errors' import type { BrowserLaunchOpts, BrowserNewTabOpts, RunModeVideoApi } from '@packages/types' import type { RemoteConfig } from 'webdriver' import type { GeckodriverParameters } from 'geckodriver' import { WebDriver } from './webdriver' +export class CDPFailedToStartFirefox extends Error { + private static readonly ERROR_NAME = 'CDPFailedToStartFirefox' + constructor (message) { + super(message) + this.name = CDPFailedToStartFirefox.ERROR_NAME + } + + public static isCDPFailedToStartFirefoxError (error?: SerializedError): error is CDPFailedToStartFirefox { + return error?.name === CDPFailedToStartFirefox.ERROR_NAME + } +} + const debug = Debug('cypress:server:browsers:firefox') const debugVerbose = Debug('cypress-verbose:server:browsers:firefox') @@ -623,6 +635,8 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc }, // @see https://firefox-source-docs.mozilla.org/testing/geckodriver/Capabilities.html#moz-debuggeraddress // we specify the debugger address option for Webdriver, which will return us the CDP address when the capability is returned. + // NOTE: this typing is fixed in @wdio/types 9.1.0 https://github.com/webdriverio/webdriverio/commit/ed14717ac4269536f9e7906e4d1612f74650b09b + // Once we have a node engine that can support the package (i.e., electron 32+ update) we can update the package // @ts-expect-error 'moz:debuggerAddress': true, // @see https://webdriver.io/docs/capabilities/#wdiogeckodriveroptions @@ -645,10 +659,6 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc debugVerbose(`received capabilities %o`, webdriverClient.capabilities) - const cdpPort = parseInt(new URL(`ws://${webdriverClient.capabilities['moz:debuggerAddress']}`).port) - - debug(`CDP running on port ${cdpPort}`) - const browserPID: number = webdriverClient.capabilities['moz:processID'] debug(`firefox running on pid: ${browserPID}`) @@ -676,6 +686,21 @@ export async function open (browser: Browser, url: string, options: BrowserLaunc return browserReturnStatus || driverReturnStatus } + // In some cases, the webdriver session will NOT return the moz:debuggerAddress capability even though + // we set it to true in the capabilities. This is out of our control, so when this happens, we fail the browser + // and gracefully terminate the related processes and attempt to relaunch the browser in the hopes we get a + // CDP address. @see https://github.com/cypress-io/cypress/issues/30352#issuecomment-2405701867 for more details. + if (!webdriverClient.capabilities['moz:debuggerAddress']) { + debug(`firefox failed to spawn with CDP connection. Failing current instance and retrying`) + // since this fails before the instance is created, we need to kill the processes here or else they will stay open + browserInstanceWrapper.kill() + throw new CDPFailedToStartFirefox(`webdriver session failed to start CDP even though "moz:debuggerAddress" was provided. Please try to relaunch the browser`) + } + + const cdpPort = parseInt(new URL(`ws://${webdriverClient.capabilities['moz:debuggerAddress']}`).port) + + debug(`CDP running on port ${cdpPort}`) + // makes it so get getRemoteDebuggingPort() is calculated correctly process.env.CYPRESS_REMOTE_DEBUGGING_PORT = cdpPort.toString() diff --git a/packages/server/lib/modes/run.ts b/packages/server/lib/modes/run.ts index 6014c1e23b6b..db70e790eb7a 100644 --- a/packages/server/lib/modes/run.ts +++ b/packages/server/lib/modes/run.ts @@ -28,6 +28,8 @@ import type { ProtocolManager } from '../cloud/protocol' import { telemetry } from '@packages/telemetry' import { CypressRunResult, createPublicBrowser, createPublicConfig, createPublicRunResults, createPublicSpec, createPublicSpecResults } from './results' import { EarlyExitTerminator } from '../util/graceful_crash_handling' +import { CDPFailedToStartFirefox } from '../browsers/firefox' +import type { CypressError } from '@packages/errors' type SetScreenshotMetadata = (data: TakeScreenshotProps) => void type ScreenshotMetadata = ReturnType @@ -497,17 +499,7 @@ async function waitForBrowserToConnect (options: { project: Project, socketId: s coreData.didBrowserPreviouslyHaveUnexpectedExit = false } - return Bluebird.all([ - waitForSocketConnection(project, socketId), - // TODO: remove the need to extend options and coerce this type - launchBrowser(options as typeof options & { setScreenshotMetadata: SetScreenshotMetadata }), - ]) - .timeout(browserTimeout) - .then(() => { - telemetry.getSpan(`waitForBrowserToConnect:attempt:${browserLaunchAttempt}`)?.end() - }) - .catch(Bluebird.TimeoutError, async (err) => { - debug('Catch on waitForBrowserToConnect') + async function retryOnError (err: CypressError) { telemetry.getSpan(`waitForBrowserToConnect:attempt:${browserLaunchAttempt}`)?.end() console.log('') @@ -519,7 +511,12 @@ async function waitForBrowserToConnect (options: { project: Project, socketId: s // try again up to 3 attempts const word = browserLaunchAttempt === 1 ? 'Retrying...' : 'Retrying again...' - errors.warning('TESTS_DID_NOT_START_RETRYING', word) + if (CDPFailedToStartFirefox.isCDPFailedToStartFirefoxError(err?.originalError)) { + errors.warning('FIREFOX_CDP_FAILED_TO_CONNECT', word) + } else { + errors.warning('TESTS_DID_NOT_START_RETRYING', word) + } + browserLaunchAttempt += 1 return await wait() @@ -529,6 +526,33 @@ async function waitForBrowserToConnect (options: { project: Project, socketId: s errors.log(err) onError(err) + } + + return Bluebird.all([ + waitForSocketConnection(project, socketId), + // TODO: remove the need to extend options and coerce this type + launchBrowser(options as typeof options & { setScreenshotMetadata: SetScreenshotMetadata }), + ]).catch((e: CypressError) => { + // if the error wrapped is a CDPFailedToStartFirefox, try to relaunch the browser + if (CDPFailedToStartFirefox.isCDPFailedToStartFirefoxError(e?.originalError)) { + // if CDP fails to connect, which is ultimately out of our control and in the hands of webdriver + // we retry launching the browser in the hopes the session is spawned correctly + debug(`Caught in launchBrowser: ${e.details}`) + + return retryOnError(e) + } + + // otherwise, fail + throw e + }) + .timeout(browserTimeout) + .then(() => { + telemetry.getSpan(`waitForBrowserToConnect:attempt:${browserLaunchAttempt}`)?.end() + }) + .catch(Bluebird.TimeoutError, async (err) => { + debug('Catch on waitForBrowserToConnect') + + return retryOnError(err as CypressError) }) } diff --git a/packages/server/test/unit/browsers/firefox_spec.ts b/packages/server/test/unit/browsers/firefox_spec.ts index f492d6269671..ead4d9bc83a8 100644 --- a/packages/server/test/unit/browsers/firefox_spec.ts +++ b/packages/server/test/unit/browsers/firefox_spec.ts @@ -605,6 +605,22 @@ describe('lib/browsers/firefox', () => { // makes sure the exit event is called to signal to the rest of cypress server that the processes are killed expect(instance.emit).to.have.been.calledWith('exit') }) + + it('throws CDPFailedToStartFirefox if the mox:debuggerAddress capability is not returned by webdriver', function () { + delete wdInstance.capabilities['moz:debuggerAddress'] + sinon.stub(process, 'kill').returns(true) + + return firefox.open(this.browser, 'http://', this.options, this.automation).catch((err) => { + // make sure we through the correct error here to prompt @packages/server/lib/modes/run.ts + // to retry the browser connection + expect(err.details).to.include('CDPFailedToStartFirefox: webdriver session failed to start CDP even though "moz:debuggerAddress" was provided. Please try to relaunch the browser') + expect(err.type).to.equal('FIREFOX_COULD_NOT_CONNECT') + // kills the browser + expect(process.kill).to.have.been.calledWith(1234) + // kills the webdriver process / geckodriver process + expect(process.kill).to.have.been.calledWith(5678) + }) + }) }) })