Skip to content

Commit 7698bcc

Browse files
bartlomiejuclaude
andauthored
fix(runtime): display proper error when throwing in event handlers (#32663)
## Summary - Fixes `"Uncaught null"` being displayed instead of the actual error when throwing in `unload`, `beforeunload`, `load`, or process `exit`/`beforeExit` event handlers - Root cause: `reportException` in JS calls `op_dispatch_exception` which stores the real exception in `ExceptionState` and terminates execution. The Rust-side dispatch functions then checked `tc_scope.exception()` which returned `null` (the termination exception) instead of the stored one - Fix: use `exception_to_err` which properly checks `ExceptionState` for a dispatched exception before falling back to `tc_scope.exception()` Before: ``` error: Uncaught null ``` After: ``` error: Uncaught Error: foo globalThis.addEventListener("unload", () => { throw new Error("foo"); }); ^ at file:///...$deno$eval.mts:1:53 at innerInvokeEventListeners (ext:deno_web/02_event.js:781:9) ... ``` Closes #29087 --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 9eed28a commit 7698bcc

File tree

7 files changed

+45
-11
lines changed

7 files changed

+45
-11
lines changed

libs/core/error.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1313,7 +1313,7 @@ pub(crate) fn exception_to_err_result<'s, 'i, T>(
13131313
Err(exception_to_err(scope, exception, in_promise, clear_error))
13141314
}
13151315

1316-
pub(crate) fn exception_to_err<'s, 'i>(
1316+
pub fn exception_to_err<'s, 'i>(
13171317
scope: &mut v8::PinScope<'s, 'i>,
13181318
exception: v8::Local<'s, v8::Value>,
13191319
mut in_promise: bool,

libs/core/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ pub use crate::convert::FromV8;
9090
pub use crate::convert::FromV8Scopeless;
9191
pub use crate::convert::ToV8;
9292
pub use crate::cppgc::GarbageCollected;
93+
pub use crate::error::exception_to_err;
9394
pub use crate::extensions::AccessorType;
9495
pub use crate::extensions::Extension;
9596
pub use crate::extensions::ExtensionArguments;

runtime/worker.rs

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -946,8 +946,9 @@ impl MainWorker {
946946
let undefined = v8::undefined(tc_scope);
947947
dispatch_load_event_fn.call(tc_scope, undefined.into(), &[]);
948948
if let Some(exception) = tc_scope.exception() {
949-
let error = JsError::from_v8_exception(tc_scope, exception);
950-
return Err(error);
949+
return Err(deno_core::exception_to_err(
950+
tc_scope, exception, false, true,
951+
));
951952
}
952953
Ok(())
953954
}
@@ -963,8 +964,9 @@ impl MainWorker {
963964
let undefined = v8::undefined(tc_scope);
964965
dispatch_unload_event_fn.call(tc_scope, undefined.into(), &[]);
965966
if let Some(exception) = tc_scope.exception() {
966-
let error = JsError::from_v8_exception(tc_scope, exception);
967-
return Err(error);
967+
return Err(deno_core::exception_to_err(
968+
tc_scope, exception, false, true,
969+
));
968970
}
969971
Ok(())
970972
}
@@ -978,8 +980,9 @@ impl MainWorker {
978980
let undefined = v8::undefined(tc_scope);
979981
dispatch_process_exit_event_fn.call(tc_scope, undefined.into(), &[]);
980982
if let Some(exception) = tc_scope.exception() {
981-
let error = JsError::from_v8_exception(tc_scope, exception);
982-
return Err(error);
983+
return Err(deno_core::exception_to_err(
984+
tc_scope, exception, false, true,
985+
));
983986
}
984987
Ok(())
985988
}
@@ -1032,8 +1035,9 @@ impl MainWorker {
10321035
let ret_val =
10331036
dispatch_beforeunload_event_fn.call(tc_scope, undefined.into(), &[]);
10341037
if let Some(exception) = tc_scope.exception() {
1035-
let error = JsError::from_v8_exception(tc_scope, exception);
1036-
return Err(error);
1038+
return Err(deno_core::exception_to_err(
1039+
tc_scope, exception, false, true,
1040+
));
10371041
}
10381042
let ret_val = ret_val.unwrap();
10391043
Ok(ret_val.is_false())
@@ -1056,8 +1060,9 @@ impl MainWorker {
10561060
&[],
10571061
);
10581062
if let Some(exception) = tc_scope.exception() {
1059-
let error = JsError::from_v8_exception(tc_scope, exception);
1060-
return Err(error);
1063+
return Err(deno_core::exception_to_err(
1064+
tc_scope, exception, false, true,
1065+
));
10611066
}
10621067
let ret_val = ret_val.unwrap();
10631068
Ok(ret_val.is_true())
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"tests": {
3+
"throw_in_unload": {
4+
"args": "run throw_in_unload.ts",
5+
"output": "error: Uncaught Error: unload error\n[WILDCARD]",
6+
"exitCode": 1
7+
},
8+
"throw_in_beforeunload": {
9+
"args": "run throw_in_beforeunload.ts",
10+
"output": "error: Uncaught Error: beforeunload error\n[WILDCARD]",
11+
"exitCode": 1
12+
},
13+
"throw_in_load": {
14+
"args": "run throw_in_load.ts",
15+
"output": "error: Uncaught Error: load error\n[WILDCARD]",
16+
"exitCode": 1
17+
}
18+
}
19+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
globalThis.addEventListener("beforeunload", () => {
2+
throw new Error("beforeunload error");
3+
});
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
globalThis.addEventListener("load", () => {
2+
throw new Error("load error");
3+
});
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
globalThis.addEventListener("unload", () => {
2+
throw new Error("unload error");
3+
});

0 commit comments

Comments
 (0)