Skip to content

Commit

Permalink
Handle esm's error "improvements"
Browse files Browse the repository at this point in the history
esm changes error stacks in a way that AVA is not expecting. This adds a
few workarounds, though admittedly all the packages we're using and
workarounds we're applying are making a bit of a mess.

See https://github.com/standard-things/esm/wiki/improved-errors for
esm's behavior.
  • Loading branch information
novemberborn committed Oct 28, 2018
1 parent a130a9e commit 37390e6
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 2 deletions.
5 changes: 4 additions & 1 deletion lib/beautify-stack.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,8 @@ module.exports = stack => {

return stackUtils.clean(stack)
// Remove the trailing newline inserted by the `stack-utils` module
.trim();
.trim()
// Remove remaining file:// prefixes, inserted by `esm`, that are not
// cleaned up by `stack-utils`
.split('\n').map(line => line.replace(/\(file:\/\/([^/].+:\d+:\d+)\)$/, '($1)')).join('\n');

This comment has been minimized.

Copy link
@jdalton

jdalton Dec 6, 2018

Contributor

☝️ Node currently uses file:// URL's for their experimental ESM usage too (is reported in their stacks).

};
4 changes: 3 additions & 1 deletion lib/serialize-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ function trySerializeError(err, shouldBeautifyStack) {
}
retval.summary = retval.summary.trim();
} else {
retval.summary = lines[0];
// Skip the source line header inserted by `esm`:
// <https://github.com/standard-things/esm/wiki/improved-errors>
retval.summary = lines.find(line => !/:\d+$/.test(line));

This comment has been minimized.

Copy link
@jdalton

jdalton Dec 6, 2018

Contributor

☝️ The source line header exists in built-in stack traces that are decorated by Node as well (isn't esm specific).

}
}

Expand Down
26 changes: 26 additions & 0 deletions test/serialize-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,3 +176,29 @@ test('creates multiline summaries for syntax errors', t => {
t.is(serializedError.summary, 'Hello\nThere\nSyntaxError here');
t.end();
});

test('skips esm enhancement lines when finding the summary', t => {
const error = new Error();
Object.defineProperty(error, 'stack', {
value: 'file://file.js:1\nHello'
});
const serializedError = serialize(error);
t.is(serializedError.summary, 'Hello');
t.end();
});

test('works around esm\'s insertion of file:// urls', t => {
const fixture = sourceMapFixtures.mapFile('throws');
try {
fixture.require().run();
t.fail('Fixture should have thrown');
} catch (error) {
const expected = serialize(error);
Object.defineProperty(error, 'stack', {
value: error.stack.split('\n').map(line => line.replace('(/', '(file:///')).join('\n')
});
const serializedError = serialize(error);
t.is(serializedError.source.file, expected.source.file);
t.end();
}
});

1 comment on commit 37390e6

@jdalton
Copy link
Contributor

@jdalton jdalton commented on 37390e6 Dec 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's anything else you run into please don't hesitate to ping me on it (even for things like this).

Please sign in to comment.