Skip to content

Commit

Permalink
feat(cli): improve error logging
Browse files Browse the repository at this point in the history
  • Loading branch information
@P0lip
P0lip committed Feb 21, 2022
1 parent 2953155 commit 2b5b9c3
Show file tree
Hide file tree
Showing 11 changed files with 375 additions and 97 deletions.
7 changes: 6 additions & 1 deletion packages/cli/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,18 @@
"lodash": "~4.17.21",
"pony-cause": "^1.0.0",
"proxy-agent": "5.0.0",
"stacktracey": "^2.1.7",
"strip-ansi": "6.0",
"text-table": "0.2",
"tslib": "^2.3.0",
"yargs": "17.3.1"
},
"devDependencies": {
"@types/es-aggregate-error": "^1.0.2",
"@types/xml2js": "^0.4.9",
"@types/yargs": "^17.0.8",
"copyfiles": "^2.4.1",
"es-aggregate-error": "^1.0.7",
"jest-when": "^3.4.2",
"nock": "^13.1.3",
"node-html-parser": "^4.1.5",
Expand All @@ -73,7 +76,9 @@
],
"assets": [
"./dist/**/*.json",
"./dist/**/*.html"
"./dist/**/*.html",
"../*/dist/**/*.js.map",
"../*/src/**/*.ts"
]
}
}
93 changes: 70 additions & 23 deletions packages/cli/src/commands/__tests__/lint.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,28 @@
import * as yargs from 'yargs';
import { noop } from 'lodash';
import { DiagnosticSeverity } from '@stoplight/types';
import { IRuleResult } from '@stoplight/spectral-core';
import * as process from 'process';
import { ErrorWithCause } from 'pony-cause';
import AggregateError from 'es-aggregate-error';

import { lint } from '../../services/linter';
import { formatOutput, writeOutput } from '../../services/output';
import lintCommand from '../lint';
import chalk from 'chalk';

jest.mock('process', () => ({
exit: jest.fn(),
stdin: {
fd: 0,
isTTY: true,
},
stdout: {
write: jest.fn(),
},
stderr: {
write: jest.fn(),
},
}));
jest.mock('../../services/output');
jest.mock('../../services/linter');

Expand All @@ -24,9 +40,6 @@ function run(command: string) {
}

