Skip to content

Commit f023a22

Browse files
committed
test(linter/plugins): include stack trace in plugin loading errors (#13808)
Improve error output when there's an error loading a JS plugin. Previously only the error message was output, but no indication of where the error came from. Now we output stack trace for the error. In tests, adapt output normalizer to remove extraneous lines from stack traces and normalize file paths, so snapshots do not change when there are irrelevant changes to internal code.
1 parent a2342a6 commit f023a22

File tree

5 files changed

+64
-10
lines changed

5 files changed

+64
-10
lines changed

apps/oxlint/src-js/plugins/utils.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,28 @@
33
*
44
* `err` is expected to be an `Error` object, but can be anything.
55
*
6-
* This function will never throw, and always returns a string, even if:
6+
* * If it's an `Error`, the error message and stack trace is returned.
7+
* * If it's another object with a string `message` property, `message` is returned.
8+
* * Otherwise, a generic "Unknown error" message is returned.
9+
*
10+
* This function will never throw, and always returns a non-empty string, even if:
711
*
812
* * `err` is `null` or `undefined`.
913
* * `err` is an object with a getter for `message` property which throws.
10-
* * `err` has a getter for `message` property which returns a different value each time it's accessed.
14+
* * `err` has a getter for `stack` or `message` property which returns a different value each time it's accessed.
1115
*
1216
* @param err - Error
1317
* @returns Error message
1418
*/
1519
export function getErrorMessage(err: unknown): string {
1620
try {
17-
const { message } = err as undefined | { message: string };
21+
if (err instanceof Error) {
22+
// Note: `stack` includes the error message
23+
const { stack } = err;
24+
if (typeof stack === 'string' && stack !== '') return stack;
25+
}
26+
27+
const { message } = err as { message?: unknown };
1828
if (typeof message === 'string' && message !== '') return message;
1929
} catch {}
2030

apps/oxlint/test/__snapshots__/e2e.test.ts.snap

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -444,18 +444,27 @@ exports[`oxlint CLI > should report an error if a a rule is not found within a c
444444
"
445445
`;
446446

447-
exports[`oxlint CLI > should report an error if a custom plugin cannot be loaded 1`] = `
447+
exports[`oxlint CLI > should report an error if a custom plugin in config but JS plugins are not enabled 1`] = `
448+
"Failed to parse configuration file.
449+
450+
x JS plugins are not supported without \`--experimental-js-plugins\` CLI option
451+
"
452+
`;
453+
454+
exports[`oxlint CLI > should report an error if a custom plugin is missing 1`] = `
448455
"Failed to parse configuration file.
449456
450457
x Failed to load JS plugin: ./test_plugin
451458
| Cannot find module './test_plugin'
452459
"
453460
`;
454461

455-
exports[`oxlint CLI > should report an error if a custom plugin in config but JS plugins are not enabled 1`] = `
462+
exports[`oxlint CLI > should report an error if a custom plugin throws an error during import 1`] = `
456463
"Failed to parse configuration file.
457464
458-
x JS plugins are not supported without \`--experimental-js-plugins\` CLI option
465+
x Failed to load JS plugin: ./test_plugin
466+
| Error: whoops!
467+
| at <root>/apps/oxlint/test/fixtures/custom_plugin_import_error/test_plugin/index.js:1:7
459468
"
460469
`;
461470

apps/oxlint/test/e2e.test.ts

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import { execa } from 'execa';
66

77
const PACKAGE_ROOT_PATH = dirname(import.meta.dirname);
88
const CLI_PATH = pathJoin(PACKAGE_ROOT_PATH, 'dist/cli.js');
9+
const ROOT_URL = new URL('../../../', import.meta.url).href;
10+
const FIXTURES_URL = new URL('./fixtures/', import.meta.url).href;
911

1012
async function runOxlintWithoutPlugins(cwd: string, args: string[] = []) {
1113
return await execa('node', [CLI_PATH, ...args], {
@@ -19,9 +21,28 @@ async function runOxlint(cwd: string, args: string[] = []) {
1921
}
2022

2123
function normalizeOutput(output: string): string {
22-
return output
23-
.replace(/Finished in \d+(\.\d+)?(s|ms|us|ns)/, 'Finished in Xms')
24-
.replace(/using \d+ threads./, 'using X threads.');
24+
let lines = output.split('\n');
25+
26+
// Remove timing and thread count info which can vary between runs
27+
lines[lines.length - 1] = lines[lines.length - 1].replace(
28+
/^Finished in \d+(?:\.\d+)?(?:s|ms|us|ns) on (\d+) file(s?) using \d+ threads.$/,
29+
'Finished in Xms on $1 file$2 using X threads.',
30+
);
31+
32+
// Remove lines from stack traces which are outside `fixtures` directory.
33+
// Replace path to repo root in stack traces with `<root>`.
34+
lines = lines.flatMap((line) => {
35+
// e.g. ` | at file:///path/to/oxc/apps/oxlint/test/fixtures/foor/bar.js:1:1`
36+
// e.g. ` | at whatever (file:///path/to/oxc/apps/oxlint/test/fixtures/foor/bar.js:1:1)`
37+
const match = line.match(/^(\s*\|\s+at (?:.+?\()?)(.+)$/);
38+
if (match) {
39+
const [, premable, at] = match;
40+
return at.startsWith(FIXTURES_URL) ? [`${premable}<root>/${at.slice(ROOT_URL.length)}`] : [];
41+
}
42+
return [line];
43+
});
44+
45+
return lines.join('\n');
2546
}
2647

2748
describe('oxlint CLI', () => {
@@ -73,12 +94,18 @@ describe('oxlint CLI', () => {
7394
expect(normalizeOutput(stdout)).toMatchSnapshot();
7495
});
7596

76-
it('should report an error if a custom plugin cannot be loaded', async () => {
97+
it('should report an error if a custom plugin is missing', async () => {
7798
const { stdout, exitCode } = await runOxlint('test/fixtures/missing_custom_plugin');
7899
expect(exitCode).toBe(1);
79100
expect(normalizeOutput(stdout)).toMatchSnapshot();
80101
});
81102

103+
it('should report an error if a custom plugin throws an error during import', async () => {
104+
const { stdout, exitCode } = await runOxlint('test/fixtures/custom_plugin_import_error');
105+
expect(exitCode).toBe(1);
106+
expect(normalizeOutput(stdout)).toMatchSnapshot();
107+
});
108+
82109
it('should report an error if a rule is not found within a custom plugin', async () => {
83110
const { stdout, exitCode } = await runOxlint('test/fixtures/custom_plugin_missing_rule');
84111
expect(exitCode).toBe(1);
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"plugins": ["./test_plugin"],
3+
"categories": { "correctness": "off" },
4+
"rules": {
5+
"basic-custom-plugin/unknown-rule": "error"
6+
}
7+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
throw new Error("whoops!");

0 commit comments

Comments
 (0)