Skip to content

Commit

Permalink
fix: propagate failed monitor scans all the way to the user
Browse files Browse the repository at this point in the history
- failed monitor paths generate an error but not failed
attempts to get dependencies, these were skipped and shown
only with -d output. Propagate full status back to the user.

- this may affect the expected exit code for some users where
previous failures were skipped in this manner

- log failed snyk test scans with --all-projects
  • Loading branch information
lili2311 committed Aug 3, 2020
1 parent f90157d commit 7ef59ed
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 21 deletions.
5 changes: 4 additions & 1 deletion .gitignore
Expand Up @@ -19,4 +19,7 @@ cert.pem
key.pem

# Diagnostic reports (https://nodejs.org/api/report.html)
report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json
report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json
# test fixture build artifacts
/test/acceptance/workspaces/**/project/
/test/acceptance/workspaces/**/target/
23 changes: 17 additions & 6 deletions src/cli/commands/monitor/index.ts
Expand Up @@ -42,7 +42,7 @@ import {
GitRepoCommitStats,
execShell,
} from '../../../lib/monitor/dev-count-analysis';
import { FailedToRunTestError } from '../../../lib/errors';
import { FailedToRunTestError, MonitorError } from '../../../lib/errors';

const SEPARATOR = '\n-------------------------------------------------------\n';
const debug = Debug('snyk');
Expand Down Expand Up @@ -166,13 +166,8 @@ async function monitor(...args0: MethodArgs): Promise<any> {
}),
spinner.clear(analyzingDepsSpinnerLabel),
);

analytics.add('pluginName', inspectResult.plugin.name);

const postingMonitorSpinnerLabel =
'Posting monitor snapshot for ' + displayPath + ' ...';
await spinner(postingMonitorSpinnerLabel);

// We send results from "all-sub-projects" scanning as different Monitor objects
// multi result will become default, so start migrating code to always work with it
let perProjectResult: MultiProjectResultCustom;
Expand All @@ -183,6 +178,22 @@ async function monitor(...args0: MethodArgs): Promise<any> {
perProjectResult = convertMultiResultToMultiCustom(inspectResult);
}

const failedResults = (inspectResult as MultiProjectResultCustom)
.failedResults;
if (failedResults?.length) {
failedResults.forEach((result) => {
results.push({
ok: false,
data: new MonitorError(500, result.errMessage),
path: result.targetFile || '',
});
});
}

const postingMonitorSpinnerLabel =
'Posting monitor snapshot for ' + displayPath + ' ...';
await spinner(postingMonitorSpinnerLabel);

