From d53936eb7d3fe4cda8e06f7310e4c8f12702b413 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Wed, 20 Jul 2022 20:28:19 +0200 Subject: [PATCH] Reland "feat: add "unhandledrejection" event support" (#15211) --- cli/bench/main.rs | 1 + cli/dts/lib.deno.shared_globals.d.ts | 11 +++ cli/tests/integration/run_tests.rs | 5 ++ cli/tests/testdata/unhandled_rejection.js | 11 +++ cli/tests/testdata/unhandled_rejection.js.out | 8 ++ core/01_core.js | 4 + core/bindings.rs | 51 ++++++++----- core/ops_builtin_v8.rs | 48 ++++++++++++ ext/web/02_event.js | 53 +++++++++++++ runtime/js/99_main.js | 75 +++++++++++++++++++ tools/wpt/expectation.json | 1 - 11 files changed, 249 insertions(+), 19 deletions(-) create mode 100644 cli/tests/testdata/unhandled_rejection.js create mode 100644 cli/tests/testdata/unhandled_rejection.js.out diff --git a/cli/bench/main.rs b/cli/bench/main.rs index e4cbf3eda9..0ef520edde 100644 --- a/cli/bench/main.rs +++ b/cli/bench/main.rs @@ -174,6 +174,7 @@ fn run_exec_time( benchmark_file, "--warmup", "3", + "--show-output", ] .iter() .map(|s| s.to_string()) diff --git a/cli/dts/lib.deno.shared_globals.d.ts b/cli/dts/lib.deno.shared_globals.d.ts index fc40bb54ea..abf5e41fb5 100644 --- a/cli/dts/lib.deno.shared_globals.d.ts +++ b/cli/dts/lib.deno.shared_globals.d.ts @@ -400,6 +400,17 @@ declare class ErrorEvent extends Event { constructor(type: string, eventInitDict?: ErrorEventInit); } +interface PromiseRejectionEventInit extends EventInit { + promise: Promise; + reason?: any; +} + +declare class PromiseRejectionEvent extends Event { + readonly promise: Promise; + readonly reason: any; + constructor(type: string, eventInitDict?: PromiseRejectionEventInit); +} + interface AbstractWorkerEventMap { "error": ErrorEvent; } diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 96e4cfce50..d477bb8964 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -2782,3 +2782,8 @@ itest!(followup_dyn_import_resolved { args: "run --unstable --allow-read followup_dyn_import_resolves/main.ts", output: "followup_dyn_import_resolves/main.ts.out", }); + +itest!(unhandled_rejection { + args: "run --allow-read unhandled_rejection.js", + output: "unhandled_rejection.js.out", +}); diff --git a/cli/tests/testdata/unhandled_rejection.js b/cli/tests/testdata/unhandled_rejection.js new file mode 100644 index 0000000000..352e861b46 --- /dev/null +++ b/cli/tests/testdata/unhandled_rejection.js @@ -0,0 +1,11 @@ +globalThis.addEventListener("unhandledrejection", (e) => { + console.log("unhandled rejection at:", e.promise, "reason:", e.reason); + e.preventDefault(); +}); + +function Foo() { + this.bar = Promise.reject(new Error("bar not available")); +} + +new Foo(); +Promise.reject(); diff --git a/cli/tests/testdata/unhandled_rejection.js.out b/cli/tests/testdata/unhandled_rejection.js.out new file mode 100644 index 0000000000..4c41795ce5 --- /dev/null +++ b/cli/tests/testdata/unhandled_rejection.js.out @@ -0,0 +1,8 @@ +unhandled rejection at: Promise { + Error: bar not available + at new Foo (file:///[WILDCARD]/testdata/unhandled_rejection.js:7:29) + at file:///[WILDCARD]/testdata/unhandled_rejection.js:10:1 +} reason: Error: bar not available + at new Foo (file:///[WILDCARD]/testdata/unhandled_rejection.js:7:29) + at file:///[WILDCARD]/testdata/unhandled_rejection.js:10:1 +unhandled rejection at: Promise { undefined } reason: undefined diff --git a/core/01_core.js b/core/01_core.js index 1563003274..21376b695b 100644 --- a/core/01_core.js +++ b/core/01_core.js @@ -268,6 +268,10 @@ terminate: opSync.bind(null, "op_terminate"), opNames: opSync.bind(null, "op_op_names"), eventLoopHasMoreWork: opSync.bind(null, "op_event_loop_has_more_work"), + setPromiseRejectCallback: opSync.bind( + null, + "op_set_promise_reject_callback", + ), }); ObjectAssign(globalThis.__bootstrap, { core }); diff --git a/core/bindings.rs b/core/bindings.rs index 66c3e436de..6fa9f745b0 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -323,14 +323,6 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) { let state_rc = JsRuntime::state(scope); let mut state = state_rc.borrow_mut(); - // Node compat: perform synchronous process.emit("unhandledRejection"). - // - // Note the callback follows the (type, promise, reason) signature of Node's - // internal promiseRejectHandler from lib/internal/process/promises.js, not - // the (promise, reason) signature of the "unhandledRejection" event listener. - // - // Short-circuits Deno's regular unhandled rejection logic because that's - // a) asynchronous, and b) always terminates. if let Some(js_promise_reject_cb) = state.js_promise_reject_cb.clone() { let js_uncaught_exception_cb = state.js_uncaught_exception_cb.clone(); @@ -338,14 +330,6 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) { let undefined: v8::Local = v8::undefined(tc_scope).into(); let type_ = v8::Integer::new(tc_scope, message.get_event() as i32); let promise = message.get_promise(); - if let Some(pending_mod_evaluate) = state.pending_mod_evaluate.as_mut() { - if !pending_mod_evaluate.has_evaluated { - let promise_global = v8::Global::new(tc_scope, promise); - pending_mod_evaluate - .handled_promise_rejections - .push(promise_global); - } - } drop(state); // Drop borrow, callbacks can call back into runtime. let reason = match message.get_event() { @@ -355,14 +339,45 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) { PromiseHandlerAddedAfterReject => undefined, }; + let promise_global = v8::Global::new(tc_scope, promise); let args = &[type_.into(), promise.into(), reason]; - js_promise_reject_cb + let maybe_has_unhandled_rejection_handler = js_promise_reject_cb .open(tc_scope) .call(tc_scope, undefined, args); + let has_unhandled_rejection_handler = + if let Some(value) = maybe_has_unhandled_rejection_handler { + value.is_true() + } else { + false + }; + + if has_unhandled_rejection_handler { + let mut state = state_rc.borrow_mut(); + if let Some(pending_mod_evaluate) = state.pending_mod_evaluate.as_mut() { + if !pending_mod_evaluate.has_evaluated { + pending_mod_evaluate + .handled_promise_rejections + .push(promise_global.clone()); + } + } + } + if let Some(exception) = tc_scope.exception() { if let Some(js_uncaught_exception_cb) = js_uncaught_exception_cb { tc_scope.reset(); // Cancel pending exception. + { + let mut state = state_rc.borrow_mut(); + if let Some(pending_mod_evaluate) = + state.pending_mod_evaluate.as_mut() + { + if !pending_mod_evaluate.has_evaluated { + pending_mod_evaluate + .handled_promise_rejections + .push(promise_global); + } + } + } js_uncaught_exception_cb.open(tc_scope).call( tc_scope, undefined, @@ -372,6 +387,7 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) { } if tc_scope.has_caught() { + // TODO(bartlomieju): ensure that TODO provided below is still valid. // If we get here, an exception was thrown by the unhandledRejection // handler and there is ether no uncaughtException handler or the // handler threw an exception of its own. @@ -389,7 +405,6 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) { } else { let promise = message.get_promise(); let promise_global = v8::Global::new(scope, promise); - match message.get_event() { PromiseRejectWithNoHandler => { let error = message.get_value().unwrap(); diff --git a/core/ops_builtin_v8.rs b/core/ops_builtin_v8.rs index 39469c0adc..a17e273f39 100644 --- a/core/ops_builtin_v8.rs +++ b/core/ops_builtin_v8.rs @@ -48,6 +48,9 @@ pub(crate) fn init_builtins_v8() -> Vec { op_apply_source_map::decl(), op_set_format_exception_callback::decl(), op_event_loop_has_more_work::decl(), + op_store_pending_promise_exception::decl(), + op_remove_pending_promise_exception::decl(), + op_has_pending_promise_exception::decl(), ] } @@ -792,3 +795,48 @@ fn op_set_format_exception_callback<'a>( fn op_event_loop_has_more_work(scope: &mut v8::HandleScope) -> bool { JsRuntime::event_loop_pending_state(scope).is_pending() } + +#[op(v8)] +fn op_store_pending_promise_exception<'a>( + scope: &mut v8::HandleScope<'a>, + promise: serde_v8::Value<'a>, + reason: serde_v8::Value<'a>, +) { + let state_rc = JsRuntime::state(scope); + let mut state = state_rc.borrow_mut(); + let promise_value = + v8::Local::::try_from(promise.v8_value).unwrap(); + let promise_global = v8::Global::new(scope, promise_value); + let error_global = v8::Global::new(scope, reason.v8_value); + state + .pending_promise_exceptions + .insert(promise_global, error_global); +} + +#[op(v8)] +fn op_remove_pending_promise_exception<'a>( + scope: &mut v8::HandleScope<'a>, + promise: serde_v8::Value<'a>, +) { + let state_rc = JsRuntime::state(scope); + let mut state = state_rc.borrow_mut(); + let promise_value = + v8::Local::::try_from(promise.v8_value).unwrap(); + let promise_global = v8::Global::new(scope, promise_value); + state.pending_promise_exceptions.remove(&promise_global); +} + +#[op(v8)] +fn op_has_pending_promise_exception<'a>( + scope: &mut v8::HandleScope<'a>, + promise: serde_v8::Value<'a>, +) -> bool { + let state_rc = JsRuntime::state(scope); + let state = state_rc.borrow(); + let promise_value = + v8::Local::::try_from(promise.v8_value).unwrap(); + let promise_global = v8::Global::new(scope, promise_value); + state + .pending_promise_exceptions + .contains_key(&promise_global) +} diff --git a/ext/web/02_event.js b/ext/web/02_event.js index 5d8f69673c..105c7d3c41 100644 --- a/ext/web/02_event.js +++ b/ext/web/02_event.js @@ -1278,6 +1278,58 @@ [SymbolToStringTag] = "ProgressEvent"; } + class PromiseRejectionEvent extends Event { + #promise = null; + #reason = null; + + get promise() { + return this.#promise; + } + get reason() { + return this.#reason; + } + + constructor( + type, + { + bubbles, + cancelable, + composed, + promise, + reason, + } = {}, + ) { + super(type, { + bubbles: bubbles, + cancelable: cancelable, + composed: composed, + }); + + this.#promise = promise; + this.#reason = reason; + } + + [SymbolFor("Deno.privateCustomInspect")](inspect) { + return inspect(consoleInternal.createFilteredInspectProxy({ + object: this, + evaluate: this instanceof PromiseRejectionEvent, + keys: [ + ...EVENT_PROPS, + "promise", + "reason", + ], + })); + } + + // TODO(lucacasonato): remove when this interface is spec aligned + [SymbolToStringTag] = "PromiseRejectionEvent"; + } + + defineEnumerableProps(PromiseRejectionEvent, [ + "promise", + "reason", + ]); + const _eventHandlers = Symbol("eventHandlers"); function makeWrappedHandler(handler, isSpecialErrorEventHandler) { @@ -1426,6 +1478,7 @@ window.MessageEvent = MessageEvent; window.CustomEvent = CustomEvent; window.ProgressEvent = ProgressEvent; + window.PromiseRejectionEvent = PromiseRejectionEvent; window.dispatchEvent = EventTarget.prototype.dispatchEvent; window.addEventListener = EventTarget.prototype.addEventListener; window.removeEventListener = EventTarget.prototype.removeEventListener; diff --git a/runtime/js/99_main.js b/runtime/js/99_main.js index c13faa9362..3421528d20 100644 --- a/runtime/js/99_main.js +++ b/runtime/js/99_main.js @@ -11,6 +11,10 @@ delete Intl.v8BreakIterator; ((window) => { const core = Deno.core; const { + ArrayPrototypeIndexOf, + ArrayPrototypePush, + ArrayPrototypeShift, + ArrayPrototypeSplice, ArrayPrototypeMap, DateNow, Error, @@ -27,7 +31,11 @@ delete Intl.v8BreakIterator; SymbolFor, SymbolIterator, PromisePrototypeThen, + SafeWeakMap, TypeError, + WeakMapPrototypeDelete, + WeakMapPrototypeGet, + WeakMapPrototypeSet, } = window.__bootstrap.primordials; const util = window.__bootstrap.util; const eventTarget = window.__bootstrap.eventTarget; @@ -233,6 +241,7 @@ delete Intl.v8BreakIterator; function runtimeStart(runtimeOptions, source) { core.setMacrotaskCallback(timers.handleTimerMacrotask); + core.setMacrotaskCallback(promiseRejectMacrotaskCallback); core.setWasmStreamingCallback(fetch.handleWasmStreaming); core.opSync("op_set_format_exception_callback", formatException); version.setVersions( @@ -411,6 +420,7 @@ delete Intl.v8BreakIterator; PerformanceEntry: util.nonEnumerable(performance.PerformanceEntry), PerformanceMark: util.nonEnumerable(performance.PerformanceMark), PerformanceMeasure: util.nonEnumerable(performance.PerformanceMeasure), + PromiseRejectionEvent: util.nonEnumerable(PromiseRejectionEvent), ProgressEvent: util.nonEnumerable(ProgressEvent), ReadableStream: util.nonEnumerable(streams.ReadableStream), ReadableStreamDefaultReader: util.nonEnumerable( @@ -553,6 +563,63 @@ delete Intl.v8BreakIterator; postMessage: util.writable(postMessage), }; + const pendingRejections = []; + const pendingRejectionsReasons = new SafeWeakMap(); + + function promiseRejectCallback(type, promise, reason) { + switch (type) { + case 0: { + core.opSync("op_store_pending_promise_exception", promise, reason); + ArrayPrototypePush(pendingRejections, promise); + WeakMapPrototypeSet(pendingRejectionsReasons, promise, reason); + break; + } + case 1: { + core.opSync("op_remove_pending_promise_exception", promise); + const index = ArrayPrototypeIndexOf(pendingRejections, promise); + if (index > -1) { + ArrayPrototypeSplice(pendingRejections, index, 1); + WeakMapPrototypeDelete(pendingRejectionsReasons, promise); + } + break; + } + default: + return false; + } + + return !!globalThis.onunhandledrejection || + eventTarget.listenerCount(globalThis, "unhandledrejection") > 0; + } + + function promiseRejectMacrotaskCallback() { + while (pendingRejections.length > 0) { + const promise = ArrayPrototypeShift(pendingRejections); + const hasPendingException = core.opSync( + "op_has_pending_promise_exception", + promise, + ); + const reason = WeakMapPrototypeGet(pendingRejectionsReasons, promise); + WeakMapPrototypeDelete(pendingRejectionsReasons, promise); + + if (!hasPendingException) { + return; + } + + const event = new PromiseRejectionEvent("unhandledrejection", { + cancelable: true, + promise, + reason, + }); + globalThis.dispatchEvent(event); + + // If event was not prevented we will let Rust side handle it. + if (event.defaultPrevented) { + core.opSync("op_remove_pending_promise_exception", promise); + } + } + return true; + } + let hasBootstrapped = false; function bootstrapMainRuntime(runtimeOptions) { @@ -585,6 +652,10 @@ delete Intl.v8BreakIterator; defineEventHandler(window, "load"); defineEventHandler(window, "beforeunload"); defineEventHandler(window, "unload"); + defineEventHandler(window, "unhandledrejection"); + + core.setPromiseRejectCallback(promiseRejectCallback); + const isUnloadDispatched = SymbolFor("isUnloadDispatched"); // Stores the flag for checking whether unload is dispatched or not. // This prevents the recursive dispatches of unload events. @@ -685,6 +756,10 @@ delete Intl.v8BreakIterator; defineEventHandler(self, "message"); defineEventHandler(self, "error", undefined, true); + defineEventHandler(self, "unhandledrejection"); + + core.setPromiseRejectCallback(promiseRejectCallback); + // `Deno.exit()` is an alias to `self.close()`. Setting and exit // code using an op in worker context is a no-op. os.setExitHandler((_exitCode) => { diff --git a/tools/wpt/expectation.json b/tools/wpt/expectation.json index 56b0b11992..0d53dd962a 100644 --- a/tools/wpt/expectation.json +++ b/tools/wpt/expectation.json @@ -4360,7 +4360,6 @@ "The CanvasPath interface object should be exposed.", "The TextMetrics interface object should be exposed.", "The Path2D interface object should be exposed.", - "The PromiseRejectionEvent interface object should be exposed.", "The EventSource interface object should be exposed.", "The XMLHttpRequestEventTarget interface object should be exposed.", "The XMLHttpRequestUpload interface object should be exposed.",