From b719b77215bea946c60b5369f95fccba0879ab0e Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 24 Nov 2017 01:50:28 +0100 Subject: [PATCH] module: print better message on esm syntax error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/issues/17277 PR-URL: https://github.com/nodejs/node/pull/17281 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Evan Lucas Reviewed-By: Michaël Zasso --- .eslintignore | 1 + lib/module.js | 2 ++ src/module_wrap.cc | 10 +++++++++- src/node_internals.h | 2 +- test/fixtures/es-module-loaders/syntax-error.mjs | 2 ++ test/message/esm_display_syntax_error.mjs | 3 +++ test/message/esm_display_syntax_error.out | 7 +++++++ test/message/esm_display_syntax_error_module.mjs | 3 +++ test/message/esm_display_syntax_error_module.out | 7 +++++++ test/message/testcfg.py | 10 +++++----- 10 files changed, 40 insertions(+), 7 deletions(-) create mode 100644 test/fixtures/es-module-loaders/syntax-error.mjs create mode 100644 test/message/esm_display_syntax_error.mjs create mode 100644 test/message/esm_display_syntax_error.out create mode 100644 test/message/esm_display_syntax_error_module.mjs create mode 100644 test/message/esm_display_syntax_error_module.out diff --git a/.eslintignore b/.eslintignore index 4b6e6b5e0fa94a..b9b743fb536cf5 100644 --- a/.eslintignore +++ b/.eslintignore @@ -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-* diff --git a/lib/module.js b/lib/module.js index 69e6feb6c9109d..6f767c86b9428a 100644 --- a/lib/module.js +++ b/lib/module.js @@ -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'); @@ -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); }); diff --git a/src/module_wrap.cc b/src/module_wrap.cc index dfba4d5b300f66..0e1f7c9eaf8685 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -103,9 +103,17 @@ void ModuleWrap::New(const FunctionCallbackInfo& 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 that = args.This(); diff --git a/src/node_internals.h b/src/node_internals.h index 0d766a59bf01a1..d7fbcc992f6a0f 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -262,7 +262,7 @@ constexpr size_t arraysize(const T(&)[N]) { return N; } bool IsExceptionDecorated(Environment* env, v8::Local er); -enum ErrorHandlingMode { FATAL_ERROR, CONTEXTIFY_ERROR }; +enum ErrorHandlingMode { CONTEXTIFY_ERROR, FATAL_ERROR, MODULE_ERROR }; void AppendExceptionLine(Environment* env, v8::Local er, v8::Local message, diff --git a/test/fixtures/es-module-loaders/syntax-error.mjs b/test/fixtures/es-module-loaders/syntax-error.mjs new file mode 100644 index 00000000000000..bda4a7e6ebe3a3 --- /dev/null +++ b/test/fixtures/es-module-loaders/syntax-error.mjs @@ -0,0 +1,2 @@ +'use strict'; +await async () => 0; diff --git a/test/message/esm_display_syntax_error.mjs b/test/message/esm_display_syntax_error.mjs new file mode 100644 index 00000000000000..829186725554bf --- /dev/null +++ b/test/message/esm_display_syntax_error.mjs @@ -0,0 +1,3 @@ +// Flags: --experimental-modules +'use strict'; +await async () => 0; diff --git a/test/message/esm_display_syntax_error.out b/test/message/esm_display_syntax_error.out new file mode 100644 index 00000000000000..0ca2bba5494470 --- /dev/null +++ b/test/message/esm_display_syntax_error.out @@ -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 diff --git a/test/message/esm_display_syntax_error_module.mjs b/test/message/esm_display_syntax_error_module.mjs new file mode 100644 index 00000000000000..e74b70bec8cc28 --- /dev/null +++ b/test/message/esm_display_syntax_error_module.mjs @@ -0,0 +1,3 @@ +// Flags: --experimental-modules +import '../common'; +import '../fixtures/es-module-loaders/syntax-error'; diff --git a/test/message/esm_display_syntax_error_module.out b/test/message/esm_display_syntax_error_module.out new file mode 100644 index 00000000000000..a76b72bdb69b63 --- /dev/null +++ b/test/message/esm_display_syntax_error_module.out @@ -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 diff --git a/test/message/testcfg.py b/test/message/testcfg.py index 3c668459079f0a..819dfa12c5b631 100644 --- a/test/message/testcfg.py +++ b/test/message/testcfg.py @@ -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,