Automatically retry verify and run commands on Linux if suspect DISPLAY problem (#4165)

* cli: debug explanation for XVFB

* linting

* add chai-as-promised to CLI dev

* show Linux specific error solution if cannot verify

* add todo

* chore: consolidate github issue url logic

* linting

* add npm script lint-changed to quickly eslint fix changes JS files

* retry verify with our XVFB

* update errors and tests

* update CLI tests

* add test for display error message

* fix unit test

* add successful test with retry

* finish verify retry test

* warn users if hit display problem on first verify

* try to detect display problem when running electron and retry with our xvfb

* add warning message to spawn when attempting xvfb re-run

* add test for display retry behavior on spawn

* more comments for clarity

* fix typo

cli/lib/exec/spawn.js

@@ -4,10 +4,13 @@ const cp = require('child_process')
 const path = require('path')
 const Promise = require('bluebird')
 const debug = require('debug')('cypress:cli')
+const { stripIndent } = require('common-tags')
 
 const util = require('../util')
 const state = require('../tasks/state')
 const xvfb = require('./xvfb')
+const logger = require('../logger')
+const logSymbols = require('log-symbols')
 const { throwFormErrorText, errors } = require('../errors')
 
 const isXlibOrLibudevRe = /^(?:Xlib|libudev)/
@@ -52,7 +55,7 @@ module.exports = {
       executable = path.resolve(util.getEnv('CYPRESS_RUN_BINARY'))
     }
 
-    debug('needs XVFB?', needsXvfb)
+    debug('needs to start own XVFB?', needsXvfb)
 
     // always push cwd into the args
     args = [].concat(args, '--cwd', process.cwd())
@@ -69,7 +72,9 @@ module.exports = {
           // if we're in dev then reset
           // the launch cmd to be 'npm run dev'
           executable = 'node'
-          args.unshift(path.resolve(__dirname, '..', '..', '..', 'scripts', 'start.js'))
+          args.unshift(
+            path.resolve(__dirname, '..', '..', '..', 'scripts', 'start.js')
+          )
         }
 
         const overrides = util.getEnvOverrides()
@@ -91,6 +96,12 @@ module.exports = {
           options = _.extend({}, options, { windowsHide: false })
         }
 
+        if (os.platform() === 'linux' && process.env.DISPLAY) {
+          // make sure we use the latest DISPLAY variable if any
+          debug('passing DISPLAY', process.env.DISPLAY)
+          options.env.DISPLAY = process.env.DISPLAY
+        }
+
         const child = cp.spawn(executable, args, options)
 
         child.on('close', resolve)
@@ -101,17 +112,21 @@ module.exports = {
 
         // if this is defined then we are manually piping for linux
         // to filter out the garbage
-        child.stderr && child.stderr.on('data', (data) => {
-          const str = data.toString()
+        child.stderr &&
+          child.stderr.on('data', (data) => {
+            const str = data.toString()
 
-          // bail if this is warning line garbage
-          if (isXlibOrLibudevRe.test(str) || isHighSierraWarningRe.test(str)) {
-            return
-          }
+            // bail if this is warning line garbage
+            if (
+              isXlibOrLibudevRe.test(str) ||
+              isHighSierraWarningRe.test(str)
+            ) {
+              return
+            }
 
-          // else pass it along!
-          process.stderr.write(data)
-        })
+            // else pass it along!
+            process.stderr.write(data)
+          })
 
         // https://github.com/cypress-io/cypress/issues/1841
         // In some versions of node, it will throw on windows
@@ -133,18 +148,61 @@ module.exports = {
       })
     }
 
-    const userFriendlySpawn = () => {
+    const spawnInXvfb = () => {
+      return xvfb
+      .start()
+      .then(() => {
+        // call userFriendlySpawn ourselves
+        // to prevent result of previous promise
+        // from becoming a parameter to userFriendlySpawn
+        debug('spawning Cypress after starting XVFB')
+
+        return userFriendlySpawn()
+      })
+      .finally(xvfb.stop)
+    }
+
+    const userFriendlySpawn = (shouldRetryOnDisplayProblem) => {
+      debug('spawning, should retry on display problem?', Boolean(shouldRetryOnDisplayProblem))
+      if (os.platform() === 'linux') {
+        debug('DISPLAY is %s', process.env.DISPLAY)
+      }
+
       return spawn()
+      .then((code) => {
+        const POTENTIAL_DISPLAY_PROBLEM_EXIT_CODE = 234
+
+        if (shouldRetryOnDisplayProblem && code === POTENTIAL_DISPLAY_PROBLEM_EXIT_CODE) {
+          debug('Cypress thinks there is a potential display or OS problem')
+          debug('retrying the command with our XVFB')
+
+          // if we get this error, we are on Linux and DISPLAY is set
+          logger.warn(`${stripIndent`
+
+            ${logSymbols.warning} Warning: Cypress process has finished very quickly with an error,
+            which might be related to a potential problem with how the DISPLAY is configured.
+
+            DISPLAY was set to "${process.env.DISPLAY}"
+
+            We will attempt to spin our XVFB server and run Cypress again.
+          `}\n`)
+
+          return spawnInXvfb()
+        }
+
+        return code
+      })
       .catch(throwFormErrorText(errors.unexpected))
     }
 
     if (needsXvfb) {
-      return xvfb.start()
-      .then(userFriendlySpawn)
-      .finally(xvfb.stop)
+      return spawnInXvfb()
     }
 
-    return userFriendlySpawn()
+    // if we have problems spawning Cypress, maybe user DISPLAY setting is incorrect
+    // in that case retry with our own XVFB
+    const shouldRetryOnDisplayProblem = os.platform() === 'linux'
 
+    return userFriendlySpawn(shouldRetryOnDisplayProblem)
   },
 }

cli/lib/exec/xvfb.js

@@ -2,9 +2,11 @@ const os = require('os')
 const Promise = require('bluebird')
 const Xvfb = require('@cypress/xvfb')
 const R = require('ramda')
+const { stripIndent } = require('common-tags')
 const debug = require('debug')('cypress:cli')
 const debugXvfb = require('debug')('cypress:xvfb')
 const { throwFormErrorText, errors } = require('../errors')
+const util = require('../util')
 
 const xvfb = Promise.promisifyAll(new Xvfb({
   timeout: 5000, // milliseconds
@@ -41,7 +43,33 @@ module.exports = {
   },
 
   isNeeded () {
-    return os.platform() === 'linux' && !process.env.DISPLAY
+    if (os.platform() !== 'linux') {
+      return false
+    }
+
+    if (process.env.DISPLAY) {
+      const issueUrl = util.getGitHubIssueUrl(4034)
+
+      const message = stripIndent`
+        DISPLAY environment variable is set to ${process.env.DISPLAY} on Linux
+        Assuming this DISPLAY points at working X11 server,
+        Cypress will not spawn own XVFB
+
+        NOTE: if the X11 server is NOT working, Cypress will exit without explanation,
+          see ${issueUrl}
+        Solution: Unset the DISPLAY variable and try again:
+          DISPLAY= npx cypress run ...
+      `
+
+      debug(message)
+
+      return false
+    }
+
+    debug('undefined DISPLAY environment variable')
+    debug('Cypress will spawn its own XVFB')
+
+    return true
   },
 
   // async method, resolved with Boolean