From 06934db883e9ae64c7603f98051676cbcf90994f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 4 Jul 2022 23:34:39 +0200 Subject: [PATCH] Revert "feat: add "unhandledrejection" event support (#12994)" (#15075) This reverts commit f7af0b01a59aaac91473e2f920137004d39a310a. --- 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 | 10 +++- core/ops_builtin.rs | 6 --- core/ops_builtin_v8.rs | 48 ----------------- ext/web/02_event.js | 53 ------------------- runtime/js/99_main.js | 46 ---------------- tools/wpt/expectation.json | 1 + 11 files changed, 10 insertions(+), 193 deletions(-) delete mode 100644 cli/tests/testdata/unhandled_rejection.js delete mode 100644 cli/tests/testdata/unhandled_rejection.js.out diff --git a/cli/dts/lib.deno.shared_globals.d.ts b/cli/dts/lib.deno.shared_globals.d.ts index 13843319f0..efd59ca2ee 100644 --- a/cli/dts/lib.deno.shared_globals.d.ts +++ b/cli/dts/lib.deno.shared_globals.d.ts @@ -400,17 +400,6 @@ 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 7ac577d024..1cd1db0ef6 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -2744,8 +2744,3 @@ 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 deleted file mode 100644 index 352e861b46..0000000000 --- a/cli/tests/testdata/unhandled_rejection.js +++ /dev/null @@ -1,11 +0,0 @@ -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 deleted file mode 100644 index 4c41795ce5..0000000000 --- a/cli/tests/testdata/unhandled_rejection.js.out +++ /dev/null @@ -1,8 +0,0 @@ -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 21376b695b..1563003274 100644 --- a/core/01_core.js +++ b/core/01_core.js @@ -268,10 +268,6 @@ 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 47632d00dd..a88e54af70 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -282,6 +282,14 @@ 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(); drop(state); // Drop borrow, callbacks can call back into runtime. @@ -315,7 +323,6 @@ 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. @@ -333,6 +340,7 @@ 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.rs b/core/ops_builtin.rs index 53f2bd7064..a42c0bae88 100644 --- a/core/ops_builtin.rs +++ b/core/ops_builtin.rs @@ -38,7 +38,6 @@ pub(crate) fn init_builtins() -> Extension { op_metrics::decl(), op_format_file_name::decl(), op_is_proxy::decl(), - op_next_task::decl(), ]) .ops(crate::ops_builtin_v8::init_builtins_v8()) .build() @@ -190,8 +189,3 @@ fn op_format_file_name(file_name: String) -> String { fn op_is_proxy(value: serde_v8::Value) -> bool { value.v8_value.is_proxy() } - -/// Empty op that when awaited forces a macrotask to run. Useful for -/// "unhandledrejection" event. -#[op] -async fn op_next_task() {} diff --git a/core/ops_builtin_v8.rs b/core/ops_builtin_v8.rs index 8af50f860c..4bc80faa55 100644 --- a/core/ops_builtin_v8.rs +++ b/core/ops_builtin_v8.rs @@ -48,9 +48,6 @@ 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(), ] } @@ -813,48 +810,3 @@ fn op_event_loop_has_more_work(scope: &mut v8::HandleScope) -> bool { || has_pending_background_tasks || has_tick_scheduled } - -#[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 105c7d3c41..5d8f69673c 100644 --- a/ext/web/02_event.js +++ b/ext/web/02_event.js @@ -1278,58 +1278,6 @@ [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) { @@ -1478,7 +1426,6 @@ 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 ecda556ebd..c13faa9362 100644 --- a/runtime/js/99_main.js +++ b/runtime/js/99_main.js @@ -411,7 +411,6 @@ 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( @@ -554,43 +553,6 @@ delete Intl.v8BreakIterator; postMessage: util.writable(postMessage), }; - function promiseRejectCallback(type, promise, reason) { - switch (type) { - case 0: { - core.opSync("op_store_pending_promise_exception", promise, reason); - break; - } - case 1: { - core.opSync("op_remove_pending_promise_exception", promise); - break; - } - default: - return; - } - core.opAsync("op_next_task").then(() => { - const hasPendingException = core.opSync( - "op_has_pending_promise_exception", - 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); - } - }); - } - let hasBootstrapped = false; function bootstrapMainRuntime(runtimeOptions) { @@ -623,10 +585,6 @@ 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. @@ -727,10 +685,6 @@ 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 0d53dd962a..56b0b11992 100644 --- a/tools/wpt/expectation.json +++ b/tools/wpt/expectation.json @@ -4360,6 +4360,7 @@ "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.",