From 49c7fb9ac5e7c2ec17580b284a80aef7fef988f2 Mon Sep 17 00:00:00 2001 From: Liliana Kastilio Date: Mon, 13 May 2019 09:20:22 +0100 Subject: [PATCH] feat: improve error handling when no files detected --- src/cli/args.ts | 2 +- src/cli/index.ts | 10 +-- src/lib/detect.ts | 10 +-- src/lib/errors/custom-error.ts | 15 ++++ src/lib/errors/index.ts | 1 + src/lib/{error.js => errors/legacy-errors.js} | 19 +++-- .../errors/no-supported-manifests-found.ts | 18 ++++ src/lib/protect/patch.js | 2 +- src/lib/protect/update.js | 2 +- src/lib/sln/{index.js => index.ts} | 82 +++++++++---------- src/lib/snyk-test/run-test.ts | 7 +- test/acceptance/sln-app.test.js | 17 +++- .../sln-no-supported-files/mySolution.sln | 8 ++ test/cli.test.js | 4 +- 14 files changed, 125 insertions(+), 72 deletions(-) create mode 100644 src/lib/errors/custom-error.ts create mode 100644 src/lib/errors/index.ts rename src/lib/{error.js => errors/legacy-errors.js} (91%) create mode 100644 src/lib/errors/no-supported-manifests-found.ts rename src/lib/sln/{index.js => index.ts} (60%) create mode 100644 test/acceptance/workspaces/sln-no-supported-files/mySolution.sln diff --git a/src/cli/args.ts b/src/cli/args.ts index ac5c2de8c0..b2b8aef57f 100644 --- a/src/cli/args.ts +++ b/src/cli/args.ts @@ -105,7 +105,7 @@ export function args(processargv) { if (!method) { // if we failed to find a command, then default to an error - method = require('../lib/error'); + method = require('../lib/errors/legacy-errors'); argv._.push(command); } diff --git a/src/cli/index.ts b/src/cli/index.ts index b3f498ee14..fdbaf68938 100755 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -10,7 +10,7 @@ import * as sln from '../lib/sln'; import {args as argsLib} from './args'; import {copy} from './copy'; import spinner = require('../lib/spinner'); -import errors = require('../lib/error'); +import errors = require('../lib/errors/legacy-errors'); import ansiEscapes = require('ansi-escapes'); import {isPathToPackageFile} from '../lib/detect'; import {updateCheck} from '../lib/updater'; @@ -113,15 +113,13 @@ async function main() { checkRuntime(); const args = argsLib(process.argv); - - if (args.options.file && args.options.file.match(/\.sln$/)) { - sln.updateArgs(args); - } - let res = null; let failed = false; try { + if (args.options.file && args.options.file.match(/\.sln$/)) { + sln.updateArgs(args); + } checkPaths(args); res = await runCommand(args); } catch (error) { diff --git a/src/lib/detect.ts b/src/lib/detect.ts index e90046ad96..f89d9cfd21 100644 --- a/src/lib/detect.ts +++ b/src/lib/detect.ts @@ -1,8 +1,8 @@ import * as fs from 'then-fs'; import * as pathLib from 'path'; import * as debugLib from 'debug'; -import chalk from 'chalk'; import * as _ from 'lodash'; +import { NoSupportedManifestsFoundError } from './errors'; const debug = debugLib('snyk'); @@ -93,13 +93,7 @@ export function detectPackageManager(root, options) { packageManager = detectPackageManagerFromRegistry(registry); } if (!packageManager) { - throw new Error('Could not detect supported target files in ' + root + - '.\nPlease see our documentation for supported languages and ' + - 'target files: ' + - chalk.underline( - 'https://support.snyk.io/hc/en-us/articles/360000911957-Language-support', - ) + - ' and make sure you are in the right directory.'); + throw NoSupportedManifestsFoundError([root]); } return packageManager; } diff --git a/src/lib/errors/custom-error.ts b/src/lib/errors/custom-error.ts new file mode 100644 index 0000000000..5700289390 --- /dev/null +++ b/src/lib/errors/custom-error.ts @@ -0,0 +1,15 @@ +export class CustomError extends Error { + + public innerError; + public code; + public userMessage; + + constructor(message: string) { + super(message); + Error.captureStackTrace(this, this.constructor); + this.name = this.constructor.name; + this.code = undefined; + this.innerError = undefined; + this.userMessage = undefined; + } +} diff --git a/src/lib/errors/index.ts b/src/lib/errors/index.ts new file mode 100644 index 0000000000..18571426b0 --- /dev/null +++ b/src/lib/errors/index.ts @@ -0,0 +1 @@ +export {NoSupportedManifestsFoundError} from './no-supported-manifests-found'; diff --git a/src/lib/error.js b/src/lib/errors/legacy-errors.js similarity index 91% rename from src/lib/error.js rename to src/lib/errors/legacy-errors.js index ae9ca0f378..ce151105e5 100644 --- a/src/lib/error.js +++ b/src/lib/errors/legacy-errors.js @@ -1,8 +1,9 @@ -var config = require('../lib/config'); -var chalk = require('chalk'); -var SEVERITIES = require('./snyk-test/common').SEVERITIES; -var errors = { +const config = require('../config'); +const chalk = require('chalk'); +const {SEVERITIES} = require('../snyk-test/common'); + +const errors = { connect: 'Check your network connection, failed to connect to Snyk API', endpoint: 'The Snyk API is not available on ' + config.API, auth: 'Unauthorized: please ensure you are logged in using `snyk auth`', @@ -37,12 +38,12 @@ var errors = { idRequired: 'id is a required field for `snyk ignore`', unknownCommand: '%s\n\nRun `snyk --help` for a list of available commands.', invalidSeverityThreshold: 'Invalid severity threshold, please use one of ' + - SEVERITIES.map(s => s.verboseName).join(' | '), + SEVERITIES.map((s) => s.verboseName).join(' | '), }; // a key/value pair of error.code (or error.message) as the key, and our nice // strings as the value. -var codes = { +const codes = { ECONNREFUSED: errors.connect, ENOTFOUND: errors.connect, NOT_FOUND: errors.notfound, @@ -66,13 +67,13 @@ var codes = { }; module.exports = function error(command) { - var e = new Error('Unknown command "' + command + '"'); + const e = new Error('Unknown command "' + command + '"'); e.code = 'UNKNOWN_COMMAND'; return Promise.reject(e); }; -module.exports.message = function (error) { - var message = error; // defaults to a string (which is super unlikely) +module.exports.message = function(error) { + let message = error; // defaults to a string (which is super unlikely) if (error instanceof Error) { if (error.code === 'VULNS') { return error.message; diff --git a/src/lib/errors/no-supported-manifests-found.ts b/src/lib/errors/no-supported-manifests-found.ts new file mode 100644 index 0000000000..3b9e0fa7ee --- /dev/null +++ b/src/lib/errors/no-supported-manifests-found.ts @@ -0,0 +1,18 @@ +import chalk from 'chalk'; +import {CustomError} from './custom-error'; + +export function NoSupportedManifestsFoundError(atLocations: string[]) { + const locationsStr = atLocations.join(', '); + const errorMsg = 'Could not detect supported target files in ' + locationsStr + + '.\nPlease see our documentation for supported languages and ' + + 'target files: ' + + chalk.underline( + 'https://support.snyk.io/hc/en-us/articles/360000911957-Language-support', + ) + + ' and make sure you are in the right directory.'; + + const error = new CustomError(errorMsg); + error.code = 422; + error.userMessage = errorMsg; + return error; +} diff --git a/src/lib/protect/patch.js b/src/lib/protect/patch.js index bcfe550cb8..f75fd6775a 100644 --- a/src/lib/protect/patch.js +++ b/src/lib/protect/patch.js @@ -15,7 +15,7 @@ var getVulnSource = require('./get-vuln-source'); var dedupe = require('./dedupe-patches'); var writePatchFlag = require('./write-patch-flag'); var spinner = require('../spinner'); -var errors = require('../error'); +var errors = require('../errors/legacy-errors'); var analytics = require('../analytics'); var getPatchFile = require('./fetch-patch'); diff --git a/src/lib/protect/update.js b/src/lib/protect/update.js index 6cd61e7857..6868e3bf9a 100644 --- a/src/lib/protect/update.js +++ b/src/lib/protect/update.js @@ -7,7 +7,7 @@ var chalk = require('chalk'); var _ = require('lodash'); var moduleToObject = require('snyk-module'); var semver = require('semver'); -var errors = require('../error'); +var errors = require('../errors/legacy-errors'); var npm = require('../npm'); var yarn = require('../yarn'); var spinner = require('../spinner'); diff --git a/src/lib/sln/index.js b/src/lib/sln/index.ts similarity index 60% rename from src/lib/sln/index.js rename to src/lib/sln/index.ts index 464242fb05..b5c96b28ce 100644 --- a/src/lib/sln/index.js +++ b/src/lib/sln/index.ts @@ -1,56 +1,29 @@ -var fs = require('fs'); -var path = require('path'); -var debug = require('debug')('snyk'); -var detect = require('../detect'); +import * as fs from 'fs'; +import * as path from 'path'; +import * as detect from '../detect'; +import {NoSupportedManifestsFoundError} from '../errors/no-supported-manifests-found'; +import * as Debug from 'debug'; -var sln = {}; - -sln.updateArgs = function (args) { - // save the path if --file=path/file.sln - var slnFilePath = path.dirname(args.options.file); - - // extract all referenced projects from solution - // keep only those that contain relevant manifest files - var projectFolders = sln.parsePathsFromSln(args.options.file) - .map(function (projectPath) { - var projectFolder = - path.resolve(slnFilePath, path.dirname(projectPath)); - var manifestFile = detect.detectPackageFile(projectFolder); - return manifestFile ? projectFolder : undefined; - }) - .filter(Boolean); - - debug('valid project folders in solution: ', projectFolders); - - if (projectFolders.length === 0) { - throw new Error('No relevant projects found in Solution'); - } - - // delete the file option as the solution has now been parsed - delete (args.options.file); - - // mutates args! - addProjectFoldersToArgs(args, projectFolders); -}; +const debug = Debug('snyk'); // slnFile should exist. // returns array of project paths (path/to/manifest.file) -sln.parsePathsFromSln = function (slnFile) { +export const parsePathsFromSln = (slnFile) => { // read project scopes from solution file // [\s\S] is like ., but with newlines! // *? means grab the shortest match - var projectScopes = + const projectScopes = loadFile(path.resolve(slnFile)).match(/Project[\s\S]*?EndProject/g) || []; - var paths = projectScopes.map(function (projectScope) { - var secondArg = projectScope.split(',')[1]; + const paths = projectScopes.map((projectScope) => { + const secondArg = projectScope.split(',')[1]; // expected ` "path/to/manifest.file"`, clean it up return secondArg && secondArg.trim().replace(/\"/g, ''); }) // drop falsey values .filter(Boolean) // convert path separators - .map(function (projectPath) { + .map((projectPath) => { return projectPath.replace(/\\/g, path.sep); }); @@ -58,9 +31,38 @@ sln.parsePathsFromSln = function (slnFile) { return paths; }; +export const updateArgs = (args) => { + // save the path if --file=path/file.sln + const slnFilePath = path.dirname(args.options.file); + + // extract all referenced projects from solution + // keep only those that contain relevant manifest files + const projectFolders = parsePathsFromSln(args.options.file); + + const foldersWithSupportedProjects = projectFolders + .map((projectPath) => { + const projectFolder = + path.resolve(slnFilePath, path.dirname(projectPath)); + const manifestFile = detect.detectPackageFile(projectFolder); + return manifestFile ? projectFolder : undefined; }) + .filter(Boolean); + + debug('valid project folders in solution: ', projectFolders); + + if (foldersWithSupportedProjects.length === 0) { + throw NoSupportedManifestsFoundError([...projectFolders]); + } + + // delete the file option as the solution has now been parsed + delete (args.options.file); + + // mutates args! + addProjectFoldersToArgs(args, foldersWithSupportedProjects); +}; + function addProjectFoldersToArgs(args, projectFolders) { // keep the last arg (options) aside for later use - var lastArg = args.options._.pop(); + const lastArg = args.options._.pop(); // add relevant project paths as if they were given as a runtime path args args.options._ = args.options._.concat(projectFolders); // bring back the last (options) arg @@ -74,5 +76,3 @@ function loadFile(filePath) { } return fs.readFileSync(filePath, 'utf8'); } - -module.exports = sln; diff --git a/src/lib/snyk-test/run-test.ts b/src/lib/snyk-test/run-test.ts index 9258ae290d..851d5b77cd 100644 --- a/src/lib/snyk-test/run-test.ts +++ b/src/lib/snyk-test/run-test.ts @@ -18,6 +18,7 @@ import {DepTree} from '../types'; import gemfileLockToDependencies = require('../../lib/plugins/rubygems/gemfile-lock-to-dependencies'); import {convertTestDepGraphResultToLegacy, AnnotatedIssue, LegacyVulnApiResult, TestDepGraphResponse} from './legacy'; import {SingleDepRootResult, MultiDepRootsResult, isMultiResult, TestOptions} from '../types'; +import { NoSupportedManifestsFoundError } from '../errors'; // tslint:disable-next-line:no-var-requires const debug = require('debug')('snyk'); @@ -191,6 +192,9 @@ function assemblePayloads(root: string, options): Promise { // Force getDepsFromPlugin to return depRoots for processing in assembleLocalPayload async function getDepsFromPlugin(root, options: TestOptions): Promise { options.file = options.file || detect.detectPackageFile(root); + if (!options.docker && !(options.file || options.packageManager)) { + throw NoSupportedManifestsFoundError([...root]); + } const plugin = plugins.loadPlugin(options.packageManager, options); const moduleInfo = ModuleInfo(plugin, options.policy); const pluginOptions = plugins.getPluginOptions(options.packageManager, options); @@ -324,7 +328,8 @@ async function assembleLocalPayloads(root, options): Promise { }; if (['yarn', 'npm'].indexOf(options.packageManager) !== -1) { - const isLockFileBased = options.file.endsWith('package-lock.json') || options.file.endsWith('yarn.lock'); + const isLockFileBased = options.file + && options.file.endsWith('package-lock.json') || options.file.endsWith('yarn.lock'); if (!isLockFileBased || options.traverseNodeModules) { payload.modules = pkg as DepTreeFromResolveDeps; // See the output of resolve-deps } diff --git a/test/acceptance/sln-app.test.js b/test/acceptance/sln-app.test.js index 4d21a1f45a..4bd90596d1 100644 --- a/test/acceptance/sln-app.test.js +++ b/test/acceptance/sln-app.test.js @@ -3,7 +3,7 @@ var path = require('path'); var sln = require('../../src/lib/sln'); test('parseFoldersFromSln when passed an existant filename', function (t) { - var slnFile = 'test/acceptance/workspaces/sln-example-app/mySolution.sln'; + var slnFile = 'test/acceptance/workspaces/sln-example-app/mySolution.sln'; var expected = JSON.stringify([ "dotnet2_new_mvc_project/new_mvc_project.csproj", "WebApplication2/WebApplication2.csproj" @@ -15,7 +15,7 @@ test('parseFoldersFromSln when passed an existant filename', function (t) { test('parseFoldersFromSln when non existant filename', function (t) { var response; - var slnFile = 'test/acceptance/workspaces/sln-example-app/mySolution1.sln'; + var slnFile = 'test/acceptance/workspaces/sln-example-app/mySolution1.sln'; try { response = sln.parsePathsFromSln(slnFile); t.fail('an exception should be thrown'); @@ -27,6 +27,19 @@ test('parseFoldersFromSln when non existant filename', function (t) { t.end(); }); +test('parseFoldersFromSln when no supported files found', function (t) { + let response; + const slnFile = 'test/acceptance/workspaces/sln-no-supported-files/mySolution1.sln'; + try { + response = sln.parsePathsFromSln(slnFile); + t.fail('an exception should be thrown'); + } catch (e) { + t.match(e.message, 'File not found: ', 'should throw exception'); + t.equal(response, undefined, 'shouldnt return'); + } + t.end(); +}); + test('sln.updateArgs for existing sln with regular paths', function (t) { var args = {options: { file: 'test/acceptance/workspaces/sln-example-app/mySolution.sln', _: []}}; diff --git a/test/acceptance/workspaces/sln-no-supported-files/mySolution.sln b/test/acceptance/workspaces/sln-no-supported-files/mySolution.sln new file mode 100644 index 0000000000..0e9745aec7 --- /dev/null +++ b/test/acceptance/workspaces/sln-no-supported-files/mySolution.sln @@ -0,0 +1,8 @@ +Microsoft Visual Studio Solution File, Format Version 12.00 +# Visual Studio 2012 +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "new_mvc_project", "dotnet2_new_mvc_project\new_mvc_project.csproj", "{26B9B59B-C5AC-49CE-9BD6-4C72940DAC89}" +EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "WebApplication2", "WebApplication2\WebApplication2.csproj", "{9A79CF26-C7E8-47A3-B718-C5E654AA2701}" +EndProject +Global +EndGlobal diff --git a/test/cli.test.js b/test/cli.test.js index 0f07b8f8c5..e0611be3ca 100644 --- a/test/cli.test.js +++ b/test/cli.test.js @@ -207,7 +207,7 @@ test('snyk ignore - no ID', function (t) { }).then(function (res) { t.fail('should not succeed with missing ID'); }).catch(function (e) { - var errors = require('../src/lib/error'); + var errors = require('../src/lib/errors/legacy-errors'); var message = stripAnsi(errors.message(e)); t.equal(message.toLowerCase().indexOf('id is a required field'), 0, 'captured failed ignore (no --id given)'); @@ -274,7 +274,7 @@ test('auth via key', function (t) { test('auth via invalid key', function (t) { t.plan(1); - var errors = require('../src/lib/error'); + var errors = require('../src/lib/errors/legacy-errors'); cli.auth('_____________').then(function (res) { t.fail('auth should not succeed: ' + res);