diff options
Diffstat (limited to 'cli')
-rw-r--r-- | cli/fmt_errors.rs | 100 | ||||
-rw-r--r-- | cli/js/format_error.ts | 6 | ||||
-rw-r--r-- | cli/js/globals.ts | 2 | ||||
-rw-r--r-- | cli/js/repl.ts | 9 | ||||
-rw-r--r-- | cli/op_error.rs | 15 | ||||
-rw-r--r-- | cli/ops/errors.rs | 21 | ||||
-rw-r--r-- | cli/ops/worker_host.rs | 11 | ||||
-rw-r--r-- | cli/source_maps.rs | 158 | ||||
-rw-r--r-- | cli/worker.rs | 4 |
9 files changed, 124 insertions, 202 deletions
diff --git a/cli/fmt_errors.rs b/cli/fmt_errors.rs index 2bf0e859b..73d6bb2d2 100644 --- a/cli/fmt_errors.rs +++ b/cli/fmt_errors.rs @@ -3,11 +3,12 @@ use crate::colors; use crate::source_maps::apply_source_map; use crate::source_maps::SourceMapGetter; +use deno_core; use deno_core::ErrBox; -use deno_core::StackFrame; -use deno_core::V8Exception; +use deno_core::JSStackFrame; use std::error::Error; use std::fmt; +use std::ops::Deref; /// A trait which specifies parts of a diagnostic like item needs to be able to /// generate to conform its display to other diagnostic like items @@ -21,37 +22,37 @@ pub trait DisplayFormatter { fn format_source_name( script_name: String, - line: i64, + line_number: i64, column: i64, is_internal: bool, ) -> String { - let line = line + 1; + let line_number = line_number + 1; let column = column + 1; if is_internal { - format!("{}:{}:{}", script_name, line, column) + format!("{}:{}:{}", script_name, line_number, column) } else { let script_name_c = colors::cyan(script_name); - let line_c = colors::yellow(line.to_string()); + let line_c = colors::yellow(line_number.to_string()); let column_c = colors::yellow(column.to_string()); format!("{}:{}:{}", script_name_c, line_c, column_c) } } -/// Formats optional source, line and column into a single string. +/// Formats optional source, line number and column into a single string. pub fn format_maybe_source_name( script_name: Option<String>, - line: Option<i64>, + line_number: Option<i64>, column: Option<i64>, ) -> String { if script_name.is_none() { return "".to_string(); } - assert!(line.is_some()); + assert!(line_number.is_some()); assert!(column.is_some()); format_source_name( script_name.unwrap(), - line.unwrap(), + line_number.unwrap(), column.unwrap(), false, ) @@ -80,11 +81,11 @@ pub fn format_maybe_source_line( assert!(start_column.is_some()); assert!(end_column.is_some()); - let line = (1 + line_number.unwrap()).to_string(); - let line_color = colors::black_on_white(line.to_string()); - let line_len = line.len(); + let line_number = (1 + line_number.unwrap()).to_string(); + let line_color = colors::black_on_white(line_number.to_string()); + let line_number_len = line_number.len(); let line_padding = - colors::black_on_white(format!("{:indent$}", "", indent = line_len)) + colors::black_on_white(format!("{:indent$}", "", indent = line_number_len)) .to_string(); let mut s = String::new(); let start_column = start_column.unwrap(); @@ -124,7 +125,7 @@ pub fn format_error_message(msg: String) -> String { format!("{} {}", preamble, msg) } -fn format_stack_frame(frame: &StackFrame, is_internal_frame: bool) -> String { +fn format_stack_frame(frame: &JSStackFrame, is_internal_frame: bool) -> String { // Note when we print to string, we change from 0-indexed to 1-indexed. let function_name = if is_internal_frame { colors::italic_bold_gray(frame.function_name.clone()).to_string() @@ -133,7 +134,7 @@ fn format_stack_frame(frame: &StackFrame, is_internal_frame: bool) -> String { }; let mut source_loc = format_source_name( frame.script_name.clone(), - frame.line, + frame.line_number, frame.column, is_internal_frame, ); @@ -160,37 +161,25 @@ fn format_stack_frame(frame: &StackFrame, is_internal_frame: bool) -> String { } } -/// Wrapper around V8Exception which provides color to_string. +/// Wrapper around deno_core::JSError which provides color to_string. #[derive(Debug)] -pub struct JSError(V8Exception); +pub struct JSError(deno_core::JSError); impl JSError { - pub fn new(v8_exception: V8Exception) -> Self { - Self(v8_exception) - } - - pub fn from_json( - json_str: &str, - source_map_getter: &impl SourceMapGetter, - ) -> ErrBox { - let unmapped_exception = V8Exception::from_json(json_str).unwrap(); - Self::from_v8_exception(unmapped_exception, source_map_getter) - } - - pub fn from_v8_exception( - unmapped_exception: V8Exception, + pub fn create( + core_js_error: deno_core::JSError, source_map_getter: &impl SourceMapGetter, ) -> ErrBox { - let mapped_exception = - apply_source_map(&unmapped_exception, source_map_getter); - let js_error = Self(mapped_exception); + let core_js_error = apply_source_map(&core_js_error, source_map_getter); + let js_error = Self(core_js_error); ErrBox::from(js_error) } } -impl Into<V8Exception> for JSError { - fn into(self) -> V8Exception { - self.0 +impl Deref for JSError { + type Target = deno_core::JSError; + fn deref(&self) -> &Self::Target { + &self.0 } } @@ -264,53 +253,46 @@ mod tests { use super::*; use crate::colors::strip_ansi_codes; - fn error1() -> V8Exception { - V8Exception { + #[test] + fn js_error_to_string() { + let core_js_error = deno_core::JSError { message: "Error: foo bar".to_string(), source_line: None, script_resource_name: None, line_number: None, - start_position: None, - end_position: None, - error_level: None, start_column: None, end_column: None, frames: vec![ - StackFrame { - line: 4, + JSStackFrame { + line_number: 4, column: 16, script_name: "foo_bar.ts".to_string(), function_name: "foo".to_string(), is_eval: false, is_constructor: false, - is_wasm: false, }, - StackFrame { - line: 5, + JSStackFrame { + line_number: 5, column: 20, script_name: "bar_baz.ts".to_string(), function_name: "qat".to_string(), is_eval: false, is_constructor: false, - is_wasm: false, }, - StackFrame { - line: 1, + JSStackFrame { + line_number: 1, column: 1, script_name: "deno_main.js".to_string(), function_name: "".to_string(), is_eval: false, is_constructor: false, - is_wasm: false, }, ], - } - } - - #[test] - fn js_error_to_string() { - let e = error1(); - assert_eq!("error: Error: foo bar\n at foo (foo_bar.ts:5:17)\n at qat (bar_baz.ts:6:21)\n at deno_main.js:2:2", strip_ansi_codes(&JSError(e).to_string())); + }; + let formatted_error = JSError(core_js_error).to_string(); + let actual = strip_ansi_codes(&formatted_error); + let expected = "error: Error: foo bar\n at foo (foo_bar.ts:5:17)\n at qat (bar_baz.ts:6:21)\n at deno_main.js:2:2"; + assert_eq!(actual, expected); } #[test] diff --git a/cli/js/format_error.ts b/cli/js/format_error.ts index 21c4bcfa8..0a322338a 100644 --- a/cli/js/format_error.ts +++ b/cli/js/format_error.ts @@ -2,12 +2,6 @@ import { DiagnosticItem } from "./diagnostics.ts"; import { sendSync } from "./dispatch_json.ts"; -// TODO(bartlomieju): move to `repl.ts`? -export function formatError(errString: string): string { - const res = sendSync("op_format_error", { error: errString }); - return res.error; -} - /** * Format an array of diagnostic items and return them as a single string. * @param items An array of diagnostic items to format diff --git a/cli/js/globals.ts b/cli/js/globals.ts index 9a7161ff0..fd2082e40 100644 --- a/cli/js/globals.ts +++ b/cli/js/globals.ts @@ -96,7 +96,7 @@ declare global { // eslint-disable-next-line @typescript-eslint/no-explicit-any evalContext(code: string): [any, EvalErrorInfo | null]; - errorToJSON: (e: Error) => string; + formatError: (e: Error) => string; } // Only `var` variables show up in the `globalThis` type when doing a global diff --git a/cli/js/repl.ts b/cli/js/repl.ts index 6e4f2545a..0ef86ecd9 100644 --- a/cli/js/repl.ts +++ b/cli/js/repl.ts @@ -2,7 +2,6 @@ import { close } from "./files.ts"; import { exit } from "./os.ts"; import { core } from "./core.ts"; -import { formatError } from "./format_error.ts"; import { stringifyArgs } from "./console.ts"; import { sendSync, sendAsync } from "./dispatch_json.ts"; @@ -89,9 +88,7 @@ function evaluate(code: string): boolean { } else { lastThrownError = errInfo.thrown; if (errInfo.isNativeError) { - const formattedError = formatError( - core.errorToJSON(errInfo.thrown as Error) - ); + const formattedError = core.formatError(errInfo.thrown as Error); replError(formattedError); } else { replError("Thrown:", errInfo.thrown); @@ -162,7 +159,7 @@ export async function replLoop(): Promise<void> { if (err.message !== "Interrupted") { // e.g. this happens when we have deno.close(3). // We want to display the problem. - const formattedError = formatError(core.errorToJSON(err)); + const formattedError = core.formatError(err); replError(formattedError); } // Quit REPL anyways. @@ -184,7 +181,7 @@ export async function replLoop(): Promise<void> { } else { // e.g. this happens when we have deno.close(3). // We want to display the problem. - const formattedError = formatError(core.errorToJSON(err)); + const formattedError = core.formatError(err); replError(formattedError); quitRepl(1); } diff --git a/cli/op_error.rs b/cli/op_error.rs index 6a7dab20c..d81d4cad1 100644 --- a/cli/op_error.rs +++ b/cli/op_error.rs @@ -3,19 +3,16 @@ //! There are many types of errors in Deno: //! - ErrBox: a generic boxed object. This is the super type of all //! errors handled in Rust. -//! - JSError: exceptions thrown from V8 into Rust. Usually a user exception. -//! These are basically a big JSON structure which holds information about -//! line numbers. We use this to pretty-print stack traces. These are -//! never passed back into the runtime. +//! - JSError: a container for the error message and stack trace for exceptions +//! thrown in JavaScript code. We use this to pretty-print stack traces. //! - OpError: these are errors that happen during ops, which are passed //! back into the runtime, where an exception object is created and thrown. -//! OpErrors have an integer code associated with them - access this via the `kind` field. +//! OpErrors have an integer code associated with them - access this via the +//! `kind` field. //! - Diagnostic: these are errors that originate in TypeScript's compiler. //! They're similar to JSError, in that they have line numbers. -//! But Diagnostics are compile-time type errors, whereas JSErrors are runtime exceptions. -//! -//! TODO: -//! - rename/merge JSError with V8Exception? +//! But Diagnostics are compile-time type errors, whereas JSErrors are runtime +//! exceptions. use crate::import_map::ImportMapError; use deno_core::ErrBox; diff --git a/cli/ops/errors.rs b/cli/ops/errors.rs index c588759cf..4d2ce1bef 100644 --- a/cli/ops/errors.rs +++ b/cli/ops/errors.rs @@ -1,7 +1,6 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. use super::dispatch_json::{Deserialize, JsonOp, Value}; use crate::diagnostics::Diagnostic; -use crate::fmt_errors::JSError; use crate::op_error::OpError; use crate::source_maps::get_orig_position; use crate::source_maps::CachedMaps; @@ -14,7 +13,6 @@ pub fn init(i: &mut Isolate, s: &State) { "op_apply_source_map", s.stateful_json_op(op_apply_source_map), ); - i.register_op("op_format_error", s.stateful_json_op(op_format_error)); i.register_op( "op_format_diagnostic", s.stateful_json_op(op_format_diagnostic), @@ -22,25 +20,6 @@ pub fn init(i: &mut Isolate, s: &State) { } #[derive(Deserialize)] -struct FormatErrorArgs { - error: String, -} - -fn op_format_error( - state: &State, - args: Value, - _zero_copy: Option<ZeroCopyBuf>, -) -> Result<JsonOp, OpError> { - let args: FormatErrorArgs = serde_json::from_value(args)?; - let error = - JSError::from_json(&args.error, &state.borrow().global_state.ts_compiler); - - Ok(JsonOp::Sync(json!({ - "error": error.to_string(), - }))) -} - -#[derive(Deserialize)] struct ApplySourceMap { filename: String, line: i32, diff --git a/cli/ops/worker_host.rs b/cli/ops/worker_host.rs index 55afd6bf9..0690a3977 100644 --- a/cli/ops/worker_host.rs +++ b/cli/ops/worker_host.rs @@ -215,15 +215,14 @@ fn serialize_worker_event(event: WorkerEvent) -> Value { } }); - if let Ok(err) = error.downcast::<JSError>() { - let exception: V8Exception = err.into(); + if let Ok(js_error) = error.downcast::<JSError>() { serialized_error = json!({ "type": "error", "error": { - "message": exception.message, - "fileName": exception.script_resource_name, - "lineNumber": exception.line_number, - "columnNumber": exception.start_column, + "message": js_error.message, + "fileName": js_error.script_resource_name, + "lineNumber": js_error.line_number, + "columnNumber": js_error.start_column, } }); } diff --git a/cli/source_maps.rs b/cli/source_maps.rs index 6b177cc69..9e29e554c 100644 --- a/cli/source_maps.rs +++ b/cli/source_maps.rs @@ -1,7 +1,7 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. -//! This mod provides functions to remap a deno_core::V8Exception based on a source map -use deno_core::StackFrame; -use deno_core::V8Exception; +//! This mod provides functions to remap a deno_core::deno_core::JSError based on a source map +use deno_core; +use deno_core::JSStackFrame; use serde_json; use source_map_mappings::parse_mappings; use source_map_mappings::Bias; @@ -12,7 +12,11 @@ use std::str; pub trait SourceMapGetter { /// Returns the raw source map file. fn get_source_map(&self, script_name: &str) -> Option<Vec<u8>>; - fn get_source_line(&self, script_name: &str, line: usize) -> Option<String>; + fn get_source_line( + &self, + script_name: &str, + line_number: usize, + ) -> Option<String>; } /// Cached filename lookups. The key can be None if a previous lookup failed to @@ -82,36 +86,36 @@ fn builtin_source_map(script_name: &str) -> Option<Vec<u8>> { } } -/// Apply a source map to a V8Exception, returning a V8Exception where the filenames, -/// the lines and the columns point to their original source location, not their -/// transpiled location if applicable. +/// Apply a source map to a deno_core::JSError, returning a JSError where file +/// names and line/column numbers point to the location in the original source, +/// rather than the transpiled source code. pub fn apply_source_map<G: SourceMapGetter>( - v8_exception: &V8Exception, + js_error: &deno_core::JSError, getter: &G, -) -> V8Exception { +) -> deno_core::JSError { let mut mappings_map: CachedMaps = HashMap::new(); - let mut frames = Vec::<StackFrame>::new(); - for frame in &v8_exception.frames { + let mut frames = Vec::<JSStackFrame>::new(); + for frame in &js_error.frames { let f = frame_apply_source_map(&frame, &mut mappings_map, getter); frames.push(f); } let (script_resource_name, line_number, start_column) = get_maybe_orig_position( - v8_exception.script_resource_name.clone(), - v8_exception.line_number, - v8_exception.start_column, + js_error.script_resource_name.clone(), + js_error.line_number, + js_error.start_column, &mut mappings_map, getter, ); // It is better to just move end_column to be the same distance away from // start column because sometimes the code point is not available in the // source file map. - let end_column = match v8_exception.end_column { + let end_column = match js_error.end_column { Some(ec) => { if let Some(sc) = start_column { - Some(ec - (v8_exception.start_column.unwrap() - sc)) + Some(ec - (js_error.start_column.unwrap() - sc)) } else { None } @@ -122,74 +126,67 @@ pub fn apply_source_map<G: SourceMapGetter>( // will go fetch it from the getter let source_line = match line_number { Some(ln) - if v8_exception.source_line.is_some() - && script_resource_name.is_some() => + if js_error.source_line.is_some() && script_resource_name.is_some() => { getter.get_source_line( - &v8_exception.script_resource_name.clone().unwrap(), + &js_error.script_resource_name.clone().unwrap(), ln as usize, ) } - _ => v8_exception.source_line.clone(), + _ => js_error.source_line.clone(), }; - V8Exception { - message: v8_exception.message.clone(), - frames, - error_level: v8_exception.error_level, + deno_core::JSError { + message: js_error.message.clone(), source_line, script_resource_name, line_number, start_column, end_column, - // These are difficult to map to their original position and they are not - // currently used in any output, so we don't remap them. - start_position: v8_exception.start_position, - end_position: v8_exception.end_position, + frames, } } fn frame_apply_source_map<G: SourceMapGetter>( - frame: &StackFrame, + frame: &JSStackFrame, mappings_map: &mut CachedMaps, getter: &G, -) -> StackFrame { - let (script_name, line, column) = get_orig_position( +) -> JSStackFrame { + let (script_name, line_number, column) = get_orig_position( frame.script_name.to_string(), - frame.line, + frame.line_number, frame.column, mappings_map, getter, ); - StackFrame { + JSStackFrame { script_name, function_name: frame.function_name.clone(), - line, + line_number, column, is_eval: frame.is_eval, is_constructor: frame.is_constructor, - is_wasm: frame.is_wasm, } } fn get_maybe_orig_position<G: SourceMapGetter>( script_name: Option<String>, - line: Option<i64>, + line_number: Option<i64>, column: Option<i64>, mappings_map: &mut CachedMaps, getter: &G, ) -> (Option<String>, Option<i64>, Option<i64>) { - match (script_name, line, column) { + match (script_name, line_number, column) { (Some(script_name_v), Some(line_v), Some(column_v)) => { - let (script_name, line, column) = get_orig_position( + let (script_name, line_number, column) = get_orig_position( script_name_v, line_v - 1, column_v, mappings_map, getter, ); - (Some(script_name), Some(line), Some(column)) + (Some(script_name), Some(line_number), Some(column)) } _ => (None, None, None), } @@ -197,18 +194,18 @@ fn get_maybe_orig_position<G: SourceMapGetter>( pub fn get_orig_position<G: SourceMapGetter>( script_name: String, - line: i64, + line_number: i64, column: i64, mappings_map: &mut CachedMaps, getter: &G, ) -> (String, i64, i64) { let maybe_sm = get_mappings(&script_name, mappings_map, getter); - let default_pos = (script_name, line, column); + let default_pos = (script_name, line_number, column); match maybe_sm { None => default_pos, Some(sm) => match sm.mappings.original_location_for( - line as u32, + line_number as u32, column as u32, Bias::default(), ) { @@ -274,7 +271,7 @@ mod tests { fn get_source_line( &self, script_name: &str, - line: usize, + line_number: usize, ) -> Option<String> { let s = match script_name { "foo_bar.ts" => vec![ @@ -286,99 +283,83 @@ mod tests { ], _ => return None, }; - if s.len() > line { - Some(s[line].to_string()) + if s.len() > line_number { + Some(s[line_number].to_string()) } else { None } } } - fn error1() -> V8Exception { - V8Exception { + #[test] + fn apply_source_map_1() { + let core_js_error = deno_core::JSError { message: "Error: foo bar".to_string(), source_line: None, script_resource_name: None, line_number: None, - start_position: None, - end_position: None, - error_level: None, start_column: None, end_column: None, frames: vec![ - StackFrame { - line: 4, + JSStackFrame { + line_number: 4, column: 16, script_name: "foo_bar.ts".to_string(), function_name: "foo".to_string(), is_eval: false, is_constructor: false, - is_wasm: false, }, - StackFrame { - line: 5, + JSStackFrame { + line_number: 5, column: 20, script_name: "bar_baz.ts".to_string(), function_name: "qat".to_string(), is_eval: false, is_constructor: false, - is_wasm: false, }, - StackFrame { - line: 1, + JSStackFrame { + line_number: 1, column: 1, script_name: "deno_main.js".to_string(), function_name: "".to_string(), is_eval: false, is_constructor: false, - is_wasm: false, }, ], - } - } - - #[test] - fn v8_exception_apply_source_map_1() { - let e = error1(); + }; let getter = MockSourceMapGetter {}; - let actual = apply_source_map(&e, &getter); - let expected = V8Exception { + let actual = apply_source_map(&core_js_error, &getter); + let expected = deno_core::JSError { message: "Error: foo bar".to_string(), source_line: None, script_resource_name: None, line_number: None, - start_position: None, - end_position: None, - error_level: None, start_column: None, end_column: None, frames: vec![ - StackFrame { - line: 5, + JSStackFrame { + line_number: 5, column: 12, script_name: "foo_bar.ts".to_string(), function_name: "foo".to_string(), is_eval: false, is_constructor: false, - is_wasm: false, }, - StackFrame { - line: 4, + JSStackFrame { + line_number: 4, column: 14, script_name: "bar_baz.ts".to_string(), function_name: "qat".to_string(), is_eval: false, is_constructor: false, - is_wasm: false, }, - StackFrame { - line: 1, + JSStackFrame { + line_number: 1, column: 1, script_name: "deno_main.js".to_string(), function_name: "".to_string(), is_eval: false, is_constructor: false, - is_wasm: false, }, ], }; @@ -386,25 +367,21 @@ mod tests { } #[test] - fn v8_exception_apply_source_map_2() { - let e = V8Exception { + fn apply_source_map_2() { + let e = deno_core::JSError { message: "TypeError: baz".to_string(), source_line: None, script_resource_name: None, line_number: None, - start_position: None, - end_position: None, - error_level: None, start_column: None, end_column: None, - frames: vec![StackFrame { - line: 11, + frames: vec![JSStackFrame { + line_number: 11, column: 12, script_name: "CLI_SNAPSHOT.js".to_string(), function_name: "setLogDebug".to_string(), is_eval: false, is_constructor: false, - is_wasm: false, }], }; let getter = MockSourceMapGetter {}; @@ -416,15 +393,12 @@ mod tests { } #[test] - fn v8_exception_apply_source_map_line() { - let e = V8Exception { + fn apply_source_map_line() { + let e = deno_core::JSError { message: "TypeError: baz".to_string(), source_line: Some("foo".to_string()), script_resource_name: Some("foo_bar.ts".to_string()), line_number: Some(4), - start_position: None, - end_position: None, - error_level: None, start_column: Some(16), end_column: None, frames: vec![], diff --git a/cli/worker.rs b/cli/worker.rs index 3bc424ebe..370e3f297 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -105,8 +105,8 @@ impl Worker { let mut isolate = deno_core::EsIsolate::new(loader, startup_data, false); let global_state_ = state.borrow().global_state.clone(); - isolate.set_js_error_create(move |v8_exception| { - JSError::from_v8_exception(v8_exception, &global_state_.ts_compiler) + isolate.set_js_error_create_fn(move |core_js_error| { + JSError::create(core_js_error, &global_state_.ts_compiler) }); let (internal_channels, external_channels) = create_channels(); |