From 74b8048f70fe0d105eac51bf63617b2b730a940b Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Tue, 29 Dec 2020 16:52:09 -0500 Subject: [PATCH 01/11] fix: use stdio for CDP instead of TCP --- packages/launcher/lib/browsers.ts | 11 ++++++++++- packages/server/lib/browsers/chrome.ts | 20 ++++++++++++++++---- packages/server/lib/browsers/cri-client.ts | 12 +++++++++--- packages/server/package.json | 2 +- yarn.lock | 15 +++++++-------- 5 files changed, 43 insertions(+), 17 deletions(-) diff --git a/packages/launcher/lib/browsers.ts b/packages/launcher/lib/browsers.ts index 821cda74a7b4..e7b9b466edd4 100644 --- a/packages/launcher/lib/browsers.ts +++ b/packages/launcher/lib/browsers.ts @@ -97,6 +97,7 @@ export function launch ( browser: FoundBrowser, url: string, args: string[] = [], + opts: { pipeStdio?: boolean } = {}, ) { log('launching browser %o', { browser, url }) @@ -110,7 +111,15 @@ export function launch ( log('spawning browser with args %o', { args }) - const proc = cp.spawn(browser.path, args, { stdio: ['ignore', 'pipe', 'pipe'] }) + const stdio = ['ignore', 'pipe', 'pipe'] + + if (opts.pipeStdio) { + // also pipe stdio 3 and 4 for access to debugger protocol + stdio.push('pipe', 'pipe') + } + + // @ts-ignore + const proc = cp.spawn(browser.path, args, { stdio }) proc.stdout.on('data', (buf) => { log('%s stdout: %s', browser.name, String(buf).trim()) diff --git a/packages/server/lib/browsers/chrome.ts b/packages/server/lib/browsers/chrome.ts index d4ad82ff5456..90de7449b203 100644 --- a/packages/server/lib/browsers/chrome.ts +++ b/packages/server/lib/browsers/chrome.ts @@ -243,9 +243,15 @@ const _disableRestorePagesPrompt = function (userDir) { .catch(() => { }) } +const supportsStdioCdp = (browser) => browser.majorVersion >= 72 + // After the browser has been opened, we can connect to // its remote interface via a websocket. -const _connectToChromeRemoteInterface = function (port, onError) { +const _connectToChromeRemoteInterface = function (browser, process, port, onError) { + if (supportsStdioCdp(browser)) { + return CriClient.create({ process }, onError) + } + // @ts-ignore la(check.userPort(port), 'expected port number to connect CRI to', port) @@ -255,7 +261,7 @@ const _connectToChromeRemoteInterface = function (port, onError) { .then((wsUrl) => { debug('received wsUrl %s for port %d', wsUrl, port) - return CriClient.create(wsUrl, onError) + return CriClient.create({ target: wsUrl }, onError) }) } @@ -398,6 +404,10 @@ export = { args.push(`--remote-debugging-port=${port}`) args.push('--remote-debugging-address=127.0.0.1') + if (supportsStdioCdp(browser)) { + args.push('--remote-debugging-pipe') + } + return args }, @@ -453,14 +463,16 @@ export = { // first allows us to connect the remote interface, // start video recording and then // we will load the actual page - const launchedBrowser = await utils.launch(browser, 'about:blank', args) + const launchedBrowser = await utils.launch(browser, 'about:blank', args, { + pipeStdio: supportsStdioCdp(browser), + }) la(launchedBrowser, 'did not get launched browser instance') // SECOND connect to the Chrome remote interface // and when the connection is ready // navigate to the actual url - const criClient = await this._connectToChromeRemoteInterface(port, options.onError) + const criClient = await this._connectToChromeRemoteInterface(browser, launchedBrowser, port, options.onError) la(criClient, 'expected Chrome remote interface reference', criClient) diff --git a/packages/server/lib/browsers/cri-client.ts b/packages/server/lib/browsers/cri-client.ts index f776d79f4041..0ba0418a662f 100644 --- a/packages/server/lib/browsers/cri-client.ts +++ b/packages/server/lib/browsers/cri-client.ts @@ -1,6 +1,7 @@ import Bluebird from 'bluebird' import debugModule from 'debug' import _ from 'lodash' +import { ChildProcess } from 'child_process' const chromeRemoteInterface = require('chrome-remote-interface') const errors = require('../errors') @@ -132,7 +133,12 @@ export { chromeRemoteInterface } type DeferredPromise = { resolve: Function, reject: Function } -export const create = Bluebird.method((target: websocketUrl, onAsynchronousError: Function): Bluebird => { +type CreateOpts = { + target?: websocketUrl + process?: ChildProcess +} + +export const create = Bluebird.method((opts: CreateOpts, onAsynchronousError: Function): Bluebird => { const subscriptions: {eventName: CRI.EventName, cb: Function}[] = [] let enqueuedCommands: {command: CRI.Command, params: any, p: DeferredPromise }[] = [] @@ -173,10 +179,10 @@ export const create = Bluebird.method((target: websocketUrl, onAsynchronousError const connect = () => { cri?.close() - debug('connecting %o', { target }) + debug('connecting %o', opts) return chromeRemoteInterface({ - target, + ...opts, local: true, }) .then((newCri) => { diff --git a/packages/server/package.json b/packages/server/package.json index 79444d9b5a94..582b85a8be81 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -37,7 +37,7 @@ "chalk": "2.4.2", "check-more-types": "2.24.0", "chokidar": "3.2.2", - "chrome-remote-interface": "0.28.2", + "chrome-remote-interface": "cypress-io/chrome-remote-interface#8146ca360bb68bfdabe3f031b71b612498c59745", "cli-table3": "0.5.1", "coffeescript": "1.12.7", "color-string": "1.5.4", diff --git a/yarn.lock b/yarn.lock index 904530ec88e8..bf3652c106b6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10366,14 +10366,6 @@ chrome-har-capturer@0.13.4: chrome-remote-interface "^0.25.7" commander "2.x.x" -chrome-remote-interface@0.28.2: - version "0.28.2" - resolved "https://registry.yarnpkg.com/chrome-remote-interface/-/chrome-remote-interface-0.28.2.tgz#6be3554d2c227ff07eb74baa7e5d4911da12a5a6" - integrity sha512-F7mjof7rWvRNsJqhVXuiFU/HWySCxTA9tzpLxUJxVfdLkljwFJ1aMp08AnwXRmmP7r12/doTDOMwaNhFCJsacw== - dependencies: - commander "2.11.x" - ws "^7.2.0" - chrome-remote-interface@^0.25.7: version "0.25.7" resolved "https://registry.yarnpkg.com/chrome-remote-interface/-/chrome-remote-interface-0.25.7.tgz#827e85fbef3cc561a9ef2404eb7eee355968c5bc" @@ -10382,6 +10374,13 @@ chrome-remote-interface@^0.25.7: commander "2.11.x" ws "3.3.x" +chrome-remote-interface@cypress-io/chrome-remote-interface#8146ca360bb68bfdabe3f031b71b612498c59745: + version "0.28.2" + resolved "https://codeload.github.com/cypress-io/chrome-remote-interface/tar.gz/8146ca360bb68bfdabe3f031b71b612498c59745" + dependencies: + commander "2.11.x" + ws "^7.2.0" + chrome-trace-event@^1.0.0, chrome-trace-event@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/chrome-trace-event/-/chrome-trace-event-1.0.2.tgz#234090ee97c7d4ad1a2c4beae27505deffc608a4" From fbb12786be9593eb6e506297ffb8dcd66851a8c9 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Mon, 11 Jan 2021 11:46:46 -0500 Subject: [PATCH 02/11] findTarget --- packages/server/lib/browsers/cri-client.ts | 68 ++++++++++++++++++++-- packages/server/lib/errors.js | 2 + 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/packages/server/lib/browsers/cri-client.ts b/packages/server/lib/browsers/cri-client.ts index 0ba0418a662f..8cbc834b5732 100644 --- a/packages/server/lib/browsers/cri-client.ts +++ b/packages/server/lib/browsers/cri-client.ts @@ -138,17 +138,31 @@ type CreateOpts = { process?: ChildProcess } +type Message = { + method: CRI.Command + params?: any + sessionId?: string +} + export const create = Bluebird.method((opts: CreateOpts, onAsynchronousError: Function): Bluebird => { const subscriptions: {eventName: CRI.EventName, cb: Function}[] = [] - let enqueuedCommands: {command: CRI.Command, params: any, p: DeferredPromise }[] = [] + let enqueuedCommands: {message: Message, params: any, p: DeferredPromise }[] = [] let closed = false // has the user called .close on this? let connected = false // is this currently connected to CDP? let cri let client: CRIWrapper + let sessionId: string | undefined const reconnect = () => { + if (opts.process) { + // reconnecting doesn't make sense for stdio + onAsynchronousError(errors.get('CDP_STDIO_ERROR')) + + return + } + debug('disconnected, attempting to reconnect... %o', { closed }) connected = false @@ -165,7 +179,7 @@ export const create = Bluebird.method((opts: CreateOpts, onAsynchronousError: Fu }) enqueuedCommands.forEach((cmd) => { - cri.send(cmd.command, cmd.params) + cri.sendRaw(cmd.message) .then(cmd.p.resolve, cmd.p.reject) }) @@ -196,6 +210,43 @@ export const create = Bluebird.method((opts: CreateOpts, onAsynchronousError: Fu // @see https://github.com/cyrus-and/chrome-remote-interface/issues/72 cri._notifier.on('disconnect', reconnect) + + if (opts.process) { + // if using stdio, we need to find the target before declaring the connection complete + return findTarget() + } + + return + }) + } + + const findTarget = () => { + return new Promise((resolve, reject) => { + const isAboutBlank = (target) => target.type === 'page' && target.url === 'about:blank' + + const attachToTarget = _.once(async ({ targetId }) => { + const result = await cri.send('Target.attachToTarget', { + targetId, + flatten: true, // enable selecting via sessionId + }).catch(reject) + + sessionId = result.sessionId + + resolve() + }) + + cri.send('Target.setDiscoverTargets', { discover: true }) + .then(() => { + cri.on('Target.targetCreated', (target) => { + if (isAboutBlank(target)) { + attachToTarget(target) + } + }) + + return cri.send('Target.getTargets') + .then(({ targetInfos }) => targetInfos.filter(isAboutBlank).map(attachToTarget)) + }) + .catch(reject) }) } @@ -225,14 +276,23 @@ export const create = Bluebird.method((opts: CreateOpts, onAsynchronousError: Fu ensureMinimumProtocolVersion, getProtocolVersion, send: Bluebird.method((command: CRI.Command, params?: object) => { + const message: Message = { + method: command, + params, + } + + if (sessionId) { + message.sessionId = sessionId + } + const enqueue = () => { return new Bluebird((resolve, reject) => { - enqueuedCommands.push({ command, params, p: { resolve, reject } }) + enqueuedCommands.push({ message, params, p: { resolve, reject } }) }) } if (connected) { - return cri.send(command, params) + return cri.sendRaw(message) .catch((err) => { if (!WEBSOCKET_NOT_OPEN_RE.test(err.message)) { throw err diff --git a/packages/server/lib/errors.js b/packages/server/lib/errors.js index 0e7f68829341..f93f5d936204 100644 --- a/packages/server/lib/errors.js +++ b/packages/server/lib/errors.js @@ -851,6 +851,8 @@ const getMsgByType = function (type, arg1 = {}, arg2, arg3) { There was an error reconnecting to the Chrome DevTools protocol. Please restart the browser. ${arg1.stack}` + case 'CDP_STDIO_ERROR': + return 'The connection between Cypress and Chrome has unexpectedly ended. Please restart the browser.' case 'CDP_RETRYING_CONNECTION': return `Failed to connect to Chrome, retrying in 1 second (attempt ${chalk.yellow(arg1)}/62)` case 'DEPRECATED_BEFORE_BROWSER_LAUNCH_ARGS': From 111324419c2209177ee60a53fe3289d8c34aff02 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Mon, 11 Jan 2021 13:22:54 -0500 Subject: [PATCH 03/11] update tests --- packages/server/lib/browsers/cri-client.ts | 11 ++- .../test/unit/browsers/cri-client_spec.ts | 88 +++++++++++++++---- 2 files changed, 77 insertions(+), 22 deletions(-) diff --git a/packages/server/lib/browsers/cri-client.ts b/packages/server/lib/browsers/cri-client.ts index 8cbc834b5732..54d6e19b6dec 100644 --- a/packages/server/lib/browsers/cri-client.ts +++ b/packages/server/lib/browsers/cri-client.ts @@ -224,15 +224,14 @@ export const create = Bluebird.method((opts: CreateOpts, onAsynchronousError: Fu return new Promise((resolve, reject) => { const isAboutBlank = (target) => target.type === 'page' && target.url === 'about:blank' - const attachToTarget = _.once(async ({ targetId }) => { - const result = await cri.send('Target.attachToTarget', { + const attachToTarget = _.once(({ targetId }) => { + cri.send('Target.attachToTarget', { targetId, flatten: true, // enable selecting via sessionId + }).then((result) => { + sessionId = result.sessionId + resolve() }).catch(reject) - - sessionId = result.sessionId - - resolve() }) cri.send('Target.setDiscoverTargets', { discover: true }) diff --git a/packages/server/test/unit/browsers/cri-client_spec.ts b/packages/server/test/unit/browsers/cri-client_spec.ts index eeb45bcfb520..c584d85b7a4e 100644 --- a/packages/server/test/unit/browsers/cri-client_spec.ts +++ b/packages/server/test/unit/browsers/cri-client_spec.ts @@ -11,32 +11,39 @@ describe('lib/browsers/cri-client', function () { create: typeof create } let send: sinon.SinonStub + let sendRaw: sinon.SinonStub + let criStub: any let criImport: sinon.SinonStub let onError: sinon.SinonStub - let getClient: () => ReturnType + let getClient: (opts?: any) => ReturnType beforeEach(function () { sinon.stub(Bluebird, 'promisify').returnsArg(0) send = sinon.stub() + sendRaw = sinon.stub() onError = sinon.stub() + criStub = { + send, + sendRaw, + on: sinon.stub(), + close: sinon.stub(), + _notifier: new EventEmitter(), + } + criImport = sinon.stub() .withArgs({ target: DEBUGGER_URL, local: true, }) - .resolves({ - send, - close: sinon.stub(), - _notifier: new EventEmitter(), - }) + .resolves(criStub) criClient = proxyquire('../lib/browsers/cri-client', { 'chrome-remote-interface': criImport, }) - getClient = () => criClient.create(DEBUGGER_URL, onError) + getClient = (opts = { target: DEBUGGER_URL }) => criClient.create(opts, onError) }) context('.create', function () { @@ -46,19 +53,65 @@ describe('lib/browsers/cri-client', function () { expect(client.send).to.be.instanceOf(Function) }) + context('with process', function () { + let process: any + + beforeEach(function () { + process = { /** stubbed */} + + criImport.withArgs({ + process, + local: true, + }) + .resolves(criStub) + }) + + it('finds and attaches to target and persists sessionId', async function () { + const target = { + targetId: 'good', + type: 'page', + url: 'about:blank', + } + + const otherTarget = { + targetId: 'bad', + } + + send + .withArgs('Target.setDiscoverTargets').resolves() + .withArgs('Target.getTargets').resolves({ targetInfos: [otherTarget, target] }) + .withArgs('Target.attachToTarget', { targetId: 'good', flatten: true }).resolves({ sessionId: 'session-1' }) + + sendRaw.resolves() + + const client = await getClient({ process }) + + await client.send('Browser.getVersion') + + expect(sendRaw).to.be.calledWith({ + method: 'Browser.getVersion', + params: undefined, + sessionId: 'session-1', + }) + }) + }) + context('#send', function () { - it('calls cri.send with command and data', async function () { - send.resolves() + it('calls cri.sendRaw with command and data', async function () { + sendRaw.resolves() const client = await getClient() client.send('Browser.getVersion', { baz: 'quux' }) - expect(send).to.be.calledWith('Browser.getVersion', { baz: 'quux' }) + expect(sendRaw).to.be.calledWith({ + method: 'Browser.getVersion', + params: { baz: 'quux' }, + }) }) - it('rejects if cri.send rejects', async function () { + it('rejects if cri.sendRaw rejects', async function () { const err = new Error - send.rejects(err) + sendRaw.rejects(err) const client = await getClient() await expect(client.send('Browser.getVersion', { baz: 'quux' })) @@ -74,14 +127,14 @@ describe('lib/browsers/cri-client', function () { it(`with '${msg}'`, async function () { const err = new Error(msg) - send.onFirstCall().rejects(err) - send.onSecondCall().resolves() + sendRaw.onFirstCall().rejects(err) + sendRaw.onSecondCall().resolves() const client = await getClient() await client.send('Browser.getVersion', { baz: 'quux' }) - expect(send).to.be.calledTwice + expect(sendRaw).to.be.calledTwice }) }) }) @@ -90,7 +143,10 @@ describe('lib/browsers/cri-client', function () { context('#ensureMinimumProtocolVersion', function () { function withProtocolVersion (actual, test) { if (actual) { - send.withArgs('Browser.getVersion') + sendRaw.withArgs({ + method: 'Browser.getVersion', + params: undefined, + }) .resolves({ protocolVersion: actual }) } From 09204c1196c578419a44d95335054a7f5234e20b Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Mon, 11 Jan 2021 13:29:58 -0500 Subject: [PATCH 04/11] update debugging code for ws + stdio compat --- packages/server/lib/browsers/cri-client.ts | 49 +++++++++++----------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/packages/server/lib/browsers/cri-client.ts b/packages/server/lib/browsers/cri-client.ts index 54d6e19b6dec..95b855fcaeca 100644 --- a/packages/server/lib/browsers/cri-client.ts +++ b/packages/server/lib/browsers/cri-client.ts @@ -83,41 +83,40 @@ const getMajorMinorVersion = (version: string): Version => { const maybeDebugCdpMessages = (cri) => { if (debugVerboseReceive.enabled) { - cri._ws.on('message', (data) => { - data = _ - .chain(JSON.parse(data)) - .tap((data) => { - ([ - 'params.data', // screencast frame data - 'result.data', // screenshot data - ]).forEach((truncatablePath) => { - const str = _.get(data, truncatablePath) - - if (!_.isString(str)) { - return - } + const handleMessage = cri._handleMessage - _.set(data, truncatablePath, _.truncate(str, { - length: 100, - omission: `... [truncated string of total bytes: ${str.length}]`, - })) - }) + cri._handleMessage = (message) => { + const formatted = _.cloneDeep(message) + + ;([ + 'params.data', // screencast frame data + 'result.data', // screenshot data + ]).forEach((truncatablePath) => { + const str = _.get(formatted, truncatablePath) + + if (!_.isString(str)) { + return + } - return data + _.set(formatted, truncatablePath, _.truncate(str, { + length: 100, + omission: `... [truncated string of total bytes: ${str.length}]`, + })) }) - .value() - debugVerboseReceive('received CDP message %o', data) - }) + debugVerboseReceive('received CDP message %o', formatted) + + return handleMessage.call(cri, message) + } } if (debugVerboseSend.enabled) { - const send = cri._ws.send + const send = cri._send - cri._ws.send = (data, callback) => { + cri._send = (data, callback) => { debugVerboseSend('sending CDP command %o', JSON.parse(data)) - return send.call(cri._ws, data, callback) + return send.call(cri, data, callback) } } } From 8435aff27a7f0308bd5eed752befa0e08987f70c Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Mon, 11 Jan 2021 13:50:31 -0500 Subject: [PATCH 05/11] update e2e test --- packages/server/lib/browsers/chrome.ts | 22 ++++++--- packages/server/test/e2e/5_cdp_spec.ts | 63 ++++++++++++++++---------- 2 files changed, 53 insertions(+), 32 deletions(-) diff --git a/packages/server/lib/browsers/chrome.ts b/packages/server/lib/browsers/chrome.ts index 90de7449b203..649572818a3b 100644 --- a/packages/server/lib/browsers/chrome.ts +++ b/packages/server/lib/browsers/chrome.ts @@ -35,6 +35,8 @@ type ChromePreferences = { localState: object } +const staticCdpPort = Number(process.env.CYPRESS_REMOTE_DEBUGGING_PORT) + const pathToExtension = extension.getPathToExtension() const pathToTheme = extension.getPathToTheme() @@ -173,9 +175,7 @@ const _writeChromePreferences = (userDir: string, originalPrefs: ChromePreferenc } const getRemoteDebuggingPort = async () => { - const port = Number(process.env.CYPRESS_REMOTE_DEBUGGING_PORT) - - return port || utils.getPort() + return staticCdpPort || utils.getPort() } /** @@ -243,12 +243,20 @@ const _disableRestorePagesPrompt = function (userDir) { .catch(() => { }) } -const supportsStdioCdp = (browser) => browser.majorVersion >= 72 +const useStdioCdp = (browser) => { + return ( + // CDP via stdio doesn't seem to work in browsers older than 72 + // @see https://github.com/cyrus-and/chrome-remote-interface/issues/381#issuecomment-445277683 + browser.majorVersion >= 72 + // allow users to force TCP by specifying a port in environment + && !staticCdpPort + ) +} // After the browser has been opened, we can connect to // its remote interface via a websocket. const _connectToChromeRemoteInterface = function (browser, process, port, onError) { - if (supportsStdioCdp(browser)) { + if (useStdioCdp(browser)) { return CriClient.create({ process }, onError) } @@ -404,7 +412,7 @@ export = { args.push(`--remote-debugging-port=${port}`) args.push('--remote-debugging-address=127.0.0.1') - if (supportsStdioCdp(browser)) { + if (useStdioCdp(browser)) { args.push('--remote-debugging-pipe') } @@ -464,7 +472,7 @@ export = { // start video recording and then // we will load the actual page const launchedBrowser = await utils.launch(browser, 'about:blank', args, { - pipeStdio: supportsStdioCdp(browser), + pipeStdio: useStdioCdp(browser), }) la(launchedBrowser, 'did not get launched browser instance') diff --git a/packages/server/test/e2e/5_cdp_spec.ts b/packages/server/test/e2e/5_cdp_spec.ts index c80af4680e13..770138ea02e1 100644 --- a/packages/server/test/e2e/5_cdp_spec.ts +++ b/packages/server/test/e2e/5_cdp_spec.ts @@ -4,36 +4,49 @@ import Fixtures from '../support/helpers/fixtures' describe('e2e cdp', function () { e2e.setup() - let restoreEnv: Function - beforeEach(() => { - restoreEnv = mockedEnv({ - CYPRESS_REMOTE_DEBUGGING_PORT: '7777', + context('with TCP transport', function () { + let restoreEnv: Function + + beforeEach(() => { + restoreEnv = mockedEnv({ + CYPRESS_REMOTE_DEBUGGING_PORT: '7777', + }) }) - }) - afterEach(() => { - restoreEnv() - }) + afterEach(() => { + restoreEnv() + }) - // NOTE: this test takes almost a minute and is largely redundant with protocol_spec - e2e.it.skip('fails when remote debugging port cannot be connected to', { - project: Fixtures.projectPath('remote-debugging-port-removed'), - spec: 'spec.ts', - browser: 'chrome', - expectedExitCode: 1, + // NOTE: this test takes almost a minute and is largely redundant with protocol_spec + e2e.it.skip('fails when remote debugging port cannot be connected to', { + project: Fixtures.projectPath('remote-debugging-port-removed'), + spec: 'spec.ts', + browser: 'chrome', + expectedExitCode: 1, + }) + + // https://github.com/cypress-io/cypress/issues/5685 + e2e.it('handles disconnections as expected', { + project: Fixtures.projectPath('remote-debugging-disconnect'), + spec: 'spec.ts', + browser: 'chrome', + expectedExitCode: 1, + snapshot: true, + onStdout: (stdout) => { + // the location of this warning is non-deterministic + return stdout.replace('The automation client disconnected. Cannot continue running tests.\n', '') + }, + }) }) - // https://github.com/cypress-io/cypress/issues/5685 - e2e.it('handles disconnections as expected', { - project: Fixtures.projectPath('remote-debugging-disconnect'), - spec: 'spec.ts', - browser: 'chrome', - expectedExitCode: 1, - snapshot: true, - onStdout: (stdout) => { - // the location of this warning is non-deterministic - return stdout.replace('The automation client disconnected. Cannot continue running tests.\n', '') - }, + // @see https://github.com/cypress-io/cypress/pull/14348 + context('with stdio transport', function () { + e2e.it('can run tests in chrome even with remote-debugging-port omitted', { + project: Fixtures.projectPath('remote-debugging-port-removed'), + spec: 'spec.ts', + browser: 'chrome', + expectedExitCode: 0, + }) }) }) From b060e2b09ca638f630f1615d43b012be6eb97aeb Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Mon, 11 Jan 2021 14:23:16 -0500 Subject: [PATCH 06/11] tests --- .../server/__snapshots__/5_cdp_spec.ts.js | 64 ++++++++++++++++++- packages/server/lib/browsers/chrome.ts | 32 +++++++--- packages/server/lib/browsers/cri-client.ts | 24 ++++--- packages/server/lib/errors.js | 3 + packages/server/test/e2e/5_cdp_spec.ts | 12 ++++ .../cypress/plugins.js | 6 +- 6 files changed, 118 insertions(+), 23 deletions(-) diff --git a/packages/server/__snapshots__/5_cdp_spec.ts.js b/packages/server/__snapshots__/5_cdp_spec.ts.js index 6c378afb74dc..fe1beaf263a4 100644 --- a/packages/server/__snapshots__/5_cdp_spec.ts.js +++ b/packages/server/__snapshots__/5_cdp_spec.ts.js @@ -59,4 +59,66 @@ Error: connect ECONNREFUSED 127.0.0.1:7777 ✖ 1 of 1 failed (100%) XX:XX - - 1 - - -` \ No newline at end of file +` + +exports['e2e cdp / with stdio transport / falls back to connecting via tcp when stdio cannot be connected'] = ` + +==================================================================================================== + + (Run Starting) + + ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ + │ Cypress: 1.2.3 │ + │ Browser: FooBrowser 88 │ + │ Specs: 1 found (spec.ts) │ + │ Searched: cypress/integration/spec.ts │ + └────────────────────────────────────────────────────────────────────────────────────────────────┘ + + +──────────────────────────────────────────────────────────────────────────────────────────────────── + + Running: spec.ts (1 of 1) +Warning: Cypress failed to connect to Chrome via stdio after 1 second. Falling back to TCP... + + + passes + ✓ passes + + + 1 passing + + + (Results) + + ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ + │ Tests: 1 │ + │ Passing: 1 │ + │ Failing: 0 │ + │ Pending: 0 │ + │ Skipped: 0 │ + │ Screenshots: 0 │ + │ Video: true │ + │ Duration: X seconds │ + │ Spec Ran: spec.ts │ + └────────────────────────────────────────────────────────────────────────────────────────────────┘ + + + (Video) + + - Started processing: Compressing to 32 CRF + - Finished processing: /XXX/XXX/XXX/cypress/videos/spec.ts.mp4 (X second) + + +==================================================================================================== + + (Run Finished) + + + Spec Tests Passing Failing Pending Skipped + ┌────────────────────────────────────────────────────────────────────────────────────────────────┐ + │ ✔ spec.ts XX:XX 1 1 - - - │ + └────────────────────────────────────────────────────────────────────────────────────────────────┘ + ✔ All specs passed! XX:XX 1 1 - - - + + +` diff --git a/packages/server/lib/browsers/chrome.ts b/packages/server/lib/browsers/chrome.ts index 649572818a3b..a6031c9ea8f7 100644 --- a/packages/server/lib/browsers/chrome.ts +++ b/packages/server/lib/browsers/chrome.ts @@ -13,6 +13,7 @@ import * as CriClient from './cri-client' import * as protocol from './protocol' import utils from './utils' import { Browser } from './types' +import errors from '../errors' // TODO: this is defined in `cypress-npm-api` but there is currently no way to get there type CypressConfiguration = any @@ -36,6 +37,7 @@ type ChromePreferences = { } const staticCdpPort = Number(process.env.CYPRESS_REMOTE_DEBUGGING_PORT) +const stdioTimeoutMs = Number(process.env.CYPRESS_CDP_TARGET_TIMEOUT) || 15000 const pathToExtension = extension.getPathToExtension() const pathToTheme = extension.getPathToTheme() @@ -256,20 +258,30 @@ const useStdioCdp = (browser) => { // After the browser has been opened, we can connect to // its remote interface via a websocket. const _connectToChromeRemoteInterface = function (browser, process, port, onError) { - if (useStdioCdp(browser)) { - return CriClient.create({ process }, onError) - } + const connectTcp = () => { + // @ts-ignore + la(check.userPort(port), 'expected port number to connect CRI to', port) - // @ts-ignore - la(check.userPort(port), 'expected port number to connect CRI to', port) + debug('connecting to Chrome remote interface at random port %d', port) - debug('connecting to Chrome remote interface at random port %d', port) + return protocol.getWsTargetFor(port) + .then((wsUrl) => { + debug('received wsUrl %s for port %d', wsUrl, port) + + return CriClient.create({ target: wsUrl }, onError) + }) + } + + if (!useStdioCdp(browser)) { + return connectTcp() + } - return protocol.getWsTargetFor(port) - .then((wsUrl) => { - debug('received wsUrl %s for port %d', wsUrl, port) + return CriClient.create({ process }, onError) + .timeout(stdioTimeoutMs) + .catch(Bluebird.TimeoutError, () => { + errors.warning('CDP_STDIO_TIMEOUT', browser.displayName, stdioTimeoutMs) - return CriClient.create({ target: wsUrl }, onError) + return connectTcp() }) } diff --git a/packages/server/lib/browsers/cri-client.ts b/packages/server/lib/browsers/cri-client.ts index 95b855fcaeca..e145548722c9 100644 --- a/packages/server/lib/browsers/cri-client.ts +++ b/packages/server/lib/browsers/cri-client.ts @@ -1,4 +1,4 @@ -import Bluebird from 'bluebird' +import Promise from 'bluebird' import debugModule from 'debug' import _ from 'lodash' import { ChildProcess } from 'child_process' @@ -44,17 +44,17 @@ interface CRIWrapper { /** * Get the `protocolVersion` supported by the browser. */ - getProtocolVersion (): Bluebird + getProtocolVersion (): Promise /** * Rejects if `protocolVersion` is less than the current version. * @param protocolVersion CDP version string (ex: 1.3) */ - ensureMinimumProtocolVersion(protocolVersion: string): Bluebird + ensureMinimumProtocolVersion(protocolVersion: string): Promise /** * Sends a command to the Chrome remote interface. * @example client.send('Page.navigate', { url }) */ - send (command: CRI.Command, params?: object): Bluebird + send (command: CRI.Command, params?: object): Promise /** * Registers callback for particular event. * @see https://github.com/cyrus-and/chrome-remote-interface#class-cdp @@ -63,7 +63,7 @@ interface CRIWrapper { /** * Calls underlying remote interface client close */ - close (): Bluebird + close (): Promise } interface Version { @@ -143,7 +143,7 @@ type Message = { sessionId?: string } -export const create = Bluebird.method((opts: CreateOpts, onAsynchronousError: Function): Bluebird => { +export const create = Promise.method((opts: CreateOpts, onAsynchronousError: Function): Promise => { const subscriptions: {eventName: CRI.EventName, cb: Function}[] = [] let enqueuedCommands: {message: Message, params: any, p: DeferredPromise }[] = [] @@ -204,8 +204,8 @@ export const create = Bluebird.method((opts: CreateOpts, onAsynchronousError: Fu maybeDebugCdpMessages(cri) - cri.send = Bluebird.promisify(cri.send, { context: cri }) - cri.close = Bluebird.promisify(cri.close, { context: cri }) + cri.send = Promise.promisify(cri.send, { context: cri }) + cri.close = Promise.promisify(cri.close, { context: cri }) // @see https://github.com/cyrus-and/chrome-remote-interface/issues/72 cri._notifier.on('disconnect', reconnect) @@ -220,14 +220,18 @@ export const create = Bluebird.method((opts: CreateOpts, onAsynchronousError: Fu } const findTarget = () => { + debug('finding CDP target...') + return new Promise((resolve, reject) => { const isAboutBlank = (target) => target.type === 'page' && target.url === 'about:blank' const attachToTarget = _.once(({ targetId }) => { + debug('attaching to target %o', { targetId }) cri.send('Target.attachToTarget', { targetId, flatten: true, // enable selecting via sessionId }).then((result) => { + debug('attached to target %o', result) sessionId = result.sessionId resolve() }).catch(reject) @@ -273,7 +277,7 @@ export const create = Bluebird.method((opts: CreateOpts, onAsynchronousError: Fu client = { ensureMinimumProtocolVersion, getProtocolVersion, - send: Bluebird.method((command: CRI.Command, params?: object) => { + send: Promise.method((command: CRI.Command, params?: object) => { const message: Message = { method: command, params, @@ -284,7 +288,7 @@ export const create = Bluebird.method((opts: CreateOpts, onAsynchronousError: Fu } const enqueue = () => { - return new Bluebird((resolve, reject) => { + return new Promise((resolve, reject) => { enqueuedCommands.push({ message, params, p: { resolve, reject } }) }) } diff --git a/packages/server/lib/errors.js b/packages/server/lib/errors.js index f93f5d936204..c58dfbf5d87b 100644 --- a/packages/server/lib/errors.js +++ b/packages/server/lib/errors.js @@ -5,6 +5,7 @@ const chalk = require('chalk') const AU = require('ansi_up') const Promise = require('bluebird') const { stripIndent } = require('./util/strip_indent') +const humanTime = require('./util/human_time') const ansi_up = new AU.default @@ -853,6 +854,8 @@ const getMsgByType = function (type, arg1 = {}, arg2, arg3) { ${arg1.stack}` case 'CDP_STDIO_ERROR': return 'The connection between Cypress and Chrome has unexpectedly ended. Please restart the browser.' + case 'CDP_STDIO_TIMEOUT': + return `Warning: Cypress failed to connect to ${arg1} via stdio after ${humanTime.long(arg2)}. Falling back to TCP...` case 'CDP_RETRYING_CONNECTION': return `Failed to connect to Chrome, retrying in 1 second (attempt ${chalk.yellow(arg1)}/62)` case 'DEPRECATED_BEFORE_BROWSER_LAUNCH_ARGS': diff --git a/packages/server/test/e2e/5_cdp_spec.ts b/packages/server/test/e2e/5_cdp_spec.ts index 770138ea02e1..677c7d744ea6 100644 --- a/packages/server/test/e2e/5_cdp_spec.ts +++ b/packages/server/test/e2e/5_cdp_spec.ts @@ -48,5 +48,17 @@ describe('e2e cdp', function () { browser: 'chrome', expectedExitCode: 0, }) + + e2e.it('falls back to connecting via tcp when stdio cannot be connected', { + project: Fixtures.projectPath('remote-debugging-port-removed'), + processEnv: { + CY_REMOVE_PIPE: '1', + CYPRESS_CDP_TARGET_TIMEOUT: '1000', + }, + spec: 'spec.ts', + browser: 'chrome', + expectedExitCode: 0, + snapshot: true, + }) }) }) diff --git a/packages/server/test/support/fixtures/projects/remote-debugging-port-removed/cypress/plugins.js b/packages/server/test/support/fixtures/projects/remote-debugging-port-removed/cypress/plugins.js index bcc725e90a00..244534b11503 100644 --- a/packages/server/test/support/fixtures/projects/remote-debugging-port-removed/cypress/plugins.js +++ b/packages/server/test/support/fixtures/projects/remote-debugging-port-removed/cypress/plugins.js @@ -4,8 +4,10 @@ module.exports = (on) => { on('before:browser:launch', (browser = {}, options) => { la(browser.family === 'chromium', 'this test can only be run with a chromium-family browser') - // remove debugging port so that the browser connection fails - const newArgs = options.args.filter((arg) => !arg.startsWith('--remote-debugging-port=')) + const cdpArg = process.env.CY_REMOVE_PIPE ? '--remote-debugging-pipe' : '--remote-debugging-port' + + // remove debugging pipe or port so that the browser connection fails + const newArgs = options.args.filter((arg) => !arg.startsWith(cdpArg)) la(newArgs.length === options.args.length - 1, 'exactly one argument should have been removed') From 02e3b1d9e995b63b9fb432cd0ac4599967800249 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Mon, 11 Jan 2021 14:28:25 -0500 Subject: [PATCH 07/11] minimize ze diff --- packages/server/lib/browsers/cri-client.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/server/lib/browsers/cri-client.ts b/packages/server/lib/browsers/cri-client.ts index e145548722c9..47546930688c 100644 --- a/packages/server/lib/browsers/cri-client.ts +++ b/packages/server/lib/browsers/cri-client.ts @@ -1,4 +1,4 @@ -import Promise from 'bluebird' +import Bluebird from 'bluebird' import debugModule from 'debug' import _ from 'lodash' import { ChildProcess } from 'child_process' @@ -44,17 +44,17 @@ interface CRIWrapper { /** * Get the `protocolVersion` supported by the browser. */ - getProtocolVersion (): Promise + getProtocolVersion (): Bluebird /** * Rejects if `protocolVersion` is less than the current version. * @param protocolVersion CDP version string (ex: 1.3) */ - ensureMinimumProtocolVersion(protocolVersion: string): Promise + ensureMinimumProtocolVersion(protocolVersion: string): Bluebird /** * Sends a command to the Chrome remote interface. * @example client.send('Page.navigate', { url }) */ - send (command: CRI.Command, params?: object): Promise + send (command: CRI.Command, params?: object): Bluebird /** * Registers callback for particular event. * @see https://github.com/cyrus-and/chrome-remote-interface#class-cdp @@ -63,7 +63,7 @@ interface CRIWrapper { /** * Calls underlying remote interface client close */ - close (): Promise + close (): Bluebird } interface Version { @@ -143,7 +143,7 @@ type Message = { sessionId?: string } -export const create = Promise.method((opts: CreateOpts, onAsynchronousError: Function): Promise => { +export const create = Bluebird.method((opts: CreateOpts, onAsynchronousError: Function): Bluebird => { const subscriptions: {eventName: CRI.EventName, cb: Function}[] = [] let enqueuedCommands: {message: Message, params: any, p: DeferredPromise }[] = [] @@ -204,8 +204,8 @@ export const create = Promise.method((opts: CreateOpts, onAsynchronousError: Fun maybeDebugCdpMessages(cri) - cri.send = Promise.promisify(cri.send, { context: cri }) - cri.close = Promise.promisify(cri.close, { context: cri }) + cri.send = Bluebird.promisify(cri.send, { context: cri }) + cri.close = Bluebird.promisify(cri.close, { context: cri }) // @see https://github.com/cyrus-and/chrome-remote-interface/issues/72 cri._notifier.on('disconnect', reconnect) @@ -222,7 +222,7 @@ export const create = Promise.method((opts: CreateOpts, onAsynchronousError: Fun const findTarget = () => { debug('finding CDP target...') - return new Promise((resolve, reject) => { + return new Bluebird((resolve, reject) => { const isAboutBlank = (target) => target.type === 'page' && target.url === 'about:blank' const attachToTarget = _.once(({ targetId }) => { @@ -277,7 +277,7 @@ export const create = Promise.method((opts: CreateOpts, onAsynchronousError: Fun client = { ensureMinimumProtocolVersion, getProtocolVersion, - send: Promise.method((command: CRI.Command, params?: object) => { + send: Bluebird.method((command: CRI.Command, params?: object) => { const message: Message = { method: command, params, @@ -288,7 +288,7 @@ export const create = Promise.method((opts: CreateOpts, onAsynchronousError: Fun } const enqueue = () => { - return new Promise((resolve, reject) => { + return new Bluebird((resolve, reject) => { enqueuedCommands.push({ message, params, p: { resolve, reject } }) }) } From 435fb916f496d78b60f802aefe0c2f613432f937 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Mon, 11 Jan 2021 15:03:42 -0500 Subject: [PATCH 08/11] fix cri issue causing next spec to not run --- packages/server/package.json | 2 +- yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/server/package.json b/packages/server/package.json index 582b85a8be81..048c044a2e66 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -37,7 +37,7 @@ "chalk": "2.4.2", "check-more-types": "2.24.0", "chokidar": "3.2.2", - "chrome-remote-interface": "cypress-io/chrome-remote-interface#8146ca360bb68bfdabe3f031b71b612498c59745", + "chrome-remote-interface": "cypress-io/chrome-remote-interface#147192810f29951cd96c5e406495e9b4d740ba95", "cli-table3": "0.5.1", "coffeescript": "1.12.7", "color-string": "1.5.4", diff --git a/yarn.lock b/yarn.lock index bf3652c106b6..b766673032ed 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10374,9 +10374,9 @@ chrome-remote-interface@^0.25.7: commander "2.11.x" ws "3.3.x" -chrome-remote-interface@cypress-io/chrome-remote-interface#8146ca360bb68bfdabe3f031b71b612498c59745: +chrome-remote-interface@cypress-io/chrome-remote-interface#147192810f29951cd96c5e406495e9b4d740ba95: version "0.28.2" - resolved "https://codeload.github.com/cypress-io/chrome-remote-interface/tar.gz/8146ca360bb68bfdabe3f031b71b612498c59745" + resolved "https://codeload.github.com/cypress-io/chrome-remote-interface/tar.gz/147192810f29951cd96c5e406495e9b4d740ba95" dependencies: commander "2.11.x" ws "^7.2.0" From cce7a90dcdc91af5948c1e70bf240d4547cc079f Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Mon, 11 Jan 2021 15:27:50 -0500 Subject: [PATCH 09/11] try bumping stdioTimeoutMs --- packages/server/lib/browsers/chrome.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/lib/browsers/chrome.ts b/packages/server/lib/browsers/chrome.ts index a6031c9ea8f7..3bc1cd99b62d 100644 --- a/packages/server/lib/browsers/chrome.ts +++ b/packages/server/lib/browsers/chrome.ts @@ -37,7 +37,7 @@ type ChromePreferences = { } const staticCdpPort = Number(process.env.CYPRESS_REMOTE_DEBUGGING_PORT) -const stdioTimeoutMs = Number(process.env.CYPRESS_CDP_TARGET_TIMEOUT) || 15000 +const stdioTimeoutMs = Number(process.env.CYPRESS_CDP_TARGET_TIMEOUT) || 60000 const pathToExtension = extension.getPathToExtension() const pathToTheme = extension.getPathToTheme() From 7f41984d7cfc261293fb31c13ebef59ed9f96fd3 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Mon, 11 Jan 2021 16:36:42 -0500 Subject: [PATCH 10/11] update snapshot --- packages/server/__snapshots__/5_cdp_spec.ts.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/__snapshots__/5_cdp_spec.ts.js b/packages/server/__snapshots__/5_cdp_spec.ts.js index fe1beaf263a4..b81717e36992 100644 --- a/packages/server/__snapshots__/5_cdp_spec.ts.js +++ b/packages/server/__snapshots__/5_cdp_spec.ts.js @@ -1,4 +1,4 @@ -exports['e2e cdp / handles disconnections as expected'] = ` +exports['e2e cdp / with TCP transport / handles disconnections as expected'] = ` ==================================================================================================== From 7a3ace5b47ec54de1d1a7f0a20a722004e6312e2 Mon Sep 17 00:00:00 2001 From: Zach Bloomquist Date: Tue, 19 Jan 2021 12:15:06 -0500 Subject: [PATCH 11/11] add success message --- packages/server/__snapshots__/5_cdp_spec.ts.js | 1 + packages/server/lib/browsers/chrome.ts | 8 ++++++-- packages/server/lib/errors.js | 2 ++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/server/__snapshots__/5_cdp_spec.ts.js b/packages/server/__snapshots__/5_cdp_spec.ts.js index b81717e36992..82cd998b06f9 100644 --- a/packages/server/__snapshots__/5_cdp_spec.ts.js +++ b/packages/server/__snapshots__/5_cdp_spec.ts.js @@ -79,6 +79,7 @@ exports['e2e cdp / with stdio transport / falls back to connecting via tcp when Running: spec.ts (1 of 1) Warning: Cypress failed to connect to Chrome via stdio after 1 second. Falling back to TCP... +Connecting to Chrome via TCP was successful, continuing with tests. passes diff --git a/packages/server/lib/browsers/chrome.ts b/packages/server/lib/browsers/chrome.ts index 3bc1cd99b62d..21aed30da4e0 100644 --- a/packages/server/lib/browsers/chrome.ts +++ b/packages/server/lib/browsers/chrome.ts @@ -278,10 +278,14 @@ const _connectToChromeRemoteInterface = function (browser, process, port, onErro return CriClient.create({ process }, onError) .timeout(stdioTimeoutMs) - .catch(Bluebird.TimeoutError, () => { + .catch(Bluebird.TimeoutError, async () => { errors.warning('CDP_STDIO_TIMEOUT', browser.displayName, stdioTimeoutMs) - return connectTcp() + const client = await connectTcp() + + errors.warning('CDP_FALLBACK_SUCCEEDED', browser.displayName) + + return client }) } diff --git a/packages/server/lib/errors.js b/packages/server/lib/errors.js index c58dfbf5d87b..63667bed410c 100644 --- a/packages/server/lib/errors.js +++ b/packages/server/lib/errors.js @@ -856,6 +856,8 @@ const getMsgByType = function (type, arg1 = {}, arg2, arg3) { return 'The connection between Cypress and Chrome has unexpectedly ended. Please restart the browser.' case 'CDP_STDIO_TIMEOUT': return `Warning: Cypress failed to connect to ${arg1} via stdio after ${humanTime.long(arg2)}. Falling back to TCP...` + case 'CDP_FALLBACK_SUCCEEDED': + return `Connecting to ${arg1} via TCP was successful, continuing with tests.` case 'CDP_RETRYING_CONNECTION': return `Failed to connect to Chrome, retrying in 1 second (attempt ${chalk.yellow(arg1)}/62)` case 'DEPRECATED_BEFORE_BROWSER_LAUNCH_ARGS':