refactor(core): remove unneeded ops for uncaughtException (#15296)

3 files changed, 7 insertions, 108 deletions

diff --git a/core/bindings.rs b/core/bindings.rs
index a93a3e746..aef479472 100644
--- a/core/bindings.rs
+++ b/core/bindings.rs
@@ -324,8 +324,6 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) {
   let mut state = state_rc.borrow_mut();
 
   if let Some(js_promise_reject_cb) = state.js_promise_reject_cb.clone() {
-    let js_uncaught_exception_cb = state.js_uncaught_exception_cb.clone();
-
     let tc_scope = &mut v8::TryCatch::new(scope);
     let undefined: v8::Local<v8::Value> = v8::undefined(tc_scope).into();
     let type_ = v8::Integer::new(tc_scope, message.get_event() as i32);
@@ -358,52 +356,10 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) {
         if !pending_mod_evaluate.has_evaluated {
           pending_mod_evaluate
             .handled_promise_rejections
-            .push(promise_global.clone());
+            .push(promise_global);
         }
       }
     }
-
-    // TODO(bartlomieju): remove this whole block, `js_uncaught_exception_cb` is
-    // not needed anymore
-    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,
-          &[exception],
-        );
-      }
-    }
-
-    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.
-      //
-      // TODO(bnoordhuis) Node terminates the process or worker thread
-      // but we don't really have that option. The exception won't bubble
-      // up either because V8 cancels it when this function returns.
-      let exception = tc_scope
-        .stack_trace()
-        .or_else(|| tc_scope.exception())
-        .map(|value| value.to_rust_string_lossy(tc_scope))
-        .unwrap_or_else(|| "no exception".into());
-      eprintln!("Unhandled exception: {}", exception);
-    }
   } else {
     let promise = message.get_promise();
     let promise_global = v8::Global::new(scope, promise);
diff --git a/core/ops_builtin_v8.rs b/core/ops_builtin_v8.rs
index a17e273f3..fbbcdd440 100644
--- a/core/ops_builtin_v8.rs
+++ b/core/ops_builtin_v8.rs
@@ -26,7 +26,6 @@ pub(crate) fn init_builtins_v8() -> Vec<OpDecl> {
     op_set_macrotask_callback::decl(),
     op_set_next_tick_callback::decl(),
     op_set_promise_reject_callback::decl(),
-    op_set_uncaught_exception_callback::decl(),
     op_run_microtasks::decl(),
     op_has_tick_scheduled::decl(),
     op_set_has_tick_scheduled::decl(),
@@ -110,18 +109,6 @@ fn op_set_promise_reject_callback<'a>(
 }
 
 #[op(v8)]
-fn op_set_uncaught_exception_callback<'a>(
-  scope: &mut v8::HandleScope<'a>,
-  cb: serde_v8::Value,
-) -> Result<Option<serde_v8::Value<'a>>, Error> {
-  let cb = to_v8_fn(scope, cb)?;
-  let state_rc = JsRuntime::state(scope);
-  let old = state_rc.borrow_mut().js_uncaught_exception_cb.replace(cb);
-  let old = old.map(|v| v8::Local::new(scope, v));
-  Ok(old.map(|v| from_v8(scope, v.into()).unwrap()))
-}
-
-#[op(v8)]
 fn op_run_microtasks(scope: &mut v8::HandleScope) {
   scope.perform_microtask_checkpoint();
 }
