Merge pull request snyk#215 from snyk/feat/exit-code-1-on-monitor-fai…

…lure

exit code 1 on monitor failure, better monitor output readability

cli/commands/monitor.js

@@ -14,6 +14,8 @@ var detect = require('../../lib/detect');
 var plugins = require('../../lib/plugins');
 var ModuleInfo = require('../../lib/module-info');
 
+var SEPARATOR = '\n-------------------------------------------------------\n';
+
 function monitor() {
   var args = [].slice.call(arguments, 0);
   var options = {};
@@ -40,7 +42,7 @@ function monitor() {
           return fs.exists(path).then(function (exists) {
             if (!exists && !options.docker) {
               throw new Error(
-                'snyk monitor should be pointed at an existing project');
+                '"' + path + '" is not a valid path for "snyk monitor"');
             }
 
             var packageManager = detect.detectPackageManager(path, options);
@@ -60,16 +62,16 @@ function monitor() {
             var analysisType = options.docker ? 'docker' : packageManager;
 
             var analyzingDepsSpinnerLabel =
-            'Analyzing ' + analysisType + ' dependencies for ' + displayPath;
+          'Analyzing ' + analysisType + ' dependencies for ' + displayPath;
 
             var postingMonitorSpinnerLabel =
-            'Posting monitor snapshot for ' + displayPath + ' ...';
+          'Posting monitor snapshot for ' + displayPath + ' ...';
 
             return spinner(analyzingDepsSpinnerLabel)
               .then(function () {
                 return moduleInfo.inspect(path, targetFile, options);
               })
-              // clear spinner in case of success or failure
+            // clear spinner in case of success or failure
               .then(spinner.clear(analyzingDepsSpinnerLabel))
               .catch(function (error) {
                 spinner.clear(analyzingDepsSpinnerLabel)();
@@ -90,12 +92,12 @@ function monitor() {
                   packageManager: packageManager,
                   'policy-path': options['policy-path'],
                   'project-name':
-                  options['project-name'] || config['PROJECT_NAME'],
+                options['project-name'] || config['PROJECT_NAME'],
                   isDocker: !!options.docker,
                 };
                 return snyk.monitor(path, meta, info);
               })
-              // clear spinner in case of success or failure
+            // clear spinner in case of success or failure
               .then(spinner.clear(postingMonitorSpinnerLabel))
               .catch(function (error) {
                 spinner.clear(postingMonitorSpinnerLabel)();
@@ -146,29 +148,39 @@ function monitor() {
             throw new Error(json);
           }
 
-          return results.map(function (res) {
+          const output = results.map(function (res) {
             if (res.ok) {
               return res.data;
             }
-            if (res.data && res.data.cliMessage) {
-              return chalk.bold.red(res.data.cliMessage);
-            }
-            return 'For path `' + res.path + '`, ' + res.data.message;
-          }).join('\n');
+
+            var errorMessage = (res.data && res.data.cliMessage) ?
+              chalk.bold.red(res.data.cliMessage) :
+              (res.data ? res.data.message : 'Unknown error occurred.');
+
+            return chalk.bold.white('\nMonitoring ' + res.path + '...\n\n') +
+              errorMessage;
+          }).join('\n' + SEPARATOR);
+
+          if (results.every(function (res) {
+            return res.ok;
+          })) {
+            return output;
+          }
+
+          throw new Error(output);
         });
     });
 }
 
 function formatMonitorOutput(packageManager, res, manageUrl, isJson) {
   var issues = res.licensesPolicy ? 'issues' : 'vulnerabilities';
-  var strOutput = (packageManager === 'yarn' ?
-    'A yarn.lock file was detected - continuing as a Yarn project.\n\n' : '') +
-    '\n\nProject path: ' + res.path +
-    '\nCaptured a snapshot of this project\'s dependencies.\n' +
-    'Explore this snapshot at ' + res.uri + '\n\n' +
+  var strOutput = chalk.bold.white('\nMonitoring ' + res.path + '...\n\n') +
+    (packageManager === 'yarn' ?
+      'A yarn.lock file was detected - continuing as a Yarn project.\n' : '') +
+      'Explore this snapshot at ' + res.uri + '\n\n' +
     (res.isMonitored ?
-      'Notifications about newly disclosed ' + issues + ' related\n' +
-      'to these dependencies will be emailed to you.' :
+      'Notifications about newly disclosed ' + issues + ' related ' +
+      'to these dependencies will be emailed to you.\n' :
       chalk.bold.red('Project is inactive, so notifications are turned ' +
         'off.\nActivate this project here: ' + manageUrl + '\n\n')) +
     (res.trialStarted ?

lib/spinner.js

@@ -37,7 +37,8 @@ createSpinner.sticky = function (s) {
 createSpinner.clear = function (label) {
   return function (res) {
     if (spinners[label] === undefined) {
-      throw new Error('unknown spinner label: ' + label);
+      // clearing a non-existend spinner is ok by default
+      return res;
     }
 
     debug('clearing %s (%s)', label, spinners[label].length);

test/acceptance/cli.acceptance.test.js

@@ -1345,14 +1345,24 @@ test('`monitor non-existing --json`', function (t) {
   .catch(function (error) {
     var errObj = JSON.parse(error.message);
     t.notOk(errObj.ok, 'ok object should be false');
-    t.match(errObj.error,
-      'snyk monitor should be pointed at an existing project',
-      'show err message');
+    t.match(errObj.error, 'is not a valid path', 'show err message');
     t.match(errObj.path, 'non-existing', 'should show specified path');
     t.pass('throws error');
   });
 });
 
+test('`monitor non-existing`', function (t) {
+  chdirWorkspaces();
+  return cli.monitor('non-existing', {json: false})
+  .then(function () {
+    t.fail('should have failed');
+  })
+  .catch(function (error) {
+    t.match(error.message, 'is not a valid path', 'show err message');
+    t.pass('throws error');
+  });
+});
+
 test('`monitor npm-package`', function (t) {
   chdirWorkspaces();
   return cli.monitor('npm-package')