Skip to content

Commit

Permalink
Include the filename in the error when an ES module has syntax errors
Browse files Browse the repository at this point in the history
* Fixes #168
  • Loading branch information
sgravrock committed Oct 24, 2020
1 parent 555e69a commit d1f4e62
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 2 deletions.
13 changes: 12 additions & 1 deletion lib/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,18 @@ function Loader(options) {

Loader.prototype.load = function(path) {
if (path.endsWith('.mjs')) {
return this.import_(path);
return this.import_(path).catch(function(e) {
if (e.message.indexOf(path) !== -1 || e.stack.indexOf(path) !== -1) {
return Promise.reject(e);
} else {
// When an ES module has a syntax error, the resulting exception does not
// include the filename. Add it. We lose the stack trace in the process,
// but the stack trace is usually not useful since it contains only frames
// from the Node module loader.
const updatedError = new Error(`While loading ${path}: ${e.constructor.name}: ${e.message}`);
return Promise.reject(updatedError);
}
});
} else {
return new Promise(resolve => {
this.require_(path);
Expand Down
7 changes: 7 additions & 0 deletions spec/fixtures/esm-load-exception/jasmine.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"spec_dir": ".",
"spec_files": [
"throws_on_load.mjs"
],
"helpers": []
}
1 change: 1 addition & 0 deletions spec/fixtures/esm-load-exception/throws_on_load.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
throw new Error("nope");
7 changes: 7 additions & 0 deletions spec/fixtures/esm-syntax-error/jasmine.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"spec_dir": ".",
"spec_files": [
"syntax_error.mjs"
],
"helpers": []
}
1 change: 1 addition & 0 deletions spec/fixtures/esm-syntax-error/syntax_error.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
function(
14 changes: 14 additions & 0 deletions spec/integration_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,26 @@ describe('Integration', function () {
expect(output).toMatch(/at .*throws_on_load.js/);
});

it('handles load-time exceptions from ESM specs properly', async function () {
const {exitCode, output} = await runJasmine('spec/fixtures/esm-load-exception');
expect(exitCode).toEqual(1);
expect(output).toContain('Error: nope');
expect(output).toMatch(/at .*throws_on_load.mjs/);
});

it('handles syntax errors in CommonJS specs properly', async function () {
const {exitCode, output} = await runJasmine('spec/fixtures/cjs-syntax-error');
expect(exitCode).toEqual(1);
expect(output).toContain('SyntaxError');
expect(output).toContain('syntax_error.js');
});

it('handles syntax errors in ESM specs properly', async function () {
const {exitCode, output} = await runJasmine('spec/fixtures/esm-syntax-error');
expect(exitCode).toEqual(1);
expect(output).toContain('SyntaxError');
expect(output).toContain('syntax_error.mjs');
});
});

async function runJasmine(cwd) {
Expand Down
13 changes: 12 additions & 1 deletion spec/loader_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,20 @@ describe('loader', function() {
await expectAsync(loaderPromise).toBeResolved();
});

it('propagates the error when import fails', async function () {
it("adds the filename to errors that don't include it", async function() {
const underlyingError = new SyntaxError('some details but no filename, not even in the stack trace');
const importShim = () => Promise.reject(underlyingError);
const loader = new Loader({importShim});

await expectAsync(loader.load('foo.mjs')).toBeRejectedWithError(
"While loading foo.mjs: SyntaxError: some details but no filename, not even in the stack trace"
);
});

it('propagates errors that already contain the filename without modifying them', async function () {
const requireShim = jasmine.createSpy('requireShim');
const underlyingError = new Error('nope');
underlyingError.stack = underlyingError.stack.replace('loader_spec.js', 'foo.mjs');
const importShim = jasmine.createSpy('importShim')
.and.callFake(() => Promise.reject(underlyingError));
const loader = new Loader({requireShim, importShim});
Expand Down

0 comments on commit d1f4e62

Please sign in to comment.