summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBartek IwaƄczuk <biwanczuk@gmail.com>2023-05-24 15:40:41 +0200
committerGitHub <noreply@github.com>2023-05-24 15:40:41 +0200
commit0bb5bbc7a0ff7565a4c7fa4ebc8c69e02f76e6b5 (patch)
treea5ab85f34d7984428288cb8d221cf09851d333bd
parent787e1f0f92e81f025ec0dfdce2acff15900267fc (diff)
fix(node): fire 'unhandledrejection' event when using node: or npm: imports (#19235)
This commit fixes emitting "unhandledrejection" event when there are "node:" or "npm:" imports. Before this commit the Node "unhandledRejection" event was emitted using a regular listener for Web "unhandledrejection" event. This listener was installed before any user listener had a chance to be installed which effectively prevent emitting "unhandledrejection" events to user code. Closes https://github.com/denoland/deno/issues/16928
-rw-r--r--cli/tests/integration/node_unit_tests.rs18
-rw-r--r--cli/tests/testdata/node/unhandled_rejection_web.ts17
-rw-r--r--cli/tests/testdata/node/unhandled_rejection_web.ts.out4
-rw-r--r--cli/tests/testdata/node/unhandled_rejection_web_process.ts21
-rw-r--r--cli/tests/testdata/node/unhandled_rejection_web_process.ts.out5
-rw-r--r--ext/node/polyfills/process.ts8
-rw-r--r--runtime/js/99_main.js12
7 files changed, 80 insertions, 5 deletions
diff --git a/cli/tests/integration/node_unit_tests.rs b/cli/tests/integration/node_unit_tests.rs
index 2cde51552..7c524eb8b 100644
--- a/cli/tests/integration/node_unit_tests.rs
+++ b/cli/tests/integration/node_unit_tests.rs
@@ -5,6 +5,7 @@ use std::process::Stdio;
use std::time::Duration;
use std::time::Instant;
use test_util as util;
+use util::env_vars_for_npm_tests;
util::unit_test_factory!(
node_unit_test,
@@ -157,3 +158,20 @@ fn node_unit_test(test: String) {
assert!(status.success());
}
+
+// Regression test for https://github.com/denoland/deno/issues/16928
+itest!(unhandled_rejection_web {
+ args: "run -A node/unhandled_rejection_web.ts",
+ output: "node/unhandled_rejection_web.ts.out",
+ envs: env_vars_for_npm_tests(),
+ http_server: true,
+});
+
+// Ensure that Web `onunhandledrejection` is fired before
+// Node's `process.on('unhandledRejection')`.
+itest!(unhandled_rejection_web_process {
+ args: "run -A node/unhandled_rejection_web_process.ts",
+ output: "node/unhandled_rejection_web_process.ts.out",
+ envs: env_vars_for_npm_tests(),
+ http_server: true,
+});
diff --git a/cli/tests/testdata/node/unhandled_rejection_web.ts b/cli/tests/testdata/node/unhandled_rejection_web.ts
new file mode 100644
index 000000000..396c58c2a
--- /dev/null
+++ b/cli/tests/testdata/node/unhandled_rejection_web.ts
@@ -0,0 +1,17 @@
+import chalk from "npm:chalk";
+
+console.log(chalk.red("Hello world!"));
+
+globalThis.addEventListener("unhandledrejection", (e) => {
+ console.log("Handled the promise rejection");
+ e.preventDefault();
+});
+
+// deno-lint-ignore require-await
+(async () => {
+ throw new Error("boom!");
+})();
+
+setTimeout(() => {
+ console.log("Success");
+}, 1000);
diff --git a/cli/tests/testdata/node/unhandled_rejection_web.ts.out b/cli/tests/testdata/node/unhandled_rejection_web.ts.out
new file mode 100644
index 000000000..19db7f90e
--- /dev/null
+++ b/cli/tests/testdata/node/unhandled_rejection_web.ts.out
@@ -0,0 +1,4 @@
+[WILDCARD]
+Hello world!
+Handled the promise rejection
+Success
diff --git a/cli/tests/testdata/node/unhandled_rejection_web_process.ts b/cli/tests/testdata/node/unhandled_rejection_web_process.ts
new file mode 100644
index 000000000..2aaacfbff
--- /dev/null
+++ b/cli/tests/testdata/node/unhandled_rejection_web_process.ts
@@ -0,0 +1,21 @@
+import chalk from "npm:chalk";
+import process from "node:process";
+
+console.log(chalk.red("Hello world!"));
+
+process.on("unhandledRejection", (_e) => {
+ console.log('process.on("unhandledRejection");');
+});
+
+globalThis.addEventListener("unhandledrejection", (_e) => {
+ console.log('globalThis.addEventListener("unhandledrejection");');
+});
+
+// deno-lint-ignore require-await
+(async () => {
+ throw new Error("boom!");
+})();
+
+setTimeout(() => {
+ console.log("Success");
+}, 1000);
diff --git a/cli/tests/testdata/node/unhandled_rejection_web_process.ts.out b/cli/tests/testdata/node/unhandled_rejection_web_process.ts.out
new file mode 100644
index 000000000..ea307474a
--- /dev/null
+++ b/cli/tests/testdata/node/unhandled_rejection_web_process.ts.out
@@ -0,0 +1,5 @@
+[WILDCARD]
+Hello world!
+globalThis.addEventListener("unhandledrejection");
+process.on("unhandledRejection");
+Success
diff --git a/ext/node/polyfills/process.ts b/ext/node/polyfills/process.ts
index d2f220734..2dc10d7b1 100644
--- a/ext/node/polyfills/process.ts
+++ b/ext/node/polyfills/process.ts
@@ -716,9 +716,9 @@ internals.__bootstrapNodeProcess = function (
core.setMacrotaskCallback(runNextTicks);
enableNextTick();
- // TODO(bartlomieju): this is buggy, see https://github.com/denoland/deno/issues/16928
- // We should use a specialized API in 99_main.js instead
- globalThis.addEventListener("unhandledrejection", (event) => {
+ // Install special "unhandledrejection" handler, that will be called
+ // last.
+ internals.nodeProcessUnhandledRejectionCallback = (event) => {
if (process.listenerCount("unhandledRejection") === 0) {
// The Node.js default behavior is to raise an uncaught exception if
// an unhandled rejection occurs and there are no unhandledRejection
@@ -734,7 +734,7 @@ internals.__bootstrapNodeProcess = function (
event.preventDefault();
process.emit("unhandledRejection", event.reason, event.promise);
- });
+ };
globalThis.addEventListener("error", (event) => {
if (process.listenerCount("uncaughtException") > 0) {
diff --git a/runtime/js/99_main.js b/runtime/js/99_main.js
index f0c63df74..91d34272f 100644
--- a/runtime/js/99_main.js
+++ b/runtime/js/99_main.js
@@ -346,7 +346,8 @@ function promiseRejectCallback(type, promise, reason) {
}
return !!globalThis_.onunhandledrejection ||
- event.listenerCount(globalThis_, "unhandledrejection") > 0;
+ event.listenerCount(globalThis_, "unhandledrejection") > 0 ||
+ typeof internals.nodeProcessUnhandledRejectionCallback !== "undefined";
}
function promiseRejectMacrotaskCallback() {
@@ -383,6 +384,15 @@ function promiseRejectMacrotaskCallback() {
globalThis_.dispatchEvent(rejectionEvent);
globalThis_.removeEventListener("error", errorEventCb);
+ // If event was not yet prevented, try handing it off to Node compat layer
+ // (if it was initialized)
+ if (
+ !rejectionEvent.defaultPrevented &&
+ typeof internals.nodeProcessUnhandledRejectionCallback !== "undefined"
+ ) {
+ internals.nodeProcessUnhandledRejectionCallback(rejectionEvent);
+ }
+
// If event was not prevented (or "unhandledrejection" listeners didn't
// throw) we will let Rust side handle it.
if (rejectionEvent.defaultPrevented) {