Skip to content

Commit

Permalink
fix(node): fire 'unhandledrejection' event when using node: or npm: i…
Browse files Browse the repository at this point in the history
…mports (#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 #16928
  • Loading branch information
bartlomieju authored May 24, 2023
1 parent 787e1f0 commit 0bb5bbc
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 5 deletions.
18 changes: 18 additions & 0 deletions cli/tests/integration/node_unit_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
});
17 changes: 17 additions & 0 deletions cli/tests/testdata/node/unhandled_rejection_web.ts
Original file line number Diff line number Diff line change
@@ -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);
4 changes: 4 additions & 0 deletions cli/tests/testdata/node/unhandled_rejection_web.ts.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[WILDCARD]
Hello world!
Handled the promise rejection
Success
21 changes: 21 additions & 0 deletions cli/tests/testdata/node/unhandled_rejection_web_process.ts
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[WILDCARD]
Hello world!
globalThis.addEventListener("unhandledrejection");
process.on("unhandledRejection");
Success
8 changes: 4 additions & 4 deletions ext/node/polyfills/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
12 changes: 11 additions & 1 deletion runtime/js/99_main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 0bb5bbc

Please sign in to comment.