feat(cli): improve error logging (#2071)

.eslintrc

@@ -84,10 +84,12 @@
 
     {
       "files": [
-        "src/**/__tests__/**/*.jest.test.{ts,js}"
+        "src/**/__tests__/**/*.jest.test.{ts,js}",
+        "__mocks__/**/*.{ts,js}"
       ],
       "env": {
-        "jest": true
+        "jest": true,
+        "node": true
       }
     },
 

__mocks__/process.js

@@ -0,0 +1,15 @@
+module.exports.exit = jest.fn();
+module.exports.cwd = jest.fn();
+module.exports.on = jest.fn();
+
+module.exports.stdin = {
+  fd: 0,
+  isTTY: true,
+};
+module.exports.stdout = {
+  write: jest.fn(),
+};
+
+module.exports.stderr = {
+  write: jest.fn(),
+};

karma.conf.ts

@@ -40,7 +40,7 @@ module.exports = (config: Config): void => {
             'nimma/legacy': require.resolve('./node_modules/nimma/dist/legacy/cjs/index.js'),
             'node-fetch': require.resolve('./__karma__/fetch'),
             fs: require.resolve('./__karma__/fs'),
-            process: require.resolve('./__karma__/process'),
+            process: require.resolve('./__mocks__/process'),
             perf_hooks: require.resolve('./__karma__/perf_hooks'),
             fsevents: require.resolve('./__karma__/fsevents'),
           },

packages/cli/package.json

@@ -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",
@@ -73,7 +76,9 @@
     ],
     "assets": [
       "./dist/**/*.json",
-      "./dist/**/*.html"
+      "./dist/**/*.html",
+      "../*/dist/**/*.js.map",
+      "../*/src/**/*.ts"
     ]
   }
 }

packages/cli/src/commands/__tests__/lint.test.ts

@@ -1,12 +1,16 @@
 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');
 jest.mock('../../services/output');
 jest.mock('../../services/linter');
 
@@ -24,9 +28,6 @@ function run(command: string) {
 }
 
 describe('lint', () => {
-  let errorSpy: jest.SpyInstance;
-  const { isTTY } = process.stdin;
-
   const results: IRuleResult[] = [
     {
       code: 'parser',
@@ -47,29 +48,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');
@@ -150,35 +148,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');
@@ -216,8 +208,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:236`),
+    );
+
+    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:237`),
+    );
+
+    expect(process.stderr.write).nthCalledWith(6, `Error #3: ${chalk.red('original exception')}\n`);
   });
 });