fix: proper error handling for snyk test command

package.json

@@ -53,6 +53,7 @@
     "@snyk/dep-graph": "1.12.0",
     "@snyk/gemfile": "1.2.0",
     "@types/agent-base": "^4.2.0",
+    "@types/restify": "^4.3.6",
     "abbrev": "^1.1.1",
     "ansi-escapes": "3.2.0",
     "chalk": "^2.4.2",

src/lib/snyk-test/run-test.ts

@@ -27,6 +27,7 @@ import { maybePrintDeps } from '../print-deps';
 import { SupportedPackageManagers } from '../package-managers';
 import { countPathsToGraphRoot, pruneGraph } from '../prune';
 import { legacyPlugin as pluginApi } from '@snyk/cli-interface';
+import { AuthFailedError } from '../errors/authentication-failed-error';
 
 // tslint:disable-next-line:no-var-requires
 const debug = require('debug')('snyk');
@@ -183,9 +184,15 @@ function handleTestHttpErrorResponse(res, body) {
   let err;
   const userMessage = body && body.userMessage;
   switch (statusCode) {
-    case (statusCode === 500):
+    case 401:
+    case 403:
+      err = AuthFailedError(userMessage, statusCode);
+      err.innerError = body.stack;
+      break;
+    case 500:
       err = new InternalServerError(userMessage);
       err.innerError = body.stack;
+      break;
     default:
       err = new FailedToGetVulnerabilitiesError(userMessage, statusCode);
       err.innerError = body.error;

test/acceptance/cli.acceptance.test.ts

@@ -6,7 +6,7 @@ import * as depGraphLib from '@snyk/dep-graph';
 import * as _ from 'lodash';
 import * as needle from 'needle';
 import * as cli from '../../src/cli/commands';
-import * as fakeServer from './fake-server';
+import {fakeServer} from './fake-server';
 import * as subProcess from '../../src/lib/sub-process';
 import * as version from '../../src/lib/version';
 
@@ -25,13 +25,15 @@ const apiKey = '123456789';
 let oldkey;
 let oldendpoint;
 let versionNumber;
-const server: any = fakeServer(process.env.SNYK_API, apiKey);
+const server = fakeServer(process.env.SNYK_API, apiKey);
 const before = tap.runOnly ? only : test;
 const after = tap.runOnly ? only : test;
 
 // Should be after `process.env` setup.
 import * as plugins from '../../src/lib/plugins';
 import { legacyPlugin as pluginApi } from '@snyk/cli-interface';
+import { AuthFailedError } from '../../src/lib/errors/authentication-failed-error';
+import { InternalServerError } from '../../src/lib/errors';
 
 // @later: remove this config stuff.
 // Was copied straight from ../src/cli-server.js
@@ -140,7 +142,7 @@ test('`test npm-package with custom --project-name`', async (t) => {
   });
   const req = server.popRequest();
   t.match(req.body.projectNameOverride, 'custom-project-name', 'custom project name is passed');
-  t.match(req.targetFile, undefined, 'target is undefined');
+  t.match(req.body.targetFile, undefined, 'target is undefined');
 });
 
 test('`test empty --file=Gemfile`', async (t) => {
@@ -979,7 +981,7 @@ test('`test maven-app --file=pom.xml --dev` sends package info', async (t) => {
   t.equal(req.headers['x-snyk-cli-version'], versionNumber, 'sends version number');
   t.match(req.url, '/test-dep-graph', 'posts to correct url');
   t.equal(req.query.org, 'nobelprize.org', 'org sent as a query in request');
-  t.match(req.targetFile, undefined, 'target is undefined');
+  t.match(req.body.targetFile, undefined, 'target is undefined');
 
   const depGraph = depGraphLib.createFromJSON(req.body.depGraph);
   t.equal(depGraph.rootPkg.name, 'com.mycompany.app:maven-app', 'root name');
@@ -994,7 +996,7 @@ test('`test npm-package` sends pkg info', async (t) => {
   await cli.test('npm-package');
   const req = server.popRequest();
   t.match(req.url, '/test-dep-graph', 'posts to correct url');
-  t.match(req.targetFile, undefined, 'target is undefined');
+  t.match(req.body.targetFile, undefined, 'target is undefined');
   const depGraph = req.body.depGraph;
 
   t.same(
@@ -1008,7 +1010,7 @@ test('`test npm-package --file=package-lock.json ` sends pkg info', async (t) =>
   await cli.test('npm-package', {file: 'package-lock.json'});
   const req = server.popRequest();
   t.match(req.url, '/test-dep-graph', 'posts to correct url');
-  t.match(req.targetFile, undefined, 'target is undefined');
+  t.match(req.body.targetFile, undefined, 'target is undefined');
   const depGraph = req.body.depGraph;
   t.same(
     depGraph.pkgs.map((p) => p.id).sort(),
@@ -1021,7 +1023,7 @@ test('`test npm-package --file=package-lock.json --dev` sends pkg info', async (
   await cli.test('npm-package', {file: 'package-lock.json', dev: true});
   const req = server.popRequest();
   t.match(req.url, '/test-dep-graph', 'posts to correct url');
-  t.match(req.targetFile, undefined, 'target is undefined');
+  t.match(req.body.targetFile, undefined, 'target is undefined');
   const depGraph = req.body.depGraph;
   t.same(
     depGraph.pkgs.map((p) => p.id).sort(),
@@ -1146,7 +1148,7 @@ test('`test yarn-package --file=yarn.lock ` sends pkg info', async (t) => {
   await cli.test('yarn-package', {file: 'yarn.lock'});
   const req = server.popRequest();
   t.match(req.url, '/test-dep-graph', 'posts to correct url');
-  t.match(req.targetFile, undefined, 'target is undefined');
+  t.match(req.body.targetFile, undefined, 'target is undefined');
   const depGraph = req.body.depGraph;
   t.same(
     depGraph.pkgs.map((p) => p.id).sort(),
@@ -1159,7 +1161,7 @@ test('`test yarn-package --file=yarn.lock --dev` sends pkg info', async (t) => {
   await cli.test('yarn-package', {file: 'yarn.lock', dev: true});
   const req = server.popRequest();
   t.match(req.url, '/test-dep-graph', 'posts to correct url');
-  t.match(req.targetFile, undefined, 'target is undefined');
+  t.match(req.body.targetFile, undefined, 'target is undefined');
   const depGraph = req.body.depGraph;
   t.same(
     depGraph.pkgs.map((p) => p.id).sort(),
@@ -1196,7 +1198,7 @@ test('`test` on a yarn package does work and displays appropriate text', async (
   t.equal(req.method, 'POST', 'makes POST request');
   t.equal(req.headers['x-snyk-cli-version'], versionNumber, 'sends version number');
   t.match(req.url, '/test-dep-graph', 'posts to correct url');
-  t.match(req.targetFile, undefined, 'target is undefined');
+  t.match(req.body.targetFile, undefined, 'target is undefined');
   const depGraph = req.body.depGraph;
   t.same(
     depGraph.pkgs.map((p) => p.id).sort(),
@@ -3190,6 +3192,45 @@ function stubExec(t, execOutputFile) {
   });
 }
 
+test('error 401 handling', async (t) => {
+  chdirWorkspaces();
+
+  server.setNextStatusCodeAndResponse(401, {});
+
+  try {
+    await cli.test('ruby-app-thresholds');
+    t.fail('should have thrown');
+  } catch (err) {
+    t.match(err.message, /Authentication failed. Please check the API token on/);
+  }
+});
+
+test('error 403 handling', async (t) => {
+  chdirWorkspaces();
+
+  server.setNextStatusCodeAndResponse(403, {});
+
+  try {
+    await cli.test('ruby-app-thresholds');
+    t.fail('should have thrown');
+  } catch (err) {
+    t.match(err.message, /Authentication failed. Please check the API token on/);
+  }
+});
+
+test('error 500 handling', async (t) => {
+  chdirWorkspaces();
+
+  server.setNextStatusCodeAndResponse(500, {});
+
+  try {
+    await cli.test('ruby-app-thresholds');
+    t.fail('should have thrown');
+  } catch (err) {
+    t.match(err.message, 'Internal server error');
+  }
+});
+
 // @later: try and remove this config stuff
 // Was copied straight from ../src/cli-server.js
 after('teardown', async (t) => {

test/acceptance/fake-server.js → test/acceptance/fake-server.ts

@@ -1,14 +1,22 @@
-var restify = require('restify');
-var fs = require('fs');
+import * as restify from 'restify';
 
-module.exports = function (root, apikey) {
+interface FakeServer extends restify.Server {
+  _reqLog: restify.Request[];
+  _nextResponse?: restify.Response;
+  _nextStatusCode?: number;
+  popRequest: () => restify.Request;
+  setNextResponse: (r: any) => void;
+  setNextStatusCodeAndResponse: (c: number, r: any) => void;
+}
+
+export function fakeServer(root, apikey) {
   var server = restify.createServer({
     name: 'snyk-mock-server',
     version: '1.0.0',
-  });
+  }) as FakeServer;
   server._reqLog = [];
   server.popRequest = function () {
-    return server._reqLog.pop();
+    return server._reqLog.pop()!;
   };
   server.use(restify.acceptParser(server.acceptable));
   server.use(restify.queryParser());
@@ -45,12 +53,18 @@ module.exports = function (root, apikey) {
   });
 
   server.use(function (req, res, next) {
-    if (!server._nextResponse) {
+    if (!server._nextResponse && !server._nextStatusCode) {
       return next();
     }
     var response = server._nextResponse;
     delete server._nextResponse;
-    res.send(response);
+    if (server._nextStatusCode) {
+      const code = server._nextStatusCode;
+      delete server._nextStatusCode;
+      res.send(code, response);
+    } else {
+      res.send(response);
+    }
   });
 
   server.get(root + '/vuln/:registry/:module', function (req, res, next) {
@@ -133,5 +147,10 @@ module.exports = function (root, apikey) {
     server._nextResponse = response;
   };
 
+  server.setNextStatusCodeAndResponse = (code, body) => {
+    server._nextStatusCode = code;
+    server._nextResponse = body;
+  };
+
   return server;
 };

test/acceptance/run-test.test.ts

@@ -1,6 +1,6 @@
 import * as tap from 'tap';
 import {only, test} from 'tap';
-import * as fakeServer from './fake-server';
+import {fakeServer} from './fake-server';
 import * as cli from '../../src/cli/commands';
 
 const port = process.env.PORT = process.env.SNYK_PORT = '12345';
@@ -10,7 +10,7 @@ process.env.LOG_LEVEL = '0';
 const apiKey = '123456789';
 let oldkey;
 let oldendpoint;
-const server: any = fakeServer(process.env.SNYK_API, apiKey);
+const server = fakeServer(process.env.SNYK_API, apiKey);
 
 const before = tap.runOnly ? only : test;
 const after = tap.runOnly ? only : test;

test/monitor-target.test.ts

@@ -1,5 +1,3 @@
-'use strict';
-
 const test = require('tape').test;
 const zlib = require('zlib');
 const requestLib = require('needle');

test/system/remote-package.test.ts

@@ -150,7 +150,7 @@ test('test for non-existing', async (t) => {
   } catch (error) {
     const res = error.message;
     const lastLine = res.trim().split('\n').pop();
-    t.deepEqual(lastLine, 'Failed to get vulns', 'expected error: Failed to get vulns');
+    t.deepEqual(lastLine, 'Internal server error', 'expected error: Internal server error');
   }
 });