refactor: Remove PrettyJsError and js_error_create_fn

cli/fmt_errors.rs

@@ -4,11 +4,8 @@ use crate::colors::cyan;
 use crate::colors::italic_bold;
 use crate::colors::red;
 use crate::colors::yellow;
-use deno_core::error::{AnyError, JsError, JsStackFrame};
+use deno_core::error::{JsError, JsStackFrame};
 use deno_core::url::Url;
-use std::error::Error;
-use std::fmt;
-use std::ops::Deref;
 
 const SOURCE_ABBREV_THRESHOLD: usize = 150;
 const DATA_URL_ABBREV_THRESHOLD: usize = 150;
@@ -181,12 +178,12 @@ fn format_maybe_source_line(
   format!("\n{}{}\n{}{}", indent, source_line, indent, color_underline)
 }
 
-fn format_js_error(js_error: &JsError, is_child: bool) -> String {
+fn format_js_error_inner(js_error: &JsError, is_child: bool) -> String {
   let mut s = String::new();
   s.push_str(&js_error.exception_message);
   if let Some(aggregated) = &js_error.aggregated {
     for aggregated_error in aggregated {
-      let error_string = format_js_error(aggregated_error, true);
+      let error_string = format_js_error_inner(aggregated_error, true);
       for line in error_string.trim_start_matches("Uncaught ").lines() {
         s.push_str(&format!("\n    {}", line));
       }
@@ -209,7 +206,7 @@ fn format_js_error(js_error: &JsError, is_child: bool) -> String {
     s.push_str(&format!("\n    at {}", format_frame(frame)));
   }
   if let Some(cause) = &js_error.cause {
-    let error_string = format_js_error(cause, true);
+    let error_string = format_js_error_inner(cause, true);
     s.push_str(&format!(
       "\nCaused by: {}",
       error_string.trim_start_matches("Uncaught ")
@@ -218,33 +215,10 @@ fn format_js_error(js_error: &JsError, is_child: bool) -> String {
   s
 }
 
-/// Wrapper around deno_core::JsError which provides colorful
-/// string representation.
-#[derive(Debug)]
-pub struct PrettyJsError(JsError);
-
-impl PrettyJsError {
-  pub fn create(js_error: JsError) -> AnyError {
-    let pretty_js_error = Self(js_error);
-    pretty_js_error.into()
-  }
-}
-
-impl Deref for PrettyJsError {
-  type Target = JsError;
-  fn deref(&self) -> &Self::Target {
-    &self.0
-  }
+pub fn format_js_error(js_error: &JsError) -> String {
+  format_js_error_inner(js_error, false)
 }
 
-impl fmt::Display for PrettyJsError {
-  fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-    write!(f, "{}", &format_js_error(&self.0, false))
-  }
-}
-
-impl Error for PrettyJsError {}
-
 #[cfg(test)]
 mod tests {
   use super::*;

cli/main.rs

@@ -63,7 +63,7 @@ use crate::flags::TypeCheckMode;
 use crate::flags::UninstallFlags;
 use crate::flags::UpgradeFlags;
 use crate::flags::VendorFlags;
-use crate::fmt_errors::PrettyJsError;
+use crate::fmt_errors::format_js_error;
 use crate::graph_util::graph_lock_or_exit;
 use crate::graph_util::graph_valid;
 use crate::module_loader::CliModuleLoader;
@@ -73,6 +73,7 @@ use crate::resolver::JsxResolver;
 use deno_ast::MediaType;
 use deno_core::error::generic_error;
 use deno_core::error::AnyError;
+use deno_core::error::JsError;
 use deno_core::futures::future::FutureExt;
 use deno_core::futures::future::LocalFutureObj;
 use deno_core::futures::Future;
@@ -102,7 +103,6 @@ use std::io::Write;
 use std::iter::once;
 use std::path::PathBuf;
 use std::pin::Pin;
-use std::rc::Rc;
 use std::sync::Arc;
 
 fn create_web_worker_preload_module_callback(
@@ -166,8 +166,8 @@ fn create_web_worker_callback(ps: ProcState) -> Arc<CreateWebWorkerCb> {
       module_loader,
       create_web_worker_cb,
       preload_module_cb,
+      format_js_error_fn: Some(Arc::new(format_js_error)),
       source_map_getter: Some(Box::new(ps.clone())),
-      js_error_create_fn: Some(Rc::new(PrettyJsError::create)),
       use_deno_namespace: args.use_deno_namespace,
       worker_type: args.worker_type,
       maybe_inspector_server,
@@ -257,7 +257,7 @@ pub fn create_main_worker(
     user_agent: version::get_user_agent(),
     seed: ps.flags.seed,
     source_map_getter: Some(Box::new(ps.clone())),
-    js_error_create_fn: Some(Rc::new(PrettyJsError::create)),
+    format_js_error_fn: Some(Arc::new(format_js_error)),
     create_web_worker_cb,
     web_worker_preload_module_cb,
     maybe_inspector_server,
@@ -1442,10 +1442,14 @@ fn unwrap_or_exit<T>(result: Result<T, AnyError>) -> T {
   match result {
     Ok(value) => value,
     Err(error) => {
+      let error_string = match error.downcast_ref::<JsError>() {
+        Some(e) => format_js_error(e),
+        None => format!("{:?}", error),
+      };
       eprintln!(
         "{}: {}",
         colors::red_bold("error"),
-        format!("{:?}", error).trim_start_matches("error: ")
+        error_string.trim_start_matches("error: ")
       );
       std::process::exit(1);
     }

cli/standalone.rs

@@ -3,6 +3,7 @@
 use crate::colors;
 use crate::file_fetcher::get_source_from_data_url;
 use crate::flags::Flags;
+use crate::fmt_errors::format_js_error;
 use crate::ops;
 use crate::proc_state::ProcState;
 use crate::version;
@@ -293,7 +294,7 @@ pub async fn run(
     root_cert_store: Some(root_cert_store),
     seed: metadata.seed,
     source_map_getter: None,
-    js_error_create_fn: None,
+    format_js_error_fn: Some(Arc::new(format_js_error)),
     create_web_worker_cb,
     web_worker_preload_module_cb,
     maybe_inspector_server: None,

cli/tests/integration/compile_tests.rs

@@ -103,11 +103,15 @@ fn standalone_error() {
     .wait_with_output()
     .unwrap();
   assert!(!output.status.success());
+  println!("{:#?}", &output);
   assert_eq!(output.stdout, b"");
   let stderr = String::from_utf8(output.stderr).unwrap();
+  let stderr = util::strip_ansi_codes(&stderr).to_string();
   // On Windows, we cannot assert the file path (because '\').
   // Instead we just check for relevant output.
-  assert!(stderr.contains("error: Error: boom!\n    at boom (file://"));
+  assert!(stderr.contains("error: Uncaught Error: boom!"));
+  assert!(stderr.contains("throw new Error(\"boom!\");"));
+  assert!(stderr.contains("\n    at boom (file://"));
   assert!(stderr.contains("standalone_error.ts:2:11"));
   assert!(stderr.contains("at foo (file://"));
   assert!(stderr.contains("standalone_error.ts:5:5"));
@@ -148,9 +152,12 @@ fn standalone_error_module_with_imports() {
   println!("{:#?}", &output);
   assert_eq!(output.stdout, b"hello\n");
   let stderr = String::from_utf8(output.stderr).unwrap();
+  let stderr = util::strip_ansi_codes(&stderr).to_string();
   // On Windows, we cannot assert the file path (because '\').
   // Instead we just check for relevant output.
-  assert!(stderr.contains("error: Error: boom!\n    at file://"));
+  assert!(stderr.contains("error: Uncaught Error: boom!"));
+  assert!(stderr.contains("throw new Error(\"boom!\");"));
+  assert!(stderr.contains("\n    at file://"));
   assert!(stderr.contains("standalone_error_module_with_imports_2.ts:2:7"));
 }
 

cli/tests/integration/worker_tests.rs

@@ -104,3 +104,9 @@ itest!(worker_terminate_tla_crash {
   args: "run --quiet --reload workers/terminate_tla_crash.js",
   output: "workers/terminate_tla_crash.js.out",
 });
+
+itest!(worker_error_event {
+  args: "run --quiet -A workers/error_event.ts",
+  output: "workers/error_event.ts.out",
+  exit_code: 1,
+});

cli/tests/testdata/workers/error_event.ts

@@ -0,0 +1,11 @@
+const worker = new Worker(new URL("error.ts", import.meta.url).href, {
+  type: "module",
+});
+worker.addEventListener("error", (e) => {
+  console.log({
+    "message": e.message,
+    "filename": e.filename?.slice?.(-100),
+    "lineno": e.lineno,
+    "colno": e.colno,
+  });
+});

cli/tests/testdata/workers/error_event.ts.out

@@ -0,0 +1,13 @@
+error: Uncaught (in worker "") Error: foo
+  throw new Error("foo");
+        ^
+    at foo ([WILDCARD]/error.ts:2:9)
+    at [WILDCARD]/error.ts:5:1
+{
+  message: "Uncaught Error: foo",
+  filename: "[WILDCARD]/error.ts",
+  lineno: 2,
+  colno: 9
+}
+error: Uncaught (in promise) Error: Unhandled error in child worker.
+    at [WILDCARD]

cli/tools/test.rs

@@ -13,7 +13,7 @@ use crate::file_watcher::ResolutionResult;
 use crate::flags::Flags;
 use crate::flags::TestFlags;
 use crate::flags::TypeCheckMode;
-use crate::fmt_errors::PrettyJsError;
+use crate::fmt_errors::format_js_error;
 use crate::fs_util::collect_specifiers;
 use crate::fs_util::is_supported_test_ext;
 use crate::fs_util::is_supported_test_path;
@@ -561,8 +561,8 @@ fn abbreviate_test_error(js_error: &JsError) -> JsError {
   js_error
 }
 
-// This function maps JsError to PrettyJsError and applies some changes
-// specifically for test runner purposes:
+// This function prettifies `JsError` and applies some changes specifically for
+// test runner purposes:
 //
 // - filter out stack frames:
 //   - if stack trace consists of mixed user and internal code, the frames
@@ -574,7 +574,7 @@ pub fn format_test_error(js_error: &JsError) -> String {
     .exception_message
     .trim_start_matches("Uncaught ")
     .to_string();
-  PrettyJsError::create(js_error).to_string()
+  format_js_error(&js_error)
 }
 
 fn create_reporter(

core/error.rs

@@ -172,10 +172,6 @@ pub(crate) struct NativeJsError {
 }
 
 impl JsError {
-  pub(crate) fn create(js_error: Self) -> Error {
-    js_error.into()
-  }
-
   pub fn from_v8_exception(
     scope: &mut v8::HandleScope,
     exception: v8::Local<v8::Value>,
@@ -389,12 +385,16 @@ impl Display for JsError {
   }
 }
 
-pub(crate) fn attach_handle_to_error(
-  scope: &mut v8::Isolate,
+// TODO(piscisaureus): rusty_v8 should implement the Error trait on
+// values of type v8::Global<T>.
+pub(crate) fn to_v8_type_error(
+  scope: &mut v8::HandleScope,
   err: Error,
-  handle: v8::Local<v8::Value>,
-) -> Error {
-  ErrWithV8Handle::new(scope, err, handle).into()
+) -> v8::Global<v8::Value> {
+  let message = err.to_string();
+  let message = v8::String::new(scope, &message).unwrap();
+  let exception = v8::Exception::type_error(scope, message);
+  v8::Global::new(scope, exception)
 }
 
 /// Implements `value instanceof primordials.Error` in JS. Similar to
@@ -431,49 +431,6 @@ pub(crate) fn is_instance_of_error<'s>(
   false
 }
 
-// TODO(piscisaureus): rusty_v8 should implement the Error trait on
-// values of type v8::Global<T>.
-pub(crate) struct ErrWithV8Handle {
-  err: Error,
-  handle: v8::Global<v8::Value>,
-}
-
-impl ErrWithV8Handle {
-  pub fn new(
-    scope: &mut v8::Isolate,
-    err: Error,
-    handle: v8::Local<v8::Value>,
-  ) -> Self {
-    let handle = v8::Global::new(scope, handle);
-    Self { err, handle }
-  }
-
-  pub fn get_handle<'s>(
-    &self,
-    scope: &mut v8::HandleScope<'s>,
-  ) -> v8::Local<'s, v8::Value> {
-    v8::Local::new(scope, &self.handle)
-  }
-}
-
-#[allow(clippy::non_send_fields_in_send_ty)]
-unsafe impl Send for ErrWithV8Handle {}
-unsafe impl Sync for ErrWithV8Handle {}
-
-impl std::error::Error for ErrWithV8Handle {}
-
-impl Display for ErrWithV8Handle {
-  fn fmt(&self, f: &mut Formatter) -> fmt::Result {
-    <Error as Display>::fmt(&self.err, f)
-  }
-}
-
-impl Debug for ErrWithV8Handle {
-  fn fmt(&self, f: &mut Formatter) -> fmt::Result {
-    <Self as Display>::fmt(self, f)
-  }
-}
-
 #[cfg(test)]
 mod tests {
   use super::*;