diff --git a/core/runtime.rs b/core/runtime.rs
index 4568f2018..0176ff791 100644
--- a/core/runtime.rs
+++ b/core/runtime.rs
@@ -154,7 +154,6 @@ pub(crate) struct JsRuntimeState {
   pub(crate) js_macrotask_cbs: Vec<v8::Global<v8::Function>>,
   pub(crate) js_nexttick_cbs: Vec<v8::Global<v8::Function>>,
   pub(crate) js_promise_reject_cb: Option<v8::Global<v8::Function>>,
-  pub(crate) js_uncaught_exception_cb: Option<v8::Global<v8::Function>>,
   pub(crate) js_format_exception_cb: Option<v8::Global<v8::Function>>,
   pub(crate) has_tick_scheduled: bool,
   pub(crate) js_wasm_streaming_cb: Option<v8::Global<v8::Function>>,
@@ -394,7 +393,6 @@ impl JsRuntime {
       js_macrotask_cbs: vec![],
       js_nexttick_cbs: vec![],
       js_promise_reject_cb: None,
-      js_uncaught_exception_cb: None,
       js_format_exception_cb: None,
       has_tick_scheduled: false,
       js_wasm_streaming_cb: None,
@@ -3211,7 +3209,6 @@ assertEquals(1, notify_return_value);
   #[tokio::test]
   async fn test_set_promise_reject_callback() {
     static PROMISE_REJECT: AtomicUsize = AtomicUsize::new(0);
-    static UNCAUGHT_EXCEPTION: AtomicUsize = AtomicUsize::new(0);
 
     #[op]
     fn op_promise_reject() -> Result<(), AnyError> {
@@ -3219,17 +3216,8 @@ assertEquals(1, notify_return_value);
       Ok(())
     }
 
-    #[op]
-    fn op_uncaught_exception() -> Result<(), AnyError> {
-      UNCAUGHT_EXCEPTION.fetch_add(1, Ordering::Relaxed);
-      Ok(())
-    }
-
     let extension = Extension::builder()
-      .ops(vec![
-        op_promise_reject::decl(),
-        op_uncaught_exception::decl(),
-      ])
+      .ops(vec![op_promise_reject::decl()])
       .build();
 
     let mut runtime = JsRuntime::new(RuntimeOptions {
@@ -3249,23 +3237,17 @@ assertEquals(1, notify_return_value);
           if (reason.message !== "reject") {
             throw Error("unexpected reason: " + reason);
           }
+          Deno.core.opSync("op_store_pending_promise_exception", promise);
           Deno.core.opSync("op_promise_reject");
-          throw Error("promiseReject"); // Triggers uncaughtException handler.
-        });
-
-        Deno.core.opSync("op_set_uncaught_exception_callback", (err) => {
-          if (err.message !== "promiseReject") throw err;
-          Deno.core.opSync("op_uncaught_exception");
         });
 
         new Promise((_, reject) => reject(Error("reject")));
         "#,
       )
       .unwrap();
-    runtime.run_event_loop(false).await.unwrap();
+    runtime.run_event_loop(false).await.unwrap_err();
 
     assert_eq!(1, PROMISE_REJECT.load(Ordering::Relaxed));
-    assert_eq!(1, UNCAUGHT_EXCEPTION.load(Ordering::Relaxed));
 
     runtime
       .execute_script(
@@ -3277,29 +3259,18 @@ assertEquals(1, notify_return_value);
           });
         }
 
-        {
-          const prev = Deno.core.opSync("op_set_uncaught_exception_callback", (...args) => {
-            prev(...args);
-            throw Error("fail");
-          });
-        }
-
         new Promise((_, reject) => reject(Error("reject")));
         "#,
       )
       .unwrap();
-    // Exception from uncaughtException handler doesn't bubble up but is
-    // printed to stderr.
-    runtime.run_event_loop(false).await.unwrap();
+    runtime.run_event_loop(false).await.unwrap_err();
 
     assert_eq!(2, PROMISE_REJECT.load(Ordering::Relaxed));
-    assert_eq!(2, UNCAUGHT_EXCEPTION.load(Ordering::Relaxed));
   }
 
   #[tokio::test]
   async fn test_set_promise_reject_callback_top_level_await() {
     static PROMISE_REJECT: AtomicUsize = AtomicUsize::new(0);
-    static UNCAUGHT_EXCEPTION: AtomicUsize = AtomicUsize::new(0);
 
     #[op]
     fn op_promise_reject() -> Result<(), AnyError> {
@@ -3307,17 +3278,8 @@ assertEquals(1, notify_return_value);
       Ok(())
     }
 
-    #[op]
-    fn op_uncaught_exception() -> Result<(), AnyError> {
-      UNCAUGHT_EXCEPTION.fetch_add(1, Ordering::Relaxed);
-      Ok(())
-    }
-
     let extension = Extension::builder()
-      .ops(vec![
-        op_promise_reject::decl(),
-        op_uncaught_exception::decl(),
-      ])
+      .ops(vec![op_promise_reject::decl()])
       .build();
 
     #[derive(Default)]
@@ -3345,11 +3307,6 @@ assertEquals(1, notify_return_value);
         let source = r#"
         Deno.core.opSync("op_set_promise_reject_callback", (type, promise, reason) => {
           Deno.core.opSync("op_promise_reject");
-          throw reason;
-        });
-        
-        Deno.core.opSync("op_set_uncaught_exception_callback", (err) => {
-          Deno.core.opSync("op_uncaught_exception");
         });
         
         throw new Error('top level throw');
@@ -3379,10 +3336,9 @@ assertEquals(1, notify_return_value);
       .unwrap();
     let receiver = runtime.mod_evaluate(id);
     runtime.run_event_loop(false).await.unwrap();
-    receiver.await.unwrap().unwrap();
+    receiver.await.unwrap().unwrap_err();
 
     assert_eq!(1, PROMISE_REJECT.load(Ordering::Relaxed));
-    assert_eq!(1, UNCAUGHT_EXCEPTION.load(Ordering::Relaxed));
   }
 
   #[test]