// Post the project dependencies to the Registry
for (const projectDeps of perProjectResult.scannedProjects) {
try {
Expand Down
2 changes: 1 addition & 1 deletion src/cli/commands/test/index.ts
Expand Up @@ -110,7 +110,7 @@ async function test(...args: MethodArgs): Promise<TestCommandResult> {
testOpts.path = path;
testOpts.projectName = testOpts['project-name'];

let res;
let res: (TestResult | TestResult[]) | Error;

try {
res = await snyk.test(path, testOpts);
Expand Down
7 changes: 5 additions & 2 deletions src/lib/plugins/get-deps-from-plugin.ts
Expand Up @@ -3,7 +3,10 @@ import { legacyPlugin as pluginApi } from '@snyk/cli-interface';
import { find } from '../find-files';
import { Options, TestOptions, MonitorOptions } from '../types';
import { NoSupportedManifestsFoundError } from '../errors';
import { getMultiPluginResult } from './get-multi-plugin-result';
import {
getMultiPluginResult,
MultiProjectResultCustom,
} from './get-multi-plugin-result';
import { getSinglePluginResult } from './get-single-plugin-result';
import {
detectPackageFile,
Expand Down Expand Up @@ -32,7 +35,7 @@ const multiProjectProcessors = {
export async function getDepsFromPlugin(
root: string,
options: Options & (TestOptions | MonitorOptions),
): Promise<pluginApi.MultiProjectResult> {
): Promise<pluginApi.MultiProjectResult | MultiProjectResultCustom> {
let inspectRes: pluginApi.InspectResult;

if (Object.keys(multiProjectProcessors).some((key) => options[key])) {
Expand Down
29 changes: 28 additions & 1 deletion src/lib/plugins/get-multi-plugin-result.ts
Expand Up @@ -12,6 +12,7 @@ import { convertSingleResultToMultiCustom } from './convert-single-splugin-res-t
import { convertMultiResultToMultiCustom } from './convert-multi-plugin-res-to-multi-custom';
import { PluginMetadata } from '@snyk/cli-interface/legacy/plugin';
import { CallGraph } from '@snyk/cli-interface/legacy/common';
import { FailedToRunTestError } from '../errors';

const debug = debugModule('snyk-test');
export interface ScannedProjectCustom
Expand All @@ -20,9 +21,16 @@ export interface ScannedProjectCustom
plugin: PluginMetadata;
callGraph?: CallGraph;
}

interface FailedProjectScanError {
targetFile?: string;
error?: Error;
errMessage: string;
}
export interface MultiProjectResultCustom
extends cliInterface.legacyPlugin.MultiProjectResult {
scannedProjects: ScannedProjectCustom[];
failedResults?: FailedProjectScanError[];
}

export async function getMultiPluginResult(
Expand All @@ -31,6 +39,8 @@ export async function getMultiPluginResult(
targetFiles: string[],
): Promise<MultiProjectResultCustom> {
const allResults: ScannedProjectCustom[] = [];
const failedResults: FailedProjectScanError[] = [];

for (const targetFile of targetFiles) {
const optionsClone = _.cloneDeep(options);
optionsClone.file = path.relative(root, targetFile);
Expand Down Expand Up @@ -68,14 +78,31 @@ export async function getMultiPluginResult(

allResults.push(...pluginResultWithCustomScannedProjects.scannedProjects);
} catch (err) {
debug(chalk.bold.red(err.message));
// TODO: propagate this all the way back and include in --json output
failedResults.push({
targetFile,
error: err,
errMessage: err.message || 'Something went wrong getting dependencies',
});
debug(
chalk.bold.red(
`\n✗ Failed to get dependencies for ${targetFile}\nERROR: ${err.message}\n`,
),
);
}
}

if (!allResults.length) {
throw new FailedToRunTestError(
`Failed to get dependencies for all ${targetFiles.length} potential projects. Run with \`-d\` for debug output and contact support@snyk.io`,
);
}

return {
plugin: {
name: 'custom-auto-detect',
},
scannedProjects: allResults,
failedResults,
};
}
1 change: 1 addition & 0 deletions src/lib/plugins/nodejs-plugin/npm-modules-parser.ts
Expand Up @@ -34,6 +34,7 @@ export async function parse(
'Analyzing npm dependencies for ' +
path.dirname(path.resolve(root, targetFile));
try {
await spinner.clear<void>(resolveModuleSpinnerLabel)();
await spinner(resolveModuleSpinnerLabel);
if (targetFile.endsWith('yarn.lock')) {
options.file =
Expand Down
20 changes: 18 additions & 2 deletions src/lib/snyk-test/run-test.ts
Expand Up @@ -2,6 +2,7 @@ import * as fs from 'fs';
import * as _ from '@snyk/lodash';
import * as path from 'path';
import * as debugModule from 'debug';
import chalk from 'chalk';
import * as pathUtil from 'path';
import { parsePackageString as moduleToObject } from 'snyk-module';
import * as depGraphLib from '@snyk/dep-graph';
Expand Down Expand Up @@ -42,7 +43,10 @@ import {
} from '../types';
import { pruneGraph } from '../prune';
import { getDepsFromPlugin } from '../plugins/get-deps-from-plugin';
import { ScannedProjectCustom } from '../plugins/get-multi-plugin-result';
import {
ScannedProjectCustom,
MultiProjectResultCustom,
} from '../plugins/get-multi-plugin-result';

import request = require('../request');
import spinner = require('../spinner');
Expand Down Expand Up @@ -72,6 +76,7 @@ async function sendAndParseResults(
): Promise<TestResult[]> {
const results: TestResult[] = [];
for (const payload of payloads) {
await spinner.clear<void>(spinnerLbl)();
await spinner(spinnerLbl);
if (options.iac) {
const iacScan: IacScan = payload.body as IacScan;
Expand Down Expand Up @@ -348,12 +353,23 @@ async function assembleLocalPayloads(

try {
const payloads: Payload[] = [];

await spinner.clear<void>(spinnerLbl)();
await spinner(spinnerLbl);
if (options.iac) {
return assembleIacLocalPayloads(root, options);
}
const deps = await getDepsFromPlugin(root, options);
const failedResults = (deps as MultiProjectResultCustom).failedResults;
if (failedResults?.length) {
await spinner.clear<void>(spinnerLbl)();
console.warn(
chalk.bold.red(
`✗ ${failedResults.length}/${failedResults.length +
deps.scannedProjects
.length} potential projects failed to get dependencies. Run with \`-d\` for debug output.`,
),
);
}
analytics.add('pluginName', deps.plugin.name);
const javaVersion = _.get(
deps.plugin,
Expand Down
19 changes: 11 additions & 8 deletions test/acceptance/cli-monitor/cli-monitor.all-projects.spec.ts
Expand Up @@ -171,10 +171,14 @@ export const AllProjectsTests: AcceptanceTests = {
utils.chdirWorkspaces();
const spyPlugin = sinon.spy(params.plugins, 'loadPlugin');
t.teardown(spyPlugin.restore);

const result = await params.cli.monitor('monorepo-bad-project', {
allProjects: true,
});
let result;
try {
await params.cli.monitor('monorepo-bad-project', {
allProjects: true,
});
} catch (error) {
result = error.message;
}
t.ok(spyPlugin.withArgs('rubygems').calledOnce, 'calls rubygems plugin');
t.ok(spyPlugin.withArgs('yarn').calledOnce, 'calls npm plugin');
t.ok(spyPlugin.withArgs('maven').notCalled, 'did not call maven plugin');
Expand All @@ -184,11 +188,10 @@ export const AllProjectsTests: AcceptanceTests = {
'rubygems/graph/some/project-id',
'rubygems project was monitored',
);

t.notMatch(
t.match(
result,
'yarn/graph/some/project-id',
'yarn project was not monitored',
'Dependency snyk was not found in yarn.lock',
'yarn project had an error and we displayed it',
);

const request = params.server.popRequest();
Expand Down
17 changes: 17 additions & 0 deletions test/acceptance/cli-test/cli-test.all-projects.spec.ts
Expand Up @@ -7,6 +7,23 @@ import { CommandResult } from '../../../src/cli/commands/types';
export const AllProjectsTests: AcceptanceTests = {
language: 'Mixed',
tests: {
'`test yarn-out-of-sync` out of sync fails': (params, utils) => async (
t,
) => {
utils.chdirWorkspaces();
try {
await params.cli.test('yarn-workspace-out-of-sync', {
allProjects: true,
});
t.fail('Should fail');
} catch (e) {
t.equal(
e.message,
'\nTesting yarn-workspace-out-of-sync...\n\nFailed to get dependencies for all 3 potential projects. Run with `-d` for debug output and contact support@snyk.io',
'Contains enough info about err',
);
}
},
'`test mono-repo-with-ignores --all-projects` respects .snyk policy': (
params,
utils,
Expand Down

0 comments on commit 7ef59ed

Please sign in to comment.