Simplify ErrBox-to-class mapping & hook it up to core json ops (#7195)

cli/errors.rs

@@ -228,9 +228,3 @@ pub fn get_error_class(e: &ErrBox) -> &'static str {
     panic!("ErrBox '{}' contains boxed error of unknown type", e);
   })
 }
-
-pub fn rust_err_to_json(error: &ErrBox) -> Box<[u8]> {
-  let error_value =
-    json!({ "kind": get_error_class(error), "message": error.to_string()});
-  serde_json::to_vec(&error_value).unwrap().into_boxed_slice()
-}

cli/ops/dispatch_json.rs

@@ -24,17 +24,19 @@ pub enum JsonOp {
 }
 
 pub fn serialize_result(
-  rust_err_to_json_fn: &'static dyn deno_core::RustErrToJsonFn,
   promise_id: Option<u64>,
   result: JsonResult,
+  get_error_class_fn: deno_core::GetErrorClassFn,
 ) -> Buf {
   let value = match result {
     Ok(v) => json!({ "ok": v, "promiseId": promise_id }),
-    Err(err) => {
-      let serialized_err = (&rust_err_to_json_fn)(&err);
-      let err_value: Value = serde_json::from_slice(&serialized_err).unwrap();
-      json!({ "err": err_value, "promiseId": promise_id })
-    }
+    Err(err) => json!({
+      "err": {
+        "className": (get_error_class_fn)(&err),
+        "message": err.to_string()
+      },
+      "promiseId": promise_id
+    }),
   };
   serde_json::to_vec(&value).unwrap().into_boxed_slice()
 }
