From ecc630e8aa2f826aa11f752b980e0f6c3967811f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 21 Oct 2019 23:44:46 +0200 Subject: [PATCH] error handling in minimal dispatch --- Cargo.lock | 1 + cli/Cargo.toml | 1 + cli/js/dispatch_minimal.ts | 64 ++++++++++++++++++++++++------------- cli/ops/dispatch_minimal.rs | 34 +++++++++++++++++--- 4 files changed, 73 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2f9fe451801ad0..c8eb15e4702ed9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -270,6 +270,7 @@ version = "0.21.0" dependencies = [ "ansi_term 0.12.1 (registry+https://github.com/rust-lang/crates.io-index)", "atty 0.2.13 (registry+https://github.com/rust-lang/crates.io-index)", + "byteorder 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "clap 2.33.0 (registry+https://github.com/rust-lang/crates.io-index)", "deno 0.21.0", "deno_typescript 0.21.0", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index b4a5afd9fabe85..a9fd17ce226c7e 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -27,6 +27,7 @@ deno_typescript = { path = "../deno_typescript", version = "0.21.0" } ansi_term = "0.12.1" atty = "0.2.13" +byteorder = "1.3.2" clap = "2.33.0" dirs = "2.0.2" futures = "0.1.29" diff --git a/cli/js/dispatch_minimal.ts b/cli/js/dispatch_minimal.ts index 74a5e211cfd41a..4da05f3fdf65d9 100644 --- a/cli/js/dispatch_minimal.ts +++ b/cli/js/dispatch_minimal.ts @@ -1,13 +1,17 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. import * as util from "./util.ts"; import { core } from "./core.ts"; +import { TextDecoder } from "./text_encoding.ts"; +import { ErrorKind, DenoError } from "./errors.ts"; -const promiseTableMin = new Map>(); +const promiseTableMin = new Map>(); // 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(): number { return _nextPromiseId++; } @@ -17,23 +21,44 @@ export interface RecordMinimal { opId: number; // Maybe better called dispatchId arg: number; result: number; + err?: { + kind: ErrorKind; + message: string; + }; } export function recordFromBufMinimal( opId: number, - buf32: Int32Array + ui8: Uint8Array ): RecordMinimal { - if (buf32.length != 3) { - throw Error("Bad message"); + const buf32 = new Int32Array(ui8.buffer, ui8.byteOffset, ui8.byteLength / 4); + const arg = buf32[1]; + let err; + + if (arg < 0) { + const kind = buf32[2] as ErrorKind; + const message = decoder.decode(ui8.slice(12)); + err = { kind, message }; + } else if (buf32.length != 3) { + err = { kind: ErrorKind.InvalidData, message: "Bad message" }; } + return { promiseId: buf32[0], opId, arg: buf32[1], - result: buf32[2] + result: buf32[2], + err }; } +function unwrapResponse(res: RecordMinimal): number { + if (res.err != null) { + throw new DenoError(res.err!.kind, res.err!.message); + } + return res.result; +} + const scratch32 = new Int32Array(3); const scratchBytes = new Uint8Array( scratch32.buffer, @@ -43,15 +68,14 @@ const scratchBytes = new Uint8Array( util.assert(scratchBytes.byteLength === scratch32.length * 4); export function asyncMsgFromRust(opId: number, ui8: Uint8Array): void { - const buf32 = new Int32Array(ui8.buffer, ui8.byteOffset, ui8.byteLength / 4); - const record = recordFromBufMinimal(opId, buf32); - const { promiseId, result } = record; + const record = recordFromBufMinimal(opId, ui8); + const { promiseId } = record; const promise = promiseTableMin.get(promiseId); promiseTableMin.delete(promiseId); - promise!.resolve(result); + promise!.resolve(record); } -export function sendAsyncMinimal( +export async function sendAsyncMinimal( opId: number, arg: number, zeroCopy: Uint8Array @@ -60,22 +84,19 @@ export function sendAsyncMinimal( scratch32[0] = promiseId; scratch32[1] = arg; scratch32[2] = 0; // result - const promise = util.createResolvable(); + const promise = util.createResolvable(); const buf = core.dispatch(opId, scratchBytes, zeroCopy); if (buf) { - const buf32 = new Int32Array( - buf.buffer, - buf.byteOffset, - buf.byteLength / 4 - ); - const record = recordFromBufMinimal(opId, buf32); + const record = recordFromBufMinimal(opId, buf); // Sync result. - promise.resolve(record.result); + promise.resolve(record); } else { // Async result. promiseTableMin.set(promiseId, promise); } - return promise; + + const res = await promise; + return unwrapResponse(res); } export function sendSyncMinimal( @@ -86,7 +107,6 @@ export function sendSyncMinimal( scratch32[0] = 0; // promiseId 0 indicates sync scratch32[1] = arg; const res = core.dispatch(opId, scratchBytes, zeroCopy)!; - const res32 = new Int32Array(res.buffer, res.byteOffset, 3); - const resRecord = recordFromBufMinimal(opId, res32); - return resRecord.result; + const resRecord = recordFromBufMinimal(opId, res); + return unwrapResponse(resRecord); } diff --git a/cli/ops/dispatch_minimal.rs b/cli/ops/dispatch_minimal.rs index 618a040bfdbc62..0db7e42b06316c 100644 --- a/cli/ops/dispatch_minimal.rs +++ b/cli/ops/dispatch_minimal.rs @@ -4,6 +4,8 @@ //! alternative to flatbuffers using a very simple list of int32s to lay out //! messages. The first i32 is used to determine if a message a flatbuffer //! message or a "minimal" message. +use crate::deno_error::GetErrorKind; +use byteorder::{LittleEndian, WriteBytesExt}; use deno::Buf; use deno::CoreOp; use deno::ErrBox; @@ -31,6 +33,25 @@ impl Into for Record { } } +pub struct ErrorRecord { + pub promise_id: i32, + pub arg: i32, + pub error_code: i32, + pub error_message: Vec, +} + +impl Into for ErrorRecord { + fn into(mut self) -> Buf { + let v32: Vec = vec![self.promise_id, self.arg, self.error_code]; + let mut v8: Vec = Vec::new(); + for n in v32 { + v8.write_i32::(n).unwrap(); + } + v8.append(&mut self.error_message); + v8.into_boxed_slice() + } +} + pub fn parse_min_record(bytes: &[u8]) -> Option { if bytes.len() % std::mem::size_of::() != 0 { return None; @@ -85,15 +106,18 @@ pub fn minimal_op( match result { Ok(r) => { record.result = r; + Ok(record.into()) } Err(err) => { - // TODO(ry) The dispatch_minimal doesn't properly pipe errors back to - // the caller. - debug!("swallowed err {}", err); - record.result = -1; + let error_record = ErrorRecord { + promise_id: record.promise_id, + arg: -1, + error_code: err.kind() as i32, + error_message: err.to_string().as_bytes().to_owned(), + }; + Ok(error_record.into()) } } - Ok(record.into()) })); if is_sync {