refactor: simplify async op error construction (#1039)

Removes usage of serde_v8 for constructing errors
denoland · Jan 9, 2025 · b6308e5 · b6308e5
1 parent eb0cfb1
commit b6308e5
Show file tree

Hide file tree

Showing 41 changed files with 268 additions and 390 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/core/00_infra.js b/core/00_infra.js
@@ -17,7 +17,7 @@
     Promise,
     PromiseReject,
     PromiseResolve,
-    PromisePrototypeThen,
+    PromisePrototypeCatch,
     RangeError,
     ReferenceError,
     SafeArrayIterator,
@@ -133,18 +133,22 @@
       MapPrototypeSet(promiseMap, oldPromiseId, oldPromise);
     }
 
-    const promise = new Promise((resolve) => {
-      promiseRing[idx] = resolve;
+    const promise = new Promise((resolve, reject) => {
+      promiseRing[idx] = [resolve, reject];
     });
-    const wrappedPromise = PromisePrototypeThen(
+    const wrappedPromise = PromisePrototypeCatch(
       promise,
-      unwrapOpError(),
+      (res) => {
+        // recreate the stacktrace and strip eventLoopTick() calls from stack trace
+        ErrorCaptureStackTrace(res, eventLoopTick);
+        throw res;
+      },
     );
     wrappedPromise[promiseIdSymbol] = promiseId;
     return wrappedPromise;
   }
 
-  function __resolvePromise(promiseId, res) {
+  function __resolvePromise(promiseId, res, isOk) {
     // Check if out of ring bounds, fallback to map
     const outOfBounds = promiseId < nextPromiseId - RING_SIZE;
     if (outOfBounds) {
@@ -153,7 +157,11 @@
         throw "Missing promise in map @ " + promiseId;
       }
       MapPrototypeDelete(promiseMap, promiseId);
-      promise(res);
+      if (isOk) {
+        promise[0](res);
+      } else {
+        promise[1](res);
+      }
     } else {
       // Otherwise take from ring
       const idx = promiseId % RING_SIZE;
@@ -162,7 +170,11 @@
         throw "Missing promise in ring @ " + promiseId;
       }
       promiseRing[idx] = NO_PROMISE;
-      promise(res);
+      if (isOk) {
+        promise[0](res);
+      } else {
+        promise[1](res);
+      }
     }
   }
 
@@ -177,38 +189,6 @@
     return promiseRing[idx] != NO_PROMISE;
   }
 
-  function unwrapOpError() {
-    return (res) => {
-      // .$err_class_name is a special key that should only exist on errors
-      const className = res?.$err_class_name;
-      if (!className) {
-        return res;
-      }
-
-      const errorBuilder = errorMap[className];
-      const err = errorBuilder ? errorBuilder(res.message) : new Error(
-        `Unregistered error class: "${className}"\n  ${res.message}\n  Classes of errors returned from ops should be registered via Deno.core.registerErrorClass().`,
-      );
-
-      if (res.additional_properties) {
-        for (
-          const property of new SafeArrayIterator(res.additional_properties)
-        ) {
-          const key = property[0];
-          if (!(key in err)) {
-            ObjectDefineProperty(err, key, {
-              value: property[1],
-              writable: false,
-            });
-          }
-        }
-      }
-      // Strip eventLoopTick() calls from stack trace
-      ErrorCaptureStackTrace(err, eventLoopTick);
-      throw err;
-    };
-  }
-
   function setUpAsyncStub(opName, originalOp, maybeProto) {
     let fn;
 

diff --git a/core/01_core.js b/core/01_core.js
@@ -161,10 +161,12 @@
   // responses of async ops.
   function eventLoopTick() {
     // First respond to all pending ops.
-    for (let i = 0; i < arguments.length - 3; i += 2) {
+    for (let i = 0; i < arguments.length - 3; i += 3) {
       const promiseId = arguments[i];
-      const res = arguments[i + 1];
-      __resolvePromise(promiseId, res);
+      const isOk = arguments[i + 1];
+      const res = arguments[i + 2];
+
+      __resolvePromise(promiseId, res, isOk);
     }
     // Drain nextTick queue if there's a tick scheduled.
     if (arguments[arguments.length - 1]) {

diff --git a/core/error.rs b/core/error.rs
@@ -1,8 +1,6 @@
 // Copyright 2018-2025 the Deno authors. MIT license.
 
 pub use super::modules::ModuleConcreteError;
-pub use super::runtime::op_driver::OpError;
-pub use super::runtime::op_driver::OpErrorWrapper;
 pub use crate::io::ResourceError;
 pub use crate::modules::ModuleLoaderError;
 use crate::runtime::v8_static_strings;

diff --git a/core/examples/hello_world.rs b/core/examples/hello_world.rs
@@ -7,7 +7,7 @@ use deno_core::*;
 /// An op for summing an array of numbers. The op-layer automatically
 /// deserializes inputs and serializes the returned Result & value.
 #[op2]
-fn op_sum(#[serde] nums: Vec<f64>) -> Result<f64, deno_core::error::OpError> {
+fn op_sum(#[serde] nums: Vec<f64>) -> Result<f64, deno_error::JsErrorBox> {
   // Sum inputs
   let sum = nums.iter().fold(0.0, |a, v| a + v);
   // return as a Result<f64, OpError>

diff --git a/core/examples/op2.rs b/core/examples/op2.rs
@@ -1,15 +1,14 @@
 // Copyright 2018-2025 the Deno authors. MIT license.
 
 use anyhow::Context;
-use deno_core::error::OpError;
 use deno_core::*;
 use std::rc::Rc;
 
 #[op2]
 fn op_use_state(
   state: &mut OpState,
   #[global] callback: v8::Global<v8::Function>,
-) -> Result<(), OpError> {
+) -> Result<(), deno_error::JsErrorBox> {
   state.put(callback);
   Ok(())
 }