@@ -56,12 +58,13 @@ where
   ) -> Result<JsonOp, ErrBox>,
 {
   move |isolate_state: &mut CoreIsolateState, zero_copy: &mut [ZeroCopyBuf]| {
+    let get_error_class_fn = isolate_state.get_error_class_fn;
+
     assert!(!zero_copy.is_empty(), "Expected JSON string at position 0");
-    let rust_err_to_json_fn = isolate_state.rust_err_to_json_fn;
     let async_args: AsyncArgs = match serde_json::from_slice(&zero_copy[0]) {
       Ok(args) => args,
       Err(e) => {
-        let buf = serialize_result(rust_err_to_json_fn, None, Err(e.into()));
+        let buf = serialize_result(None, Err(e.into()), get_error_class_fn);
         return Op::Sync(buf);
       }
     };
@@ -77,18 +80,18 @@ where
       Ok(JsonOp::Sync(sync_value)) => {
         assert!(promise_id.is_none());
         Op::Sync(serialize_result(
-          rust_err_to_json_fn,
           promise_id,
           Ok(sync_value),
+          get_error_class_fn,
         ))
       }
       Ok(JsonOp::Async(fut)) => {
         assert!(promise_id.is_some());
         let fut2 = fut.then(move |result| {
           futures::future::ready(serialize_result(
-            rust_err_to_json_fn,
             promise_id,
             result,
+            get_error_class_fn,
           ))
         });
         Op::Async(fut2.boxed_local())
@@ -97,16 +100,16 @@ where
         assert!(promise_id.is_some());
         let fut2 = fut.then(move |result| {
           futures::future::ready(serialize_result(
-            rust_err_to_json_fn,
             promise_id,
             result,
+            get_error_class_fn,
           ))
         });
         Op::AsyncUnref(fut2.boxed_local())
       }
       Err(sync_err) => {
         let buf =
-          serialize_result(rust_err_to_json_fn, promise_id, Err(sync_err));
+          serialize_result(promise_id, Err(sync_err), get_error_class_fn);
         if is_sync {
           Op::Sync(buf)
         } else {

cli/rt/10_dispatch_json.js

@@ -21,7 +21,7 @@
 
   function unwrapResponse(res) {
     if (res.err != null) {
-      throw new (core.getErrorClass(res.err.kind))(res.err.message);
+      throw new (core.getErrorClass(res.err.className))(res.err.message);
     }
     util.assert(res.ok != null);
     return res.ok;

cli/rt/10_dispatch_minimal.js

@@ -7,19 +7,20 @@
   // Using an object without a prototype because `Map` was causing GC problems.
   const promiseTableMin = Object.create(null);
 
+  const decoder = new TextDecoder();
+
   // Note it's important that promiseId starts at 1 instead of 0, because sync
   // messages are indicated with promiseId 0. If we ever add wrap around logic for
   // overflows, this should be taken into account.
   let _nextPromiseId = 1;
 
-  const decoder = new TextDecoder();
-
   function nextPromiseId() {
     return _nextPromiseId++;
   }
 
   function recordFromBufMinimal(ui8) {
-    const header = ui8.subarray(0, 12);
+    const headerLen = 12;
+    const header = ui8.subarray(0, headerLen);
     const buf32 = new Int32Array(
       header.buffer,
       header.byteOffset,
@@ -31,11 +32,10 @@
     let err;
 
     if (arg < 0) {
-      const classLen = result;
-      const classAndMessage = decoder.decode(ui8.subarray(12));
-      const errorClass = classAndMessage.slice(0, classLen);
-      const message = classAndMessage.slice(classLen);
-      err = { kind: errorClass, message };
+      err = {
+        className: decoder.decode(ui8.subarray(headerLen, headerLen + result)),
+        message: decoder.decode(ui8.subarray(headerLen + result)),
+      };
     } else if (ui8.length != 12) {
       throw new TypeError("Malformed response message");
     }
@@ -50,7 +50,7 @@
 
   function unwrapResponse(res) {
     if (res.err != null) {
-      throw new (core.getErrorClass(res.err.kind))(res.err.message);
+      throw new (core.getErrorClass(res.err.className))(res.err.message);
     }
     return res.result;
   }

cli/state.rs

@@ -86,15 +86,16 @@ impl State {
     let resource_table = resource_table.clone();
 
     move |isolate_state: &mut CoreIsolateState, bufs: &mut [ZeroCopyBuf]| {
-      let rust_err_to_json_fn = isolate_state.rust_err_to_json_fn;
+      let get_error_class_fn = isolate_state.get_error_class_fn;
+
       // The first buffer should contain JSON encoded op arguments; parse them.
       let args: Value = match serde_json::from_slice(&bufs[0]) {
         Ok(v) => v,
         Err(e) => {
           return Op::Sync(serialize_result(
-            rust_err_to_json_fn,
             None,
             Err(e.into()),
+            get_error_class_fn,
           ));
         }
       };
@@ -106,7 +107,7 @@ impl State {
         dispatcher(&state, &mut *resource_table.borrow_mut(), args, zero_copy);
 
       // Convert to Op.
-      Op::Sync(serialize_result(rust_err_to_json_fn, None, result))
+      Op::Sync(serialize_result(None, result, get_error_class_fn))
     }
   }
 
@@ -124,13 +125,14 @@ impl State {
     let resource_table = resource_table.clone();
 
     move |isolate_state: &mut CoreIsolateState, bufs: &mut [ZeroCopyBuf]| {
-      let rust_err_to_json_fn = isolate_state.rust_err_to_json_fn;
+      let get_error_class_fn = isolate_state.get_error_class_fn;
+
       // The first buffer should contain JSON encoded op arguments; parse them.
       let args: Value = match serde_json::from_slice(&bufs[0]) {
         Ok(v) => v,
         Err(e) => {
           let e = e.into();
-          return Op::Sync(serialize_result(rust_err_to_json_fn, None, Err(e)));
+          return Op::Sync(serialize_result(None, Err(e), get_error_class_fn));
         }
       };
 
@@ -139,7 +141,7 @@ impl State {
         Some(i) => i,
         None => {
           let e = ErrBox::new("TypeError", "`promiseId` invalid/missing");
-          return Op::Sync(serialize_result(rust_err_to_json_fn, None, Err(e)));
+          return Op::Sync(serialize_result(None, Err(e), get_error_class_fn));
         }
       };
 
@@ -157,7 +159,7 @@ impl State {
       // Convert to Op.
       Op::Async(
         async move {
-          serialize_result(rust_err_to_json_fn, Some(promise_id), fut.await)
+          serialize_result(Some(promise_id), fut.await, get_error_class_fn)
         }
         .boxed_local(),
       )

cli/tests/unit/dispatch_json_test.ts

@@ -39,7 +39,7 @@ unitTest(function malformedJsonControlBuffer(): void {
   const resText = new TextDecoder().decode(resBuf);
   const resObj = JSON.parse(resText);
   assertStrictEquals(resObj.ok, undefined);
-  assertStrictEquals(resObj.err.kind, "SyntaxError");
+  assertStrictEquals(resObj.err.className, "SyntaxError");
   assertMatch(resObj.err.message, /\bexpected value\b/);
 });
 
@@ -65,6 +65,6 @@ unitTest(function invalidPromiseId(): void {
   const resObj = JSON.parse(resText);
   console.error(resText);
   assertStrictEquals(resObj.ok, undefined);
-  assertStrictEquals(resObj.err.kind, "TypeError");
+  assertStrictEquals(resObj.err.className, "TypeError");
   assertMatch(resObj.err.message, /\bpromiseId\b/);
 });

cli/tsc/10_dispatch_json.js

@@ -22,7 +22,7 @@
 
   function unwrapResponse(res) {
     if (res.err != null) {
-      throw new (core.getErrorClass(res.err.kind))(res.err.message);
+      throw new (core.getErrorClass(res.err.className))(res.err.message);
     }
     util.assert(res.ok != null);
     return res.ok;

cli/worker.rs

@@ -1,5 +1,5 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
-use crate::errors::rust_err_to_json;
+use crate::errors::get_error_class;
 use crate::fmt_errors::JSError;
 use crate::global_state::GlobalState;
 use crate::inspector::DenoInspector;
@@ -116,7 +116,7 @@ impl Worker {
       core_state.set_js_error_create_fn(move |core_js_error| {
         JSError::create(core_js_error, &global_state.ts_compiler)
       });
-      core_state.set_rust_err_to_json_fn(&rust_err_to_json);
+      core_state.set_get_error_class_fn(&get_error_class);
     }
 
     let inspector = {

core/core.js

@@ -188,17 +188,17 @@ SharedQueue Binary Layout
     return send(opsCache[opName], control, ...zeroCopy);
   }
 
-  function registerErrorClass(errorName, errorClass) {
+  function registerErrorClass(errorName, className) {
     if (typeof errorMap[errorName] !== "undefined") {
       throw new TypeError(`Error class for "${errorName}" already registered`);
     }
-    errorMap[errorName] = errorClass;
+    errorMap[errorName] = className;
   }
 
   function getErrorClass(errorName) {
-    const errorClass = errorMap[errorName];
-    assert(errorClass);
-    return errorClass;
+    const className = errorMap[errorName];
+    assert(className);
+    return className;
   }
 
   // Returns Uint8Array

core/core_isolate.rs

@@ -87,16 +87,7 @@ impl StartupData<'_> {
 
 type JSErrorCreateFn = dyn Fn(JSError) -> ErrBox;
 
-pub trait RustErrToJsonFn: for<'e> Fn(&'e ErrBox) -> Box<[u8]> {}
-impl<F> RustErrToJsonFn for F where for<'e> F: Fn(&'e ErrBox) -> Box<[u8]> {}
-
-fn rust_err_to_json(e: &ErrBox) -> Box<[u8]> {
-  let value = json!({
-    "kind": "Error",
-    "message": e.to_string()
-  });
-  serde_json::to_vec(&value).unwrap().into_boxed_slice()
-}
+pub type GetErrorClassFn = &'static dyn for<'e> Fn(&'e ErrBox) -> &'static str;
 
 /// Objects that need to live as long as the isolate
 #[derive(Default)]
@@ -134,7 +125,7 @@ pub struct CoreIsolateState {
   pub(crate) js_macrotask_cb: Option<v8::Global<v8::Function>>,
   pub(crate) pending_promise_exceptions: HashMap<i32, v8::Global<v8::Value>>,
   pub(crate) js_error_create_fn: Box<JSErrorCreateFn>,
-  pub rust_err_to_json_fn: &'static dyn RustErrToJsonFn,
+  pub get_error_class_fn: GetErrorClassFn,
   pub(crate) shared: SharedQueue,
   pending_ops: FuturesUnordered<PendingOpFuture>,
   pending_unref_ops: FuturesUnordered<PendingOpFuture>,
@@ -319,7 +310,7 @@ impl CoreIsolate {
       js_recv_cb: None,
       js_macrotask_cb: None,
       js_error_create_fn: Box::new(JSError::create),
-      rust_err_to_json_fn: &rust_err_to_json,
+      get_error_class_fn: &|_| "Error",
       shared: SharedQueue::new(RECOMMENDED_SIZE),
       pending_ops: FuturesUnordered::new(),
       pending_unref_ops: FuturesUnordered::new(),
@@ -457,7 +448,7 @@ impl CoreIsolate {
       move |state: &mut CoreIsolateState, bufs: &mut [ZeroCopyBuf]| -> Op {
         let value = serde_json::from_slice(&bufs[0]).unwrap();
         let result = op(state, value, &mut bufs[1..]);
-        let buf = serialize_result(None, result);
+        let buf = serialize_result(None, result, state.get_error_class_fn);
         Op::Sync(buf)
       };
 
@@ -475,11 +466,13 @@ impl CoreIsolate {
     let core_op = move |state: &mut CoreIsolateState,
                         bufs: &mut [ZeroCopyBuf]|
           -> Op {
+      let get_error_class_fn = state.get_error_class_fn;
       let value: serde_json::Value = serde_json::from_slice(&bufs[0]).unwrap();
       let promise_id = value.get("promiseId").unwrap().as_u64().unwrap();
       let fut = op(state, value, &mut bufs[1..]);
-      let fut2 =
-        fut.map(move |result| serialize_result(Some(promise_id), result));
+      let fut2 = fut.map(move |result| {
+        serialize_result(Some(promise_id), result, get_error_class_fn)
+      });
       Op::Async(Box::pin(fut2))
     };
 
@@ -546,13 +539,14 @@ where
 fn serialize_result(
   promise_id: Option<u64>,
   result: Result<Value, ErrBox>,
+  get_error_class_fn: GetErrorClassFn,
 ) -> Buf {
   let value = match result {
     Ok(v) => json!({ "ok": v, "promiseId": promise_id }),
     Err(err) => json!({
       "promiseId": promise_id ,
       "err": {
-        "kind": "Error",
+        "className": (get_error_class_fn)(&err),
         "message": err.to_string(),
       }
     }),
@@ -681,11 +675,8 @@ impl CoreIsolateState {
     self.js_error_create_fn = Box::new(f);
   }
 
-  pub fn set_rust_err_to_json_fn(
-    &mut self,
-    f: &'static (impl for<'e> Fn(&'e ErrBox) -> Box<[u8]> + 'static),
-  ) {
-    self.rust_err_to_json_fn = f;
+  pub fn set_get_error_class_fn(&mut self, f: GetErrorClassFn) {
+    self.get_error_class_fn = f;
   }
 
   pub fn dispatch_op<'s>(

core/lib.rs

@@ -27,8 +27,8 @@ pub use rusty_v8 as v8;
 pub use crate::core_isolate::js_check;
 pub use crate::core_isolate::CoreIsolate;
 pub use crate::core_isolate::CoreIsolateState;
+pub use crate::core_isolate::GetErrorClassFn;
 pub use crate::core_isolate::HeapLimits;
-pub use crate::core_isolate::RustErrToJsonFn;
 pub use crate::core_isolate::Script;
 pub use crate::core_isolate::Snapshot;
 pub use crate::core_isolate::StartupData;