throw specific error when cannot parse Cypress arguments (#6280)

* WIP: throw specific error when cannot parse Cypress arguments

* test invalid config string

* add unit test

* rework error message, handle env, reporter options and config

* add args error unit tests

* rework snapshots

* use reporterOptions name

* update snapshot

Author: bahmutov

packages/server/__snapshots__/args_spec.coffee.js

@@ -0,0 +1,23 @@
+exports['invalid env error'] = `
+Cypress encountered an error while parsing the argument env
+
+You passed: nonono
+
+The error was: Cannot read property 'split' of undefined
+`
+
+exports['invalid reporter options error'] = `
+Cypress encountered an error while parsing the argument reporterOptions
+
+You passed: abc
+
+The error was: Cannot read property 'split' of undefined
+`
+
+exports['invalid config error'] = `
+Cypress encountered an error while parsing the argument config
+
+You passed: xyz
+
+The error was: Cannot read property 'split' of undefined
+`

packages/server/__snapshots__/cypress_spec.js

@@ -264,3 +264,27 @@ These flags can only be used when recording to the Cypress Dashboard service.
 
 https://on.cypress.io/record-params-without-recording
 `
+
+exports['could not parse config error'] = `
+Cypress encountered an error while parsing the argument config
+
+You passed: xyz
+
+The error was: Cannot read property 'split' of undefined
+`
+
+exports['could not parse env error'] = `
+Cypress encountered an error while parsing the argument env
+
+You passed: a123
+
+The error was: Cannot read property 'split' of undefined
+`
+
+exports['could not parse reporter options error'] = `
+Cypress encountered an error while parsing the argument reporterOptions
+
+You passed: nonono
+
+The error was: Cannot read property 'split' of undefined
+`

packages/server/lib/cypress.js

@@ -39,6 +39,8 @@ const exitErr = (err) => {
 
   return require('./errors').logException(err)
   .then(() => {
+    debug('calling exit 1')
+
     return exit(1)
   })
 }
@@ -151,7 +153,16 @@ module.exports = {
     // for https://github.com/cypress-io/cypress/issues/5466
     argv = R.without('--', argv)
 
-    const options = argsUtils.toObject(argv)
+    let options
+
+    try {
+      options = argsUtils.toObject(argv)
+    } catch (argumentsError) {
+      debug('could not parse CLI arguments: %o', argv)
+
+      // note - this is promise-returned call
+      return exitErr(argumentsError)
+    }
 
     debug('from argv %o got options %o', argv, options)
 

packages/server/lib/errors.coffee

@@ -62,7 +62,7 @@ trimMultipleNewLines = (str) ->
 isCypressErr = (err) ->
   Boolean(err.isCypressErr)
 
-getMsgByType = (type, arg1 = {}, arg2) ->
+getMsgByType = (type, arg1 = {}, arg2, arg3) ->
   switch type
     when "CANNOT_TRASH_ASSETS"
       """
@@ -525,7 +525,6 @@ getMsgByType = (type, arg1 = {}, arg2) ->
 
         #{chalk.blue(arg2)}
         """
-
     when "RENDERER_CRASHED"
       """
       We detected that the Chromium Renderer process just crashed.
@@ -881,9 +880,17 @@ getMsgByType = (type, arg1 = {}, arg2) ->
       """
       Failed to connect to Chrome, retrying in 1 second (attempt #{chalk.yellow(arg1)}/32)
       """
+    when "COULD_NOT_PARSE_ARGUMENTS"
+        """
+        Cypress encountered an error while parsing the argument #{chalk.gray(arg1)}
+
+        You passed: #{arg2}
+
+        The error was: #{arg3}
+        """
 
-get = (type, arg1, arg2) ->
-  msg = getMsgByType(type, arg1, arg2)
+get = (type, arg1, arg2, arg3) ->
+  msg = getMsgByType(type, arg1, arg2, arg3)
 
   if _.isObject(msg)
     details = msg.details
@@ -904,8 +911,8 @@ warning = (type, arg1, arg2) ->
 
   return null
 
-throwErr = (type, arg1, arg2) ->
-  throw get(type, arg1, arg2)
+throwErr = (type, arg1, arg2, arg3) ->
+  throw get(type, arg1, arg2, arg3)
 
 clone = (err, options = {}) ->
   _.defaults options, {

packages/server/lib/util/args.js

@@ -1,10 +1,13 @@
 const _ = require('lodash')
+const la = require('lazy-ass')
+const is = require('check-more-types')
 const path = require('path')
 const debug = require('debug')('cypress:server:args')
 const minimist = require('minimist')
 const coerceUtil = require('./coerce')
 const configUtil = require('../config')
 const proxyUtil = require('./proxy')
+const errors = require('../errors')
 
 const nestedObjectsInCurlyBracesRe = /\{(.+?)\}/g
 const nestedArraysInSquareBracketsRe = /\[(.+?)\]/g
@@ -107,32 +110,41 @@ const JSONOrCoerce = (str) => {
   return coerceUtil(str)
 }
 
-const sanitizeAndConvertNestedArgs = (str) => {
+const sanitizeAndConvertNestedArgs = (str, argname) => {
+  la(is.unemptyString(argname), 'missing config argname to be parsed')
+
+  try {
   // if this is valid JSON then just
   // parse it and call it a day
-  const parsed = tryJSONParse(str)
+    const parsed = tryJSONParse(str)
 
-  if (parsed) {
-    return parsed
-  }
+    if (parsed) {
+      return parsed
+    }
 
-  // invalid JSON, so assume mixed usage
-  // first find foo={a:b,b:c} and bar=[1,2,3]
-  // syntax and turn those into
-  // foo: a:b|b:c
-  // bar: 1|2|3
-
-  return _
-  .chain(str)
-  .replace(nestedObjectsInCurlyBracesRe, commasToPipes)
-  .replace(nestedArraysInSquareBracketsRe, commasToPipes)
-  .split(',')
-  .map((pair) => {
-    return pair.split(everythingAfterFirstEqualRe)
-  })
-  .fromPairs()
-  .mapValues(JSONOrCoerce)
-  .value()
+    // invalid JSON, so assume mixed usage
+    // first find foo={a:b,b:c} and bar=[1,2,3]
+    // syntax and turn those into
+    // foo: a:b|b:c
+    // bar: 1|2|3
+
+    return _
+    .chain(str)
+    .replace(nestedObjectsInCurlyBracesRe, commasToPipes)
+    .replace(nestedArraysInSquareBracketsRe, commasToPipes)
+    .split(',')
+    .map((pair) => {
+      return pair.split(everythingAfterFirstEqualRe)
+    })
+    .fromPairs()
+    .mapValues(JSONOrCoerce)
+    .value()
+  } catch (err) {
+    debug('could not pass config %s value %s', argname, str)
+    debug('error %o', err)
+
+    return errors.throw('COULD_NOT_PARSE_ARGUMENTS', argname, str, err.message)
+  }
 }
 
 module.exports = {
@@ -219,7 +231,7 @@ module.exports = {
     }
 
     if (env) {
-      options.env = sanitizeAndConvertNestedArgs(env)
+      options.env = sanitizeAndConvertNestedArgs(env, 'env')
     }
 
     const proxySource = proxyUtil.loadSystemProxySettings()
@@ -234,13 +246,13 @@ module.exports = {
     }
 
     if (reporterOptions) {
-      options.reporterOptions = sanitizeAndConvertNestedArgs(reporterOptions)
+      options.reporterOptions = sanitizeAndConvertNestedArgs(reporterOptions, 'reporterOptions')
     }
 
     if (config) {
       // convert config to an object
-      // annd store the config
-      options.config = sanitizeAndConvertNestedArgs(config)
+      // and store the config
+      options.config = sanitizeAndConvertNestedArgs(config, 'config')
     }
 
     // get a list of the available config keys

packages/server/test/integration/cypress_spec.js

@@ -140,14 +140,22 @@ describe('lib/cypress', () => {
       expect(process.exit).to.be.calledWith(code)
     }
 
-    this.expectExitWithErr = (type, msg) => {
-      expect(errors.log).to.be.calledWithMatch({ type })
-      expect(process.exit).to.be.calledWith(1)
-      if (msg) {
-        const err = errors.log.getCall(0).args[0]
+    // returns error object
+    this.expectExitWithErr = (type, msg1, msg2) => {
+      expect(errors.log, 'error was logged').to.be.calledWithMatch({ type })
+      expect(process.exit, 'process.exit was called').to.be.calledWith(1)
 
-        expect(err.message).to.include(msg)
+      const err = errors.log.getCall(0).args[0]
+
+      if (msg1) {
+        expect(err.message, 'error text').to.include(msg1)
+      }
+
+      if (msg2) {
+        expect(err.message, 'second error text').to.include(msg2)
       }
+
+      return err
     }
   })
 
@@ -190,6 +198,35 @@ describe('lib/cypress', () => {
     })
   })
 
+  context('error handling', function () {
+    it('exits if config cannot be parsed', function () {
+      return cypress.start(['--config', 'xyz'])
+      .then(() => {
+        const err = this.expectExitWithErr('COULD_NOT_PARSE_ARGUMENTS')
+
+        snapshot('could not parse config error', stripAnsi(err.message))
+      })
+    })
+
+    it('exits if env cannot be parsed', function () {
+      return cypress.start(['--env', 'a123'])
+      .then(() => {
+        const err = this.expectExitWithErr('COULD_NOT_PARSE_ARGUMENTS')
+
+        snapshot('could not parse env error', stripAnsi(err.message))
+      })
+    })
+
+    it('exits if reporter options cannot be parsed', function () {
+      return cypress.start(['--reporterOptions', 'nonono'])
+      .then(() => {
+        const err = this.expectExitWithErr('COULD_NOT_PARSE_ARGUMENTS')
+
+        snapshot('could not parse reporter options error', stripAnsi(err.message))
+      })
+    })
+  })
+
   context('--get-key', () => {
     it('writes out key and exits on success', function () {
       return Promise.all([

packages/server/test/unit/args_spec.coffee

@@ -3,7 +3,9 @@ require("../spec_helper")
 _        = require("lodash")
 path     = require("path")
 os       = require("os")
-argsUtil = require("#{root}lib/util/args")
+snapshot = require("snap-shot-it")
+stripAnsi = require("strip-ansi")
+argsUtil  = require("#{root}lib/util/args")
 proxyUtil = require("#{root}lib/util/proxy")
 getWindowsProxyUtil = require("#{root}lib/util/get-windows-proxy")
 
@@ -74,6 +76,17 @@ describe "lib/util/args", ->
         bar: "qux="
       })
 
+    it "throws if env string cannot be parsed", ->
+      expect () =>
+        @setup("--env", "nonono")
+      .to.throw
+
+      # now look at the error
+      try
+        @setup("--env", "nonono")
+      catch err
+        snapshot("invalid env error", stripAnsi(err.message))
+
   context "--reporterOptions", ->
     it "converts to object literal", ->
       reporterOpts = {
@@ -112,6 +125,17 @@ describe "lib/util/args", ->
         }
       })
 
+    it "throws if reporter string cannot be parsed", ->
+      expect () =>
+        @setup("--reporterOptions", "abc")
+      .to.throw
+
+      # now look at the error
+      try
+        @setup("--reporterOptions", "abc")
+      catch err
+        snapshot("invalid reporter options error", stripAnsi(err.message))
+
   context "--config", ->
     it "converts to object literal", ->
       options = @setup("--config", "pageLoadTimeout=10000,waitForAnimations=false")
@@ -172,6 +196,17 @@ describe "lib/util/args", ->
       options = @setup("--port", 2222)
       expect(options.config.port).to.eq(2222)
 
+    it "throws if config string cannot be parsed", ->
+      expect () =>
+        @setup("--config", "xyz")
+      .to.throw
+
+      # now look at the error
+      try
+        @setup("--config", "xyz")
+      catch err
+        snapshot("invalid config error", stripAnsi(err.message))
+
   context ".toArray", ->
     beforeEach ->
       @obj = {config: {foo: "bar"}, project: "foo/bar"}