module: mark evaluation rejection in require(esm) as handled

Previously the implemention of require(esm) only converted the
rejected promise from module evaluation into an error, but the
rejected promise was still treated as a pending unhandled
rejection by the promise rejection callback, because the promise
is created by V8 internals and we don't get a chance to mark
it as handled, so the rejection incorrectly marked as unhandled
would still go through unhandled rejection handling (if no
global listener is set, the default handling would print a warning
and make the Node.js instance exit with 1).

This patch fixes it by calling into the JS promise rejection
callback to mark the evalaution rejection handled so that
it doesn't go through unhandled rejection handling.

PR-URL: #56122
Fixes: #56115
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>

src/module_wrap.cc

@@ -663,6 +663,22 @@ void ModuleWrap::EvaluateSync(const FunctionCallbackInfo<Value>& args) {
   CHECK(result->IsPromise());
   Local<Promise> promise = result.As<Promise>();
   if (promise->State() == Promise::PromiseState::kRejected) {
+    // The rejected promise is created by V8, so we don't get a chance to mark
+    // it as resolved before the rejection happens from evaluation. But we can
+    // tell the promise rejection callback to treat it as a promise rejected
+    // before handler was added which would remove it from the unhandled
+    // rejection handling, since we are converting it into an error and throw
+    // from here directly.
+    Local<Value> type = v8::Integer::New(
+        isolate,
+        static_cast<int32_t>(
+            v8::PromiseRejectEvent::kPromiseHandlerAddedAfterReject));
+    Local<Value> args[] = {type, promise, Undefined(isolate)};
+    if (env->promise_reject_callback()
+            ->Call(context, Undefined(isolate), arraysize(args), args)
+            .IsEmpty()) {
+      return;
+    }
     Local<Value> exception = promise->Result();
     Local<v8::Message> message =
         v8::Exception::CreateMessage(isolate, exception);

test/es-module/test-require-module-error-catching.js

@@ -0,0 +1,21 @@
+// This tests synchronous errors in ESM from require(esm) can be caught.
+
+'use strict';
+
+require('../common');
+const assert = require('assert');
+
+// Runtime errors from throw should be caught.
+assert.throws(() => {
+  require('../fixtures/es-modules/runtime-error-esm.js');
+}, {
+  message: 'hello'
+});
+
+// References errors should be caught too.
+assert.throws(() => {
+  require('../fixtures/es-modules/reference-error-esm.js');
+}, {
+  name: 'ReferenceError',
+  message: 'exports is not defined'
+});

test/es-module/test-require-module-synchronous-rejection-handling.js

@@ -0,0 +1,13 @@
+// This synchronous rejections from require(esm) still go to the unhandled rejection
+// handler.
+
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+
+process.on('unhandledRejection', common.mustCall((reason, promise) => {
+  assert.strictEqual(reason, 'reject!');
+}));
+
+require('../fixtures/es-modules/synchronous-rejection-esm.js');

test/fixtures/es-modules/reference-error-esm.js

@@ -0,0 +1,5 @@
+// This module is invalid in both ESM and CJS, because
+// 'exports' are not defined in ESM, while require cannot be
+// redeclared in CJS.
+Object.defineProperty(exports, "__esModule", { value: true });
+const require = () => {};

test/fixtures/es-modules/runtime-error-esm.js

@@ -0,0 +1,2 @@
+import 'node:fs';  // Forces it to be recognized as ESM.
+throw new Error('hello');

test/fixtures/es-modules/synchronous-rejection-esm.js

@@ -0,0 +1,2 @@
+import 'node:fs';  // Forces it to be recognized as ESM.
+Promise.reject('reject!');