From c8b5f1e454d5cb2bd7580bbe0ac4fa83237e9f41 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Wed, 26 Aug 2020 18:20:22 +0200 Subject: [PATCH] Simplify ErrBox-to-class mapping & hook it up to core json ops (#7195) --- cli/errors.rs | 6 ----- cli/ops/dispatch_json.rs | 27 +++++++++++++---------- cli/rt/10_dispatch_json.js | 2 +- cli/rt/10_dispatch_minimal.js | 18 +++++++-------- cli/state.rs | 16 ++++++++------ cli/tests/unit/dispatch_json_test.ts | 4 ++-- cli/tsc/10_dispatch_json.js | 2 +- cli/worker.rs | 4 ++-- core/core.js | 10 ++++----- core/core_isolate.rs | 33 ++++++++++------------------ core/lib.rs | 2 +- 11 files changed, 57 insertions(+), 67 deletions(-) diff --git a/cli/errors.rs b/cli/errors.rs index 6b53ac8d07..2d7bb1cd68 100644 --- a/cli/errors.rs +++ b/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() -} diff --git a/cli/ops/dispatch_json.rs b/cli/ops/dispatch_json.rs index 6e0eddac89..031e10c51b 100644 --- a/cli/ops/dispatch_json.rs +++ b/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, 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, { 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 { diff --git a/cli/rt/10_dispatch_json.js b/cli/rt/10_dispatch_json.js index 05b4d46ed7..4b244081ac 100644 --- a/cli/rt/10_dispatch_json.js +++ b/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; diff --git a/cli/rt/10_dispatch_minimal.js b/cli/rt/10_dispatch_minimal.js index 0722852565..a843d4a9ee 100644 --- a/cli/rt/10_dispatch_minimal.js +++ b/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; } diff --git a/cli/state.rs b/cli/state.rs index cb8d507f19..2ea75ace6c 100644 --- a/cli/state.rs +++ b/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(), ) diff --git a/cli/tests/unit/dispatch_json_test.ts b/cli/tests/unit/dispatch_json_test.ts index fe05122d5f..076c9e6f8a 100644 --- a/cli/tests/unit/dispatch_json_test.ts +++ b/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/); }); diff --git a/cli/tsc/10_dispatch_json.js b/cli/tsc/10_dispatch_json.js index a25014789a..7989f55683 100644 --- a/cli/tsc/10_dispatch_json.js +++ 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; diff --git a/cli/worker.rs b/cli/worker.rs index cfdcaeda7b..0129242e61 100644 --- a/cli/worker.rs +++ b/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 = { diff --git a/core/core.js b/core/core.js index 82a8120303..bf80528717 100644 --- a/core/core.js +++ b/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 diff --git a/core/core_isolate.rs b/core/core_isolate.rs index f4acbd1ac6..a60ce6a82d 100644 --- a/core/core_isolate.rs +++ b/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 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>, pub(crate) pending_promise_exceptions: HashMap>, pub(crate) js_error_create_fn: Box, - pub rust_err_to_json_fn: &'static dyn RustErrToJsonFn, + pub get_error_class_fn: GetErrorClassFn, pub(crate) shared: SharedQueue, pending_ops: FuturesUnordered, pending_unref_ops: FuturesUnordered, @@ -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, result: Result, + 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>( diff --git a/core/lib.rs b/core/lib.rs index 78a2fc2449..4134006685 100644 --- a/core/lib.rs +++ b/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;