Skip to content

Commit

Permalink
module: print better message on esm syntax error
Browse files Browse the repository at this point in the history
Include the offending line in the output and underline the bad token.

Before this commit, it printed "SyntaxError: Unexpected reserved word"
without indicating where the syntax error is.

Now it prints the line and underlines the offending token, like it does
for syntax errors in CJS scripts.

Minor changes are made to the test runner in order to support `*.mjs`
files in test/message.

Fixes: #17277
PR-URL: #17281
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
  • Loading branch information
bnoordhuis authored and MylesBorins committed Dec 12, 2017
1 parent 742a456 commit b719b77
Show file tree
Hide file tree
Showing 10 changed files with 40 additions and 7 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ lib/internal/v8_prof_polyfill.js
lib/punycode.js
test/addons/??_*
test/fixtures
test/message/esm_display_syntax_error.mjs
tools/eslint
tools/icu
tools/remark-*
Expand Down
2 changes: 2 additions & 0 deletions lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

const NativeModule = require('native_module');
const util = require('util');
const { decorateErrorStack } = require('internal/util');
const internalModule = require('internal/module');
const { getURLFromFilePath } = require('internal/url');
const vm = require('vm');
Expand Down Expand Up @@ -471,6 +472,7 @@ Module._load = function(request, parent, isMain) {
await ESMLoader.import(getURLFromFilePath(request).pathname);
})()
.catch((e) => {
decorateErrorStack(e);
console.error(e);
process.exit(1);
});
Expand Down
10 changes: 9 additions & 1 deletion src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,17 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
False(isolate), // is opaque (?)
False(isolate), // is WASM
True(isolate)); // is ES6 module
TryCatch try_catch(isolate);
ScriptCompiler::Source source(source_text, origin);
if (!ScriptCompiler::CompileModule(isolate, &source).ToLocal(&module))
if (!ScriptCompiler::CompileModule(isolate, &source).ToLocal(&module)) {
CHECK(try_catch.HasCaught());
CHECK(!try_catch.Message().IsEmpty());
CHECK(!try_catch.Exception().IsEmpty());
AppendExceptionLine(env, try_catch.Exception(), try_catch.Message(),
ErrorHandlingMode::MODULE_ERROR);
try_catch.ReThrow();
return;
}
}

Local<Object> that = args.This();
Expand Down
2 changes: 1 addition & 1 deletion src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ constexpr size_t arraysize(const T(&)[N]) { return N; }

bool IsExceptionDecorated(Environment* env, v8::Local<v8::Value> er);

enum ErrorHandlingMode { FATAL_ERROR, CONTEXTIFY_ERROR };
enum ErrorHandlingMode { CONTEXTIFY_ERROR, FATAL_ERROR, MODULE_ERROR };
void AppendExceptionLine(Environment* env,
v8::Local<v8::Value> er,
v8::Local<v8::Message> message,
Expand Down
2 changes: 2 additions & 0 deletions test/fixtures/es-module-loaders/syntax-error.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
'use strict';
await async () => 0;
3 changes: 3 additions & 0 deletions test/message/esm_display_syntax_error.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Flags: --experimental-modules
'use strict';
await async () => 0;
7 changes: 7 additions & 0 deletions test/message/esm_display_syntax_error.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
(node:*) ExperimentalWarning: The ESM module loader is experimental.
file:///*/test/message/esm_display_syntax_error.mjs:3
await async () => 0;
^^^^^
SyntaxError: Unexpected reserved word
at loaders.set (internal/loader/ModuleRequest.js:*:*)
at <anonymous>
3 changes: 3 additions & 0 deletions test/message/esm_display_syntax_error_module.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Flags: --experimental-modules
import '../common';
import '../fixtures/es-module-loaders/syntax-error';
7 changes: 7 additions & 0 deletions test/message/esm_display_syntax_error_module.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
(node:*) ExperimentalWarning: The ESM module loader is experimental.
file:///*/test/fixtures/es-module-loaders/syntax-error.mjs:2
await async () => 0;
^^^^^
SyntaxError: Unexpected reserved word
at loaders.set (internal/loader/ModuleRequest.js:*:*)
at <anonymous>
10 changes: 5 additions & 5 deletions test/message/testcfg.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,18 @@ def __init__(self, context, root):

def Ls(self, path):
if isdir(path):
return [f[:-3] for f in os.listdir(path) if f.endswith('.js')]
return [f for f in os.listdir(path)
if f.endswith('.js') or f.endswith('.mjs')]
else:
return []
return []

def ListTests(self, current_path, path, arch, mode):
all_tests = [current_path + [t] for t in self.Ls(self.root)]
result = []
for test in all_tests:
if self.Contains(path, test):
file_prefix = join(self.root, reduce(join, test[1:], ""))
file_path = file_prefix + ".js"
output_path = file_prefix + ".out"
file_path = join(self.root, reduce(join, test[1:], ''))
output_path = file_path[:file_path.rfind('.')] + '.out'
if not exists(output_path):
raise Exception("Could not find %s" % output_path)
result.append(MessageTestCase(test, file_path, output_path,
Expand Down

0 comments on commit b719b77

Please sign in to comment.