module: print better message on esm syntax error

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 <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

10 files changed, 40 insertions, 7 deletions

diff --git a/.eslintignore b/.eslintignore
index 9b1d8b3f35..dc4e023866 100644
--- a/.eslintignore
+++ b/.eslintignore
@@ -3,6 +3,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 79c93a5aef..f404c4317d 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');
@@ -474,6 +475,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 dfba4d5b30..0e1f7c9eaf 100644
--- a/src/module_wrap.cc
+++ b/src/module_wrap.cc
@@ -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();
diff --git a/src/node_internals.h b/src/node_internals.h
index b7d032c68d..67ab0bb59e 100644
--- a/src/node_internals.h
+++ b/src/node_internals.h
@@ -269,7 +269,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,
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 0000000000..bda4a7e6eb
--- /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 0000000000..8291867255
--- /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 0000000000..0ca2bba549
--- /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 <anonymous>
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 0000000000..e74b70bec8
--- /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 0000000000..a76b72bdb6
--- /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 <anonymous>
diff --git a/test/message/testcfg.py b/test/message/testcfg.py
index 3c66845907..819dfa12c5 100644
--- a/test/message/testcfg.py
+++ b/test/message/testcfg.py
@@ -114,18 +114,18 @@ class MessageTestConfiguration(test.TestConfiguration):
 
   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,