describe('lint', () => {
let errorSpy: jest.SpyInstance;
const { isTTY } = process.stdin;

const results: IRuleResult[] = [
{
code: 'parser',
Expand All @@ -47,29 +60,26 @@ describe('lint', () => {
];

beforeEach(() => {
(lint as jest.Mock).mockClear();
(lint as jest.Mock).mockResolvedValueOnce(results);

(formatOutput as jest.Mock).mockClear();
(formatOutput as jest.Mock).mockReturnValueOnce('<formatted output>');

(writeOutput as jest.Mock).mockClear();
(writeOutput as jest.Mock).mockResolvedValueOnce(undefined);

errorSpy = jest.spyOn(console, 'error').mockImplementation(noop);
});

afterEach(() => {
errorSpy.mockRestore();
process.stdin.isTTY = isTTY;
process.stdin.isTTY = true;
jest.clearAllMocks();
jest.resetAllMocks();
});

it('shows help when no document and no STDIN are present', () => {
process.stdin.isTTY = true;
return expect(run('lint')).rejects.toContain('documents Location of JSON/YAML documents');
});

describe('when STDIN is present', () => {
beforeEach(() => {
process.stdin.isTTY = false;
});

it('does not show help when documents are missing', async () => {
const output = await run('lint');
expect(output).not.toContain('documents Location of JSON/YAML documents');
Expand Down Expand Up @@ -150,35 +160,29 @@ describe('lint', () => {

it.each(['json', 'stylish'])('calls formatOutput with %s format', async format => {
await run(`lint -f ${format} ./__fixtures__/empty-oas2-document.json`);
await new Promise(resolve => void process.nextTick(resolve));
expect(formatOutput).toBeCalledWith(results, format, { failSeverity: DiagnosticSeverity.Error });
});

it('writes formatted output to a file', async () => {
await run(`lint -o foo.json ./__fixtures__/empty-oas2-document.json`);
await new Promise(resolve => void process.nextTick(resolve));
expect(writeOutput).toBeCalledWith('<formatted output>', 'foo.json');
});

it('writes formatted output to multiple files when using format and output flags', async () => {
(formatOutput as jest.Mock).mockClear();
(formatOutput as jest.Mock).mockReturnValue('<formatted output>');

await run(
`lint --format html --format json --output.json foo.json --output.html foo.html ./__fixtures__/empty-oas2-document.json`,
);
await new Promise(resolve => void process.nextTick(resolve));
expect(writeOutput).toBeCalledTimes(2);
expect(writeOutput).nthCalledWith(1, '<formatted output>', 'foo.html');
expect(writeOutput).nthCalledWith(2, '<formatted output>', 'foo.json');
});

it('writes formatted output to multiple files and stdout when using format and output flags', async () => {
(formatOutput as jest.Mock).mockClear();
(formatOutput as jest.Mock).mockReturnValue('<formatted output>');

await run(`lint --format html --format json --output.json foo.json ./__fixtures__/empty-oas2-document.json`);
await new Promise(resolve => void process.nextTick(resolve));
expect(writeOutput).toBeCalledTimes(2);
expect(writeOutput).nthCalledWith(1, '<formatted output>', '<stdout>');
expect(writeOutput).nthCalledWith(2, '<formatted output>', 'foo.json');
Expand Down Expand Up @@ -216,8 +220,51 @@ describe('lint', () => {
const error = new Error('Failure');
(lint as jest.Mock).mockReset();
(lint as jest.Mock).mockRejectedValueOnce(error);
await run(`lint -o foo.json ./__fixtures__/empty-oas2-document.json`);
await new Promise(resolve => void process.nextTick(resolve));
expect(errorSpy).toBeCalledWith('Failure');
await run(`lint ./__fixtures__/empty-oas2-document.json`);
expect(process.stderr.write).nthCalledWith(1, chalk.red('Error running Spectral!\n'));
expect(process.stderr.write).nthCalledWith(2, chalk.red('Use --verbose flag to print the error stack.\n'));
expect(process.stderr.write).nthCalledWith(3, `Error #1: ${chalk.red('Failure')}\n`);
});

it('prints each error separately', async () => {
(lint as jest.Mock).mockReset();
(lint as jest.Mock).mockRejectedValueOnce(
new AggregateError([
new Error('some unhandled exception'),
new TypeError('another one'),
new ErrorWithCause('some error with cause', { cause: 'original exception' }),
]),
);
await run(`lint ./__fixtures__/empty-oas2-document.json`);
expect(process.stderr.write).nthCalledWith(3, `Error #1: ${chalk.red('some unhandled exception')}\n`);
expect(process.stderr.write).nthCalledWith(4, `Error #2: ${chalk.red('another one')}\n`);
expect(process.stderr.write).nthCalledWith(5, `Error #3: ${chalk.red('original exception')}\n`);
});

it('given verbose flag, prints each error together with their stacks', async () => {
(lint as jest.Mock).mockReset();
(lint as jest.Mock).mockRejectedValueOnce(
new AggregateError([
new Error('some unhandled exception'),
new TypeError('another one'),
new ErrorWithCause('some error with cause', { cause: 'original exception' }),
]),
);

await run(`lint --verbose ./__fixtures__/empty-oas2-document.json`);

expect(process.stderr.write).nthCalledWith(2, `Error #1: ${chalk.red('some unhandled exception')}\n`);
expect(process.stderr.write).nthCalledWith(
3,
expect.stringContaining(`packages/cli/src/commands/__tests__/lint.test.ts:248`),
);

expect(process.stderr.write).nthCalledWith(4, `Error #2: ${chalk.red('another one')}\n`);
expect(process.stderr.write).nthCalledWith(
5,
expect.stringContaining(`packages/cli/src/commands/__tests__/lint.test.ts:249`),
);

expect(process.stderr.write).nthCalledWith(6, `Error #3: ${chalk.red('original exception')}\n`);
});
});
135 changes: 93 additions & 42 deletions packages/cli/src/commands/lint.ts
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
import { Dictionary } from '@stoplight/types';
import { isPlainObject } from '@stoplight/json';
import { difference, noop, pick } from 'lodash';
import { ReadStream } from 'tty';
import type { CommandModule } from 'yargs';
import { getDiagnosticSeverity, IRuleResult } from '@stoplight/spectral-core';
import { isError, difference, pick } from 'lodash';
import type { ReadStream } from 'tty';
import type { CommandModule } from 'yargs';
import * as process from 'process';
import chalk from 'chalk';
import type { ErrorWithCause } from 'pony-cause';
import StackTracey from 'stacktracey';

import { lint } from '../services/linter';
import { formatOutput, writeOutput } from '../services/output';
import { FailSeverity, ILintConfig, OutputFormat } from '../services/config';

import { CLIError } from '../errors';

const formatOptions = Object.values(OutputFormat);

const lintCommand: CommandModule = {
Expand Down Expand Up @@ -55,7 +61,7 @@ const lintCommand: CommandModule = {
})
.check((argv: Dictionary<unknown>) => {
if (!Array.isArray(argv.documents) || argv.documents.length === 0) {
throw new TypeError('No documents provided.');
throw new CLIError('No documents provided.');
}

const format = argv.format as string[] & { 0: string };
Expand All @@ -66,21 +72,21 @@ const lintCommand: CommandModule = {
return true;
}

throw new TypeError('Output must be either string or unspecified when a single format is specified');
throw new CLIError('Output must be either string or unspecified when a single format is specified');
}

if (!isPlainObject(output)) {
throw new TypeError('Multiple outputs have to be provided when more than a single format is specified');
throw new CLIError('Multiple outputs have to be provided when more than a single format is specified');
}

const keys = Object.keys(output);
if (format.length !== keys.length) {
throw new TypeError('The number of outputs must match the number of formats');
throw new CLIError('The number of outputs must match the number of formats');
}

const diff = difference(format, keys);
if (diff.length !== 0) {
throw new TypeError(`Missing outputs for the following formats: ${diff.join(', ')}`);
throw new CLIError(`Missing outputs for the following formats: ${diff.join(', ')}`);
}

return true;
Expand Down Expand Up @@ -156,7 +162,7 @@ const lintCommand: CommandModule = {
},
}),

handler: args => {
async handler(args) {
const {
documents,
failSeverity,
Expand All @@ -175,45 +181,90 @@ const lintCommand: CommandModule = {
displayOnlyFailures: boolean;
};

return lint(documents, {
format,
output,
encoding,
ignoreUnknownFormat,
failOnUnmatchedGlobs,
ruleset,
stdinFilepath,
...pick<Partial<ILintConfig>, keyof ILintConfig>(config, ['verbose', 'quiet', 'resolver']),
})
.then(results => {
if (displayOnlyFailures) {
return filterResultsBySeverity(results, failSeverity);
}
return results;
})
.then(results => {
if (results.length > 0) {
process.exitCode = severeEnoughToFail(results, failSeverity) ? 1 : 0;
} else if (config.quiet !== true) {
console.log(`No results with a severity of '${failSeverity}' or higher found!`);
}
try {
let results = await lint(documents, {
format,
output,
encoding,
ignoreUnknownFormat,
failOnUnmatchedGlobs,
ruleset,
stdinFilepath,
...pick<Partial<ILintConfig>, keyof ILintConfig>(config, ['verbose', 'quiet', 'resolver']),
});

return Promise.all(
format.map(f => {
const formattedOutput = formatOutput(results, f, { failSeverity: getDiagnosticSeverity(failSeverity) });
return writeOutput(formattedOutput, output?.[f] ?? '<stdout>');
}),
).then(noop);
})
.catch(fail);
if (displayOnlyFailures) {
results = filterResultsBySeverity(results, failSeverity);
}

await Promise.all(
format.map(f => {
const formattedOutput = formatOutput(results, f, { failSeverity: getDiagnosticSeverity(failSeverity) });
return writeOutput(formattedOutput, output?.[f] ?? '<stdout>');
}),
);

if (results.length > 0) {
process.exit(severeEnoughToFail(results, failSeverity) ? 1 : 0);
} else if (config.quiet !== true) {
process.stdout.write(`No results with a severity of '${failSeverity}' or higher found!`);
}
} catch (ex) {
fail(isError(ex) ? ex : new Error(String(ex)), config.verbose === true);
}
},
};

const fail = ({ message }: Error): void => {
console.error(message);
process.exitCode = 2;
const fail = (error: Error | ErrorWithCause<unknown> | AggregateError, verbose: boolean): void => {
if (error instanceof CLIError) {
process.stderr.write(chalk.red(error.message));
process.exit(2);
}

const errors: unknown[] = 'errors' in error ? error.errors : [error];

process.stderr.write(chalk.red('Error running Spectral!\n'));

if (!verbose) {
process.stderr.write(chalk.red('Use --verbose flag to print the error stack.\n'));
}

for (const [i, error] of errors.entries()) {
const actualError: unknown = isError(error) && 'cause' in error ? (error as ErrorWithCause<unknown>).cause : error;
const message = isError(actualError) ? actualError.message : String(actualError);

const info = `Error #${i + 1}: `;

process.stderr.write(`${info}${chalk.red(message)}\n`);

if (verbose && isError(actualError)) {
process.stderr.write(`${chalk.red(printErrorStacks(actualError, info.length))}\n`);
}
}

process.exit(2);
};

function getWidth(ratio: number): number {
return Math.min(20, Math.floor(ratio * process.stderr.columns));
}

function printErrorStacks(error: Error, padding: number): string {
return new StackTracey(error)
.slice(0, 5)
.withSources()
.asTable({
maxColumnWidths: {
callee: getWidth(0.2),
file: getWidth(0.4),
sourceLine: getWidth(0.4),
},
})
.split('\n')
.map(error => `${' '.repeat(padding)}${error}`)
.join('\n');
}

const filterResultsBySeverity = (results: IRuleResult[], failSeverity: FailSeverity): IRuleResult[] => {
const diagnosticSeverity = getDiagnosticSeverity(failSeverity);
return results.filter(r => r.severity <= diagnosticSeverity);
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/errors/index.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export class CLIError extends Error {}
Loading

0 comments on commit 2b5b9c3

Please sign in to comment.