Do not convert exceptions to JSON and back (denoland#4214)

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]

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

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

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);
  }

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;

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,32 +13,12 @@ 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),
  );
 }
 
-#[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,

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,
  }
  });
  }