retry CRI.List when connecting to the browser

packages/server/lib/browsers/protocol.js

@@ -25,10 +25,7 @@ function _getDelayMsForRetry (i) {
 
 function _connectAsync (opts) {
   return Promise.fromCallback((cb) => {
-    connect.createRetryingSocket({
-      getDelayMsForRetry: _getDelayMsForRetry,
-      ...opts,
-    }, cb)
+    connect.createRetryingSocket(opts, cb)
   })
   .then((sock) => {
     // can be closed, just needed to test the connection
@@ -39,6 +36,41 @@ function _connectAsync (opts) {
   })
 }
 
+/**
+ * Tries to find the starting page (probably blank tab)
+ * among all targets returned by CRI.List call.
+ *
+ * @returns {string} web socket debugger url
+ */
+const findStartPage = (targets) => {
+  debug('CRI List %o', { numTargets: targets.length, targets })
+  // activate the first available id
+  // find the first target page that's a real tab
+  // and not the dev tools or background page.
+  // since we open a blank page first, it has a special url
+  const newTabTargetFields = {
+    type: 'page',
+    url: 'about:blank',
+  }
+
+  const target = _.find(targets, newTabTargetFields)
+
+  la(target, 'could not find CRI target')
+
+  debug('found CRI target %o', target)
+
+  return target.webSocketDebuggerUrl
+}
+
+const findStartPageTarget = (connectOpts) => {
+  debug('CRI.List %o', connectOpts)
+
+  // what happens if the next call throws an error?
+  // it seems to leave the browser instance open
+  // need to clone connectOpts, CRI modifies it
+  return CRI.List(_.clone(connectOpts)).then(findStartPage)
+}
+
 /**
  * Waits for the port to respond with connection to Chrome Remote Interface
  * @param {number} port Port number to connect to
@@ -47,42 +79,45 @@ const getWsTargetFor = (port) => {
   debug('Getting WS connection to CRI on port %d', port)
   la(is.port(port), 'expected port number', port)
 
+  let retryIndex = 0
+
   // force ipv4
   // https://github.com/cypress-io/cypress/issues/5912
   const connectOpts = {
     host: '127.0.0.1',
     port,
+    getDelayMsForRetry: (i) => {
+      retryIndex = i
+
+      return _getDelayMsForRetry(i)
+    },
   }
 
   return _connectAsync(connectOpts)
-  .tapCatch((err) => {
-    debug('failed to connect to CDP %o', { connectOpts, err })
-  })
   .then(() => {
-    debug('CRI.List on port %d', port)
+    const retry = () => {
+      debug('attempting to find CRI target... %o', { retryIndex })
 
-    // what happens if the next call throws an error?
-    // it seems to leave the browser instance open
-    return CRI.List(connectOpts)
-  })
-  .then((targets) => {
-    debug('CRI List %o', { numTargets: targets.length, targets })
-    // activate the first available id
-    // find the first target page that's a real tab
-    // and not the dev tools or background page.
-    // since we open a blank page first, it has a special url
-    const newTabTargetFields = {
-      type: 'page',
-      url: 'about:blank',
-    }
+      return findStartPageTarget(connectOpts)
+      .catch((err) => {
+        retryIndex++
+        const delay = _getDelayMsForRetry(retryIndex)
 
-    const target = _.find(targets, newTabTargetFields)
+        debug('error finding CRI target, maybe retrying %o', { delay, err })
 
-    la(target, 'could not find CRI target')
+        if (typeof delay === 'undefined') {
+          throw err
+        }
 
-    debug('found CRI target %o', target)
+        return Promise.delay(delay)
+        .then(retry)
+      })
+    }
 
-    return target.webSocketDebuggerUrl
+    return retry()
+  })
+  .tapCatch((err) => {
+    debug('failed to connect to CDP %o', { connectOpts, err })
   })
 }
 

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

@@ -1,5 +1,6 @@
 import '../../spec_helper'
 import _ from 'lodash'
+import Bluebird from 'bluebird'
 import 'chai-as-promised' // for the types!
 import chalk from 'chalk'
 import { connect } from '@packages/network'
@@ -12,9 +13,12 @@ import snapshot from 'snap-shot-it'
 import stripAnsi from 'strip-ansi'
 import { stripIndents } from 'common-tags'
 
-describe('lib/browsers/protocol', function () {
-  context('._getDelayMsForRetry', function () {
-    it('retries as expected for up to 20 seconds', function () {
+describe('lib/browsers/protocol', () => {
+  // protocol connects explicitly to this host
+  const host = '127.0.0.1'
+
+  context('._getDelayMsForRetry', () => {
+    it('retries as expected for up to 20 seconds', () => {
       const log = sinon.spy(console, 'log')
 
       let delays = []
@@ -38,8 +42,8 @@ describe('lib/browsers/protocol', function () {
     })
   })
 
-  context('.getWsTargetFor', function () {
-    it('rejects if CDP connection fails', function () {
+  context('.getWsTargetFor', () => {
+    it('rejects if CDP connection fails', () => {
       const innerErr = new Error('cdp connection failure')
 
       sinon.stub(connect, 'createRetryingSocket').callsArgWith(1, innerErr)
@@ -55,12 +59,12 @@ describe('lib/browsers/protocol', function () {
         Error details:
       `
 
-      return expect(p).to.eventually.be.rejected
+      expect(p).to.eventually.be.rejected
       .and.property('message').include(expectedError)
       .and.include(innerErr.message)
     })
 
-    it('returns the debugger URL of the first about:blank tab', async function () {
+    it('returns the debugger URL of the first about:blank tab', async () => {
       const targets = [
         {
           type: 'page',
@@ -76,7 +80,10 @@ describe('lib/browsers/protocol', function () {
 
       const end = sinon.stub()
 
-      sinon.stub(CRI, 'List').withArgs({ host: '127.0.0.1', port: 12345 }).resolves(targets)
+      sinon.stub(CRI, 'List')
+      .withArgs({ host, port: 12345, getDelayMsForRetry: sinon.match.func })
+      .resolves(targets)
+
       sinon.stub(connect, 'createRetryingSocket').callsArgWith(1, null, { end })
 
       const p = protocol.getWsTargetFor(12345)
@@ -85,4 +92,76 @@ describe('lib/browsers/protocol', function () {
       expect(end).to.be.calledOnce
     })
   })
+
+  context('CRI.List', () => {
+    const port = 1234
+    const targets = [
+      {
+        type: 'page',
+        url: 'chrome://newtab',
+        webSocketDebuggerUrl: 'foo',
+      },
+      {
+        type: 'page',
+        url: 'about:blank',
+        webSocketDebuggerUrl: 'ws://debug-url',
+      },
+    ]
+
+    it('retries several times if starting page cannot be found', async () => {
+      const end = sinon.stub()
+
+      sinon.stub(connect, 'createRetryingSocket').callsArgWith(1, null, { end })
+
+      const criList = sinon.stub(CRI, 'List')
+      .withArgs({ host, port, getDelayMsForRetry: sinon.match.func }).resolves(targets)
+      .onFirstCall().resolves([])
+      .onSecondCall().resolves([])
+      .onThirdCall().resolves(targets)
+
+      const targetUrl = await protocol.getWsTargetFor(port)
+
+      expect(criList).to.have.been.calledThrice
+      expect(targetUrl).to.equal('ws://debug-url')
+    })
+
+    it('logs correctly if retries occur while connecting to CDP and while listing CRI targets', async () => {
+      const log = sinon.spy(console, 'log')
+
+      const end = sinon.stub()
+
+      // fail 20 times to get 2 log lines from connect failures
+      sinon.stub(connect, 'createRetryingSocket').callsFake((opts, cb) => {
+        _.times(20, (i) => {
+          opts.getDelayMsForRetry(i, new Error)
+        })
+
+        // @ts-ignore
+        return cb(null, { end })
+      })
+
+      sinon.stub(Bluebird, 'delay').resolves()
+
+      // fail an additional 2 times on CRI.List
+      const criList = sinon.stub(CRI, 'List')
+      .withArgs({ host, port, getDelayMsForRetry: sinon.match.func }).resolves(targets)
+      .onFirstCall().resolves([])
+      .onSecondCall().resolves([])
+      .onThirdCall().resolves(targets)
+
+      const targetUrl = await protocol.getWsTargetFor(port)
+
+      expect(criList).to.have.been.calledThrice
+      expect(targetUrl).to.equal('ws://debug-url')
+
+      // 2 from connect failing, 2 from CRI.List failing
+      expect(log).to.have.callCount(4)
+
+      log.getCalls().forEach((log, i) => {
+        const line = stripAnsi(log.args[0])
+
+        expect(line).to.include(`Failed to connect to Chrome, retrying in 1 second (attempt ${i + 18}/32)`)
+      })
+    })
+  })
 })