summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBartek IwaƄczuk <biwanczuk@gmail.com>2022-12-23 19:46:24 +0100
committerGitHub <noreply@github.com>2022-12-23 19:46:24 +0100
commit208c91b68f4ae1b59e65acbde3de729e7058bb5e (patch)
treeaa0faab20715baee85b571b151426bf832231eb7
parent2a61b5fdd444c4b6f47f0e0bfbafe0bd26789d68 (diff)
fix(core): run macrotasks and next ticks after polling dynamic imports (#17173)
This commit fixes handling of rejected promises in dynamic imports evaluation. Previously we were running callbacks for next ticks and macrotasks _before_ polling dynamic imports and checked for unhandled rejections immediately after. This is wrong, as `unhandledrejection` event is dispatched and its callbacks are run as macrotasks. This commit changes order of actions performed by the event loop to following: - poll async ops - poll dynamic imports - run next tick callbacks - run macrotask callbacks - check for unhandled promise rejections
-rw-r--r--cli/tests/run_tests.rs13
-rw-r--r--cli/tests/testdata/run/unhandled_rejection_dynamic_import/import.ts5
-rw-r--r--cli/tests/testdata/run/unhandled_rejection_dynamic_import/main.ts1
-rw-r--r--cli/tests/testdata/run/unhandled_rejection_dynamic_import/main.ts.out3
-rw-r--r--cli/tests/testdata/run/unhandled_rejection_dynamic_import2/import.ts3
-rw-r--r--cli/tests/testdata/run/unhandled_rejection_dynamic_import2/main.ts21
-rw-r--r--cli/tests/testdata/run/unhandled_rejection_dynamic_import2/main.ts.out5
-rw-r--r--core/runtime.rs15
8 files changed, 58 insertions, 8 deletions
diff --git a/cli/tests/run_tests.rs b/cli/tests/run_tests.rs
index 092546aaf..d90a48fbb 100644
--- a/cli/tests/run_tests.rs
+++ b/cli/tests/run_tests.rs
@@ -2945,6 +2945,19 @@ mod run {
output: "run/unhandled_rejection_sync_error.ts.out",
});
+ // Regression test for https://github.com/denoland/deno/issues/15661
+ itest!(unhandled_rejection_dynamic_import {
+ args: "run --allow-read run/unhandled_rejection_dynamic_import/main.ts",
+ output: "run/unhandled_rejection_dynamic_import/main.ts.out",
+ exit_code: 1,
+ });
+
+ // Regression test for https://github.com/denoland/deno/issues/16909
+ itest!(unhandled_rejection_dynamic_import2 {
+ args: "run --allow-read run/unhandled_rejection_dynamic_import2/main.ts",
+ output: "run/unhandled_rejection_dynamic_import2/main.ts.out",
+ });
+
itest!(nested_error {
args: "run run/nested_error.ts",
output: "run/nested_error.ts.out",
diff --git a/cli/tests/testdata/run/unhandled_rejection_dynamic_import/import.ts b/cli/tests/testdata/run/unhandled_rejection_dynamic_import/import.ts
new file mode 100644
index 000000000..b490f2c20
--- /dev/null
+++ b/cli/tests/testdata/run/unhandled_rejection_dynamic_import/import.ts
@@ -0,0 +1,5 @@
+globalThis.addEventListener("unhandledrejection", () => {
+ console.log("hey");
+});
+console.log("---");
+Promise.reject();
diff --git a/cli/tests/testdata/run/unhandled_rejection_dynamic_import/main.ts b/cli/tests/testdata/run/unhandled_rejection_dynamic_import/main.ts
new file mode 100644
index 000000000..244d84467
--- /dev/null
+++ b/cli/tests/testdata/run/unhandled_rejection_dynamic_import/main.ts
@@ -0,0 +1 @@
+await import("./import.ts");
diff --git a/cli/tests/testdata/run/unhandled_rejection_dynamic_import/main.ts.out b/cli/tests/testdata/run/unhandled_rejection_dynamic_import/main.ts.out
new file mode 100644
index 000000000..f44c6d2ff
--- /dev/null
+++ b/cli/tests/testdata/run/unhandled_rejection_dynamic_import/main.ts.out
@@ -0,0 +1,3 @@
+---
+hey
+error: Uncaught (in promise) undefined
diff --git a/cli/tests/testdata/run/unhandled_rejection_dynamic_import2/import.ts b/cli/tests/testdata/run/unhandled_rejection_dynamic_import2/import.ts
new file mode 100644
index 000000000..f84297afd
--- /dev/null
+++ b/cli/tests/testdata/run/unhandled_rejection_dynamic_import2/import.ts
@@ -0,0 +1,3 @@
+export default {
+ a: "a",
+};
diff --git a/cli/tests/testdata/run/unhandled_rejection_dynamic_import2/main.ts b/cli/tests/testdata/run/unhandled_rejection_dynamic_import2/main.ts
new file mode 100644
index 000000000..3da2e1d19
--- /dev/null
+++ b/cli/tests/testdata/run/unhandled_rejection_dynamic_import2/main.ts
@@ -0,0 +1,21 @@
+globalThis.addEventListener("unhandledrejection", (e) => {
+ console.log("unhandled rejection", e.reason);
+ e.preventDefault();
+});
+
+const dummyImport = (await import("./import.ts")).default;
+
+let a = new Promise((resolve, reject) => {
+ throw "errA";
+});
+
+let i = 0;
+while (true) {
+ await new Promise((resolve) => setTimeout(resolve, 100));
+ i++;
+ console.log("running...");
+
+ if (i > 3) {
+ break;
+ }
+}
diff --git a/cli/tests/testdata/run/unhandled_rejection_dynamic_import2/main.ts.out b/cli/tests/testdata/run/unhandled_rejection_dynamic_import2/main.ts.out
new file mode 100644
index 000000000..0a913a4b5
--- /dev/null
+++ b/cli/tests/testdata/run/unhandled_rejection_dynamic_import2/main.ts.out
@@ -0,0 +1,5 @@
+unhandled rejection errA
+running...
+running...
+running...
+running...
diff --git a/core/runtime.rs b/core/runtime.rs
index 2e417bb9d..f735f5575 100644
--- a/core/runtime.rs
+++ b/core/runtime.rs
@@ -1127,12 +1127,7 @@ impl JsRuntime {
self.pump_v8_message_loop()?;
// Ops
- {
- self.resolve_async_ops(cx)?;
- self.drain_nexttick()?;
- self.drain_macrotasks()?;
- self.check_promise_exceptions()?;
- }
+ self.resolve_async_ops(cx)?;
// Dynamic module loading - ie. modules loaded using "import()"
{
// Run in a loop so that dynamic imports that only depend on another
@@ -1157,9 +1152,13 @@ impl JsRuntime {
break;
}
}
-
- self.check_promise_exceptions()?;
}
+ // Run all next tick callbacks and macrotasks callbacks and only then
+ // check for any promise exceptions (`unhandledrejection` handlers are
+ // run in macrotasks callbacks so we need to let them run first).
+ self.drain_nexttick()?;
+ self.drain_macrotasks()?;
+ self.check_promise_exceptions()?;
// Event loop middlewares
let mut maybe_scheduling = false;