diff options
author | Bartek IwaĆczuk <biwanczuk@gmail.com> | 2023-05-24 15:40:41 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-05-24 15:40:41 +0200 |
commit | 0bb5bbc7a0ff7565a4c7fa4ebc8c69e02f76e6b5 (patch) | |
tree | a5ab85f34d7984428288cb8d221cf09851d333bd | |
parent | 787e1f0f92e81f025ec0dfdce2acff15900267fc (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.rs | 18 | ||||
-rw-r--r-- | cli/tests/testdata/node/unhandled_rejection_web.ts | 17 | ||||
-rw-r--r-- | cli/tests/testdata/node/unhandled_rejection_web.ts.out | 4 | ||||
-rw-r--r-- | cli/tests/testdata/node/unhandled_rejection_web_process.ts | 21 | ||||
-rw-r--r-- | cli/tests/testdata/node/unhandled_rejection_web_process.ts.out | 5 | ||||
-rw-r--r-- | ext/node/polyfills/process.ts | 8 | ||||
-rw-r--r-- | runtime/js/99_main.js | 12 |
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) { |