fix: add support for checking multiple hosts when connecting to CDP (#…

…24155)

packages/network/lib/connect.ts

@@ -31,7 +31,7 @@ export function getAddress (port: number, hostname: string): Bluebird<net.Addres
   const lookupAsync = Bluebird.promisify<LookupAddress[], string, LookupAllOptions>(dns.lookup, { context: dns })
 
   // this does not go out to the network to figure
-  // out the addresess. in fact it respects the /etc/hosts file
+  // out the addresses. in fact it respects the /etc/hosts file
   // https://github.com/nodejs/node/blob/dbdbdd4998e163deecefbb1d34cda84f749844a4/lib/dns.js#L108
   // https://nodejs.org/api/dns.html#dns_dns_lookup_hostname_options_callback
   // @ts-ignore
@@ -52,7 +52,7 @@ export function getDelayForRetry (iteration) {
   return [0, 100, 200, 200][iteration]
 }
 
-interface RetryingOptions {
+export interface RetryingOptions {
   family: 4 | 6 | 0
   port: number
   host: string | undefined

packages/network/test/unit/connect_spec.ts

@@ -2,8 +2,13 @@ import { connect } from '../..'
 import { expect } from 'chai'
 import sinon from 'sinon'
 import net from 'net'
+import type { RetryingOptions } from '../../lib/connect'
 
 describe('lib/connect', () => {
+  afterEach(() => {
+    sinon.restore()
+  })
+
   context('.byPortAndAddress', () => {
     it('destroy connection immediately onConnect', () => {
       const socket = new net.Socket()
@@ -24,4 +29,34 @@ describe('lib/connect', () => {
       })
     })
   })
+
+  context('createRetryingSocket', () => {
+    it('cancels retries', (done) => {
+      const getDelayMsForRetry = (iteration) => {
+        if (iteration < 2) {
+          return 1
+        }
+
+        // return undefined to cancel any additional retries
+        return
+      }
+
+      const opts: RetryingOptions = {
+        family: 0,
+        useTls: false,
+        port: 3000,
+        host: '127.0.0.1',
+        getDelayMsForRetry,
+      }
+
+      const netSpy = sinon.spy(net, 'connect')
+
+      connect.createRetryingSocket(opts, (err: any, sock, _retry) => {
+        expect((err)?.code).to.equal('ECONNREFUSED')
+        expect(netSpy).to.be.calledThrice
+        expect(sock).to.be.undefined
+        done()
+      })
+    })
+  })
 })

packages/server/lib/browsers/browser-cri-client.ts

@@ -4,15 +4,18 @@ import { _connectAsync, _getDelayMsForRetry } from './protocol'
 import * as errors from '../errors'
 import { create, CriClient } from './cri-client'
 
-const HOST = '127.0.0.1'
-
 const debug = Debug('cypress:server:browsers:browser-cri-client')
 
 interface Version {
   major: number
   minor: number
 }
 
+// since we may be attempting to connect to multiple hosts, 'connected'
+// is set to true once one of the connections succeeds so the others
+// can be cancelled
+let connected = false
+
 const isVersionGte = (a: Version, b: Version) => {
   return a.major > b.major || (a.major === b.major && a.minor >= b.minor)
 }
@@ -23,23 +26,43 @@ const getMajorMinorVersion = (version: string): Version => {
   return { major, minor }
 }
 
-const ensureLiveBrowser = async (port: number, browserName: string) => {
+const tryBrowserConnection = async (host: string, port: number, browserName: string): Promise<string | undefined> => {
   const connectOpts = {
-    host: HOST,
+    host,
     port,
     getDelayMsForRetry: (i) => {
+      // if we successfully connected to a different host, cancel any remaining connection attempts
+      if (connected) {
+        debug('cancelling any additional retries %o', { host, port })
+
+        return
+      }
+
       return _getDelayMsForRetry(i, browserName)
     },
   }
 
   try {
     await _connectAsync(connectOpts)
+    connected = true
+
+    return host
   } catch (err) {
-    debug('failed to connect to CDP %o', { connectOpts, err })
-    errors.throwErr('CDP_COULD_NOT_CONNECT', browserName, port, err)
+    // don't throw an error if we've already connected
+    if (!connected) {
+      debug('failed to connect to CDP %o', { connectOpts, err })
+      errors.throwErr('CDP_COULD_NOT_CONNECT', browserName, port, err)
+    }
+
+    return
   }
 }
 
+const ensureLiveBrowser = async (hosts: string[], port: number, browserName: string) => {
+  // go through all of the hosts and attempt to make a connection
+  return Promise.any(hosts.map((host) => tryBrowserConnection(host, port, browserName)))
+}
+
 const retryWithIncreasingDelay = async <T>(retryable: () => Promise<T>, browserName: string, port: number): Promise<T> => {
   let retryIndex = 0
 
@@ -68,25 +91,26 @@ const retryWithIncreasingDelay = async <T>(retryable: () => Promise<T>, browserN
 
 export class BrowserCriClient {
   currentlyAttachedTarget: CriClient | undefined
-  private constructor (private browserClient: CriClient, private versionInfo, private port: number, private browserName: string, private onAsynchronousError: Function) {}
+  private constructor (private browserClient: CriClient, private versionInfo, private host: string, private port: number, private browserName: string, private onAsynchronousError: Function) {}
 
   /**
    * Factory method for the browser cri client. Connects to the browser and then returns a chrome remote interface wrapper around the
    * browser target
    *
+   * @param hosts the hosts to which to attempt to connect
    * @param port the port to which to connect
    * @param browserName the display name of the browser being launched
    * @param onAsynchronousError callback for any cdp fatal errors
    * @returns a wrapper around the chrome remote interface that is connected to the browser target
    */
-  static async create (port: number, browserName: string, onAsynchronousError: Function, onReconnect?: (client: CriClient) => void): Promise<BrowserCriClient> {
-    await ensureLiveBrowser(port, browserName)
+  static async create (hosts: string[], port: number, browserName: string, onAsynchronousError: Function, onReconnect?: (client: CriClient) => void): Promise<BrowserCriClient> {
+    const host = await ensureLiveBrowser(hosts, port, browserName)
 
     return retryWithIncreasingDelay(async () => {
-      const versionInfo = await CRI.Version({ host: HOST, port })
+      const versionInfo = await CRI.Version({ host, port, useHostName: true })
       const browserClient = await create(versionInfo.webSocketDebuggerUrl, onAsynchronousError, undefined, undefined, onReconnect)
 
-      return new BrowserCriClient(browserClient, versionInfo, port, browserName, onAsynchronousError)
+      return new BrowserCriClient(browserClient, versionInfo, host!, port, browserName, onAsynchronousError)
     }, browserName, port)
   }
 
@@ -111,7 +135,7 @@ export class BrowserCriClient {
    * @returns the chrome remote interface wrapper for the target
    */
   attachToTargetUrl = async (url: string): Promise<CriClient> => {
-    // Continue trying to re-attach until succcessful.
+    // Continue trying to re-attach until successful.
     // If the browser opens slowly, this will fail until
     // The browser and automation API is ready, so we try a few
     // times until eventually timing out.
@@ -125,7 +149,7 @@ export class BrowserCriClient {
         throw new Error(`Could not find url target in browser ${url}. Targets were ${JSON.stringify(targets)}`)
       }
 
-      this.currentlyAttachedTarget = await create(target.targetId, this.onAsynchronousError, HOST, this.port)
+      this.currentlyAttachedTarget = await create(target.targetId, this.onAsynchronousError, this.host, this.port)
 
       return this.currentlyAttachedTarget
     }, this.browserName, this.port)
@@ -157,7 +181,7 @@ export class BrowserCriClient {
     ])
 
     if (target) {
-      this.currentlyAttachedTarget = await create(target.targetId, this.onAsynchronousError, HOST, this.port)
+      this.currentlyAttachedTarget = await create(target.targetId, this.onAsynchronousError, this.host, this.port)
     }
   }
 
@@ -169,6 +193,7 @@ export class BrowserCriClient {
       await this.currentlyAttachedTarget.close()
     }
 
+    connected = false
     await this.browserClient.close()
   }
 }

packages/server/lib/browsers/chrome.ts

@@ -583,7 +583,7 @@ export = {
     debug('connecting to existing chrome instance with url and debugging port', { url: options.url, port })
     if (!options.onError) throw new Error('Missing onError in connectToExisting')
 
-    const browserCriClient = await BrowserCriClient.create(port, browser.displayName, options.onError, onReconnect)
+    const browserCriClient = await BrowserCriClient.create(['127.0.0.1'], port, browser.displayName, options.onError, onReconnect)
 
     if (!options.url) throw new Error('Missing url in connectToExisting')
 
@@ -681,7 +681,7 @@ export = {
     // navigate to the actual url
     if (!options.onError) throw new Error('Missing onError in chrome#open')
 
-    browserCriClient = await BrowserCriClient.create(port, browser.displayName, options.onError, onReconnect)
+    browserCriClient = await BrowserCriClient.create(['127.0.0.1'], port, browser.displayName, options.onError, onReconnect)
 
     la(browserCriClient, 'expected Chrome remote interface reference', browserCriClient)
 

packages/server/lib/browsers/cri-client.ts

@@ -142,6 +142,7 @@ export const create = async (target: string, onAsynchronousError: Function, host
       port,
       target,
       local: true,
+      useHostName: true,
     })
 
     connected = true

packages/server/lib/browsers/firefox-util.ts

@@ -135,7 +135,7 @@ async function connectToNewSpec (options, automation: Automation, browserCriClie
 }
 
 async function setupRemote (remotePort, automation, onError, options): Promise<BrowserCriClient> {
-  const browserCriClient = await BrowserCriClient.create(remotePort, 'Firefox', onError)
+  const browserCriClient = await BrowserCriClient.create(['127.0.0.1', '::1'], remotePort, 'Firefox', onError)
   const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank')
 
   await CdpAutomation.create(pageCriClient.send, pageCriClient.on, browserCriClient.resetBrowserTargets, automation, options.experimentalSessionAndOrigin)

packages/server/package.json

@@ -42,7 +42,7 @@
     "chalk": "2.4.2",
     "check-more-types": "2.24.0",
     "chokidar": "3.5.1",
-    "chrome-remote-interface": "0.31.1",
+    "chrome-remote-interface": "0.31.3",
     "cli-table3": "0.5.1",
     "coffeescript": "1.12.7",
     "color-string": "1.5.5",

packages/server/test/unit/browsers/browser-cri-client_spec.ts

@@ -3,10 +3,11 @@ import * as CriClient from '../../../lib/browsers/cri-client'
 import { expect, proxyquire, sinon } from '../../spec_helper'
 import * as protocol from '../../../lib/browsers/protocol'
 import { stripAnsi } from '@packages/errors'
+import net from 'net'
 
 const HOST = '127.0.0.1'
 const PORT = 50505
-const THROWS_PORT = 66666
+const THROWS_PORT = 65535
 
 describe('lib/browsers/cri-client', function () {
   let browserCriClient: {
@@ -29,8 +30,8 @@ describe('lib/browsers/cri-client', function () {
     criImport = sinon.stub()
 
     criImport.Version = sinon.stub()
-    criImport.Version.withArgs({ host: HOST, port: PORT }).resolves({ webSocketDebuggerUrl: 'http://web/socket/url' })
-    criImport.Version.withArgs({ host: HOST, port: THROWS_PORT })
+    criImport.Version.withArgs({ host: HOST, port: PORT, useHostName: true }).resolves({ webSocketDebuggerUrl: 'http://web/socket/url' })
+    criImport.Version.withArgs({ host: HOST, port: THROWS_PORT, useHostName: true })
     .onFirstCall().throws()
     .onSecondCall().throws()
     .onThirdCall().resolves({ webSocketDebuggerUrl: 'http://web/socket/url' })
@@ -46,7 +47,7 @@ describe('lib/browsers/cri-client', function () {
       'chrome-remote-interface': criImport,
     })
 
-    getClient = () => browserCriClient.BrowserCriClient.create(PORT, 'Chrome', onError)
+    getClient = () => browserCriClient.BrowserCriClient.create(['127.0.0.1'], PORT, 'Chrome', onError)
   })
 
   context('.create', function () {
@@ -63,13 +64,37 @@ describe('lib/browsers/cri-client', function () {
       await expect(getClient()).to.be.rejected
     })
 
+    it('attempts to connect to multiple hosts', async function () {
+      (protocol._connectAsync as any).restore()
+      const socket = new net.Socket()
+
+      sinon.stub(net, 'connect').callsFake((opts, onConnect) => {
+        process.nextTick(() => {
+          // throw an error on 127.0.0.1 so ::1 can connect
+          if (opts.host === '127.0.0.1') {
+            socket.emit('error', new Error())
+          } else {
+            onConnect()
+          }
+        })
+
+        return socket
+      })
+
+      criImport.Version.withArgs({ host: '::1', port: THROWS_PORT, useHostName: true }).resolves({ webSocketDebuggerUrl: 'http://web/socket/url' })
+
+      await browserCriClient.BrowserCriClient.create(['127.0.0.1', '::1'], THROWS_PORT, 'Chrome', onError)
+
+      expect(criImport.Version).to.be.calledOnce
+    })
+
     it('retries when Version fails', async function () {
       sinon.stub(protocol, '_getDelayMsForRetry')
       .onFirstCall().returns(100)
       .onSecondCall().returns(100)
       .onThirdCall().returns(100)
 
-      const client = await browserCriClient.BrowserCriClient.create(THROWS_PORT, 'Chrome', onError)
+      const client = await browserCriClient.BrowserCriClient.create(['127.0.0.1'], THROWS_PORT, 'Chrome', onError)
 
       expect(client.attachToTargetUrl).to.be.instanceOf(Function)
 
@@ -81,7 +106,7 @@ describe('lib/browsers/cri-client', function () {
       .onFirstCall().returns(100)
       .onSecondCall().returns(undefined)
 
-      await expect(browserCriClient.BrowserCriClient.create(THROWS_PORT, 'Chrome', onError)).to.be.rejected
+      await expect(browserCriClient.BrowserCriClient.create(['127.0.0.1'], THROWS_PORT, 'Chrome', onError)).to.be.rejected
 
       expect(criImport.Version).to.be.calledTwice
     })

packages/server/test/unit/browsers/firefox_spec.ts

@@ -10,6 +10,9 @@ import stripAnsi from 'strip-ansi'
 import Foxdriver from '@benmalka/foxdriver'
 import * as firefox from '../../../lib/browsers/firefox'
 import firefoxUtil from '../../../lib/browsers/firefox-util'
+import { CdpAutomation } from '../../../lib/browsers/cdp_automation'
+import { BrowserCriClient } from '../../../lib/browsers/browser-cri-client'
+import { CriClient } from '../../../lib/browsers/cri-client'
 
 const path = require('path')
 const _ = require('lodash')
@@ -115,7 +118,7 @@ describe('lib/browsers/firefox', () => {
     it('calls connectToNewSpec in firefoxUtil', function () {
       sinon.stub(firefoxUtil, 'connectToNewSpec').withArgs(50505, this.options, this.automation).resolves()
 
-      firefox.connectToNewSpec(this.browser, 50505, this.options, this.automation)
+      firefox.connectToNewSpec(this.browser, this.options, this.automation)
 
       expect(firefoxUtil.connectToNewSpec).to.be.called
     })
@@ -514,5 +517,36 @@ describe('lib/browsers/firefox', () => {
         expect(memory.forceGarbageCollection).to.be.calledTwice
       })
     })
+
+    context('#setupRemote', function () {
+      it('correctly sets up the remote agent', async function () {
+        const criClientStub: CriClient = {
+          targetId: '',
+          send: sinon.stub(),
+          on: sinon.stub(),
+          close: sinon.stub(),
+        }
+
+        const browserCriClient: BrowserCriClient = sinon.createStubInstance(BrowserCriClient)
+
+        browserCriClient.attachToTargetUrl = sinon.stub().resolves(criClientStub)
+
+        sinon.stub(BrowserCriClient, 'create').resolves(browserCriClient)
+        sinon.stub(CdpAutomation, 'create').resolves()
+
+        const actual = await firefoxUtil.setupRemote(port, null, null, { experimentalSessionAndOrigin: false })
+
+        expect(actual).to.equal(browserCriClient)
+        expect(browserCriClient.attachToTargetUrl).to.be.calledWith('about:blank')
+        expect(BrowserCriClient.create).to.be.calledWith(['127.0.0.1', '::1'], port, 'Firefox', null)
+        expect(CdpAutomation.create).to.be.calledWith(
+          criClientStub.send,
+          criClientStub.on,
+          browserCriClient.resetBrowserTargets,
+          null,
+          false,
+        )
+      })
+    })
   })
 })

yarn.lock

@@ -11873,10 +11873,10 @@ chrome-har-capturer@0.13.4:
     chrome-remote-interface "^0.25.7"
     commander "2.x.x"
 
-chrome-remote-interface@0.31.1:
-  version "0.31.1"
-  resolved "https://registry.npmjs.org/chrome-remote-interface/-/chrome-remote-interface-0.31.1.tgz#87c37c81f10d9f0832b6d6a42e52ac2c7ebb4008"
-  integrity sha512-cvNTnXfx4kYCaeh2sEKrdlqZsYRleACPL47O8LrrjihVfBQbfPmf03vVqSSm7SIeqyo2P77ZXovrBAs4D/nopQ==
+chrome-remote-interface@0.31.3:
+  version "0.31.3"
+  resolved "https://registry.yarnpkg.com/chrome-remote-interface/-/chrome-remote-interface-0.31.3.tgz#bd01b89f5f0e968f7eeb37b8b7c5ac20e6e1f4d0"
+  integrity sha512-NTwb1YNPHXLTus1RjqsLxJmdViKwKJg/lrFEcM6pbyQy04Ow2QKWHXyPpxzwE+dFsJghWuvSAdTy4W0trluz1g==
   dependencies:
     commander "2.11.x"
     ws "^7.2.0"