refactor: Remove PrettyJsError and js_error_create_fn (#14378)

This commit:
- removes "fmt_errors::PrettyJsError" in favor of "format_js_error" fn
- removes "deno_core::JsError::create" and 
"deno_core::RuntimeOptions::js_error_create_fn"
- adds new option to "deno_runtime::ops::worker_host::init"

5 files changed, 44 insertions, 17 deletions

diff --git a/runtime/examples/hello_runtime.rs b/runtime/examples/hello_runtime.rs
index cdffa9713..a6b7fb06b 100644
--- a/runtime/examples/hello_runtime.rs
+++ b/runtime/examples/hello_runtime.rs
@@ -45,7 +45,7 @@ async fn main() -> Result<(), AnyError> {
     user_agent: "hello_runtime".to_string(),
     seed: None,
     source_map_getter: None,
-    js_error_create_fn: None,
+    format_js_error_fn: None,
     web_worker_preload_module_cb,
     create_web_worker_cb,
     maybe_inspector_server: None,
diff --git a/runtime/js/11_workers.js b/runtime/js/11_workers.js
index 5b8d03e71..87949c1c0 100644
--- a/runtime/js/11_workers.js
+++ b/runtime/js/11_workers.js
@@ -134,17 +134,16 @@
       const event = new ErrorEvent("error", {
         cancelable: true,
         message: e.message,
-        lineno: e.lineNumber ? e.lineNumber + 1 : undefined,
-        colno: e.columnNumber ? e.columnNumber + 1 : undefined,
+        lineno: e.lineNumber ? e.lineNumber : undefined,
+        colno: e.columnNumber ? e.columnNumber : undefined,
         filename: e.fileName,
         error: null,
       });
 
       this.dispatchEvent(event);
       // Don't bubble error event to window for loader errors (`!e.fileName`).
-      // TODO(nayeemrmn): Currently these are never bubbled because worker
-      // error event fields aren't populated correctly and `e.fileName` is
-      // always empty.
+      // TODO(nayeemrmn): It's not correct to use `e.fileName` to detect user
+      // errors. It won't be there for non-awaited async ops for example.
       if (e.fileName && !event.defaultPrevented) {
         window.dispatchEvent(event);
       }
diff --git a/runtime/ops/worker_host.rs b/runtime/ops/worker_host.rs
index 6c691c5a0..23ffcd8b1 100644
--- a/runtime/ops/worker_host.rs
+++ b/runtime/ops/worker_host.rs
@@ -11,6 +11,7 @@ use crate::web_worker::WebWorkerHandle;
 use crate::web_worker::WebWorkerType;
 use crate::web_worker::WorkerControlEvent;
 use crate::web_worker::WorkerId;
+use crate::worker::FormatJsErrorFn;
 use deno_core::error::AnyError;
 use deno_core::futures::future::LocalFutureObj;
 use deno_core::op;
@@ -54,6 +55,9 @@ pub type PreloadModuleCb = dyn Fn(WebWorker) -> LocalFutureObj<'static, Result<W
 #[derive(Clone)]
 pub struct CreateWebWorkerCbHolder(Arc<CreateWebWorkerCb>);
 
+#[derive(Clone)]
+pub struct FormatJsErrorFnHolder(Option<Arc<FormatJsErrorFn>>);
+
 /// A holder for callback that can used to preload some modules into a WebWorker
 /// before actual worker code is executed. It's a struct instead of a type
 /// because `GothamState` used in `OpState` overrides
@@ -106,6 +110,7 @@ pub type WorkersTable = HashMap<WorkerId, WorkerThread>;
 pub fn init(
   create_web_worker_cb: Arc<CreateWebWorkerCb>,
   preload_module_cb: Arc<PreloadModuleCb>,
+  format_js_error_fn: Option<Arc<FormatJsErrorFn>>,
 ) -> Extension {
   Extension::builder()
     .state(move |state| {
@@ -118,6 +123,9 @@ pub fn init(
       let preload_module_cb_holder =
         PreloadModuleCbHolder(preload_module_cb.clone());
       state.put::<PreloadModuleCbHolder>(preload_module_cb_holder);
+      let format_js_error_fn_holder =
+        FormatJsErrorFnHolder(format_js_error_fn.clone());
+      state.put::<FormatJsErrorFnHolder>(format_js_error_fn_holder);
 
       Ok(())
     })
@@ -193,6 +201,8 @@ fn op_create_worker(
   state.put::<CreateWebWorkerCbHolder>(create_web_worker_cb.clone());
   let preload_module_cb = state.take::<PreloadModuleCbHolder>();
   state.put::<PreloadModuleCbHolder>(preload_module_cb.clone());
+  let format_js_error_fn = state.take::<FormatJsErrorFnHolder>();
+  state.put::<FormatJsErrorFnHolder>(format_js_error_fn.clone());
   state.put::<WorkerId>(worker_id.next().unwrap());
 
   let module_specifier = deno_core::resolve_url(&specifier)?;
@@ -238,6 +248,7 @@ fn op_create_worker(
       module_specifier,
       maybe_source_code,
       preload_module_cb.0,
+      format_js_error_fn.0,
     )
   })?;
 
diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs
index a1f5ea2ee..9f120539c 100644
--- a/runtime/web_worker.rs
+++ b/runtime/web_worker.rs
@@ -6,6 +6,7 @@ use crate::ops;
 use crate::ops::io::Stdio;
 use crate::permissions::Permissions;
 use crate::tokio_util::run_basic;
+use crate::worker::FormatJsErrorFn;
 use crate::BootstrapOptions;
 use deno_broadcast_channel::InMemoryBroadcastChannel;
 use deno_core::error::AnyError;
@@ -23,7 +24,6 @@ use deno_core::CancelHandle;
 use deno_core::CompiledWasmModuleStore;
 use deno_core::Extension;
 use deno_core::GetErrorClassFn;
-use deno_core::JsErrorCreateFn;
 use deno_core::JsRuntime;
 use deno_core::ModuleId;
 use deno_core::ModuleLoader;
@@ -94,7 +94,10 @@ impl Serialize for WorkerControlEvent {
       | WorkerControlEvent::Error(error) => {
         let value = match error.downcast_ref::<JsError>() {
           Some(js_error) => {
-            let frame = js_error.frames.first();
+            let frame = js_error.frames.iter().find(|f| match &f.file_name {
+              Some(s) => !s.trim_start_matches('[').starts_with("deno:"),
+              None => false,
+            });
             json!({
               "message": js_error.exception_message,
               "fileName": frame.map(|f| f.file_name.as_ref()),
@@ -325,8 +328,8 @@ pub struct WebWorkerOptions {
   pub module_loader: Rc<dyn ModuleLoader>,
   pub create_web_worker_cb: Arc<ops::worker_host::CreateWebWorkerCb>,
   pub preload_module_cb: Arc<ops::worker_host::PreloadModuleCb>,
+  pub format_js_error_fn: Option<Arc<FormatJsErrorFn>>,
   pub source_map_getter: Option<Box<dyn SourceMapGetter>>,
-  pub js_error_create_fn: Option<Rc<JsErrorCreateFn>>,
   pub use_deno_namespace: bool,
   pub worker_type: WebWorkerType,
   pub maybe_inspector_server: Option<Arc<InspectorServer>>,
@@ -408,6 +411,7 @@ impl WebWorker {
       ops::worker_host::init(
         options.create_web_worker_cb.clone(),
         options.preload_module_cb.clone(),
+        options.format_js_error_fn.clone(),
       ),
       // Extensions providing Deno.* features
       ops::fs_events::init().enabled(options.use_deno_namespace),
@@ -445,7 +449,6 @@ impl WebWorker {
       module_loader: Some(options.module_loader.clone()),
       startup_snapshot: Some(js::deno_isolate_init()),
       source_map_getter: options.source_map_getter,
-      js_error_create_fn: options.js_error_create_fn.clone(),
       get_error_class_fn: options.get_error_class_fn,
       shared_array_buffer_store: options.shared_array_buffer_store.clone(),
       compiled_wasm_module_store: options.compiled_wasm_module_store.clone(),
@@ -646,7 +649,18 @@ impl WebWorker {
   }
 }
 
-fn print_worker_error(error_str: String, name: &str) {
+fn print_worker_error(
+  error: &AnyError,
+  name: &str,
+  format_js_error_fn: Option<&FormatJsErrorFn>,
+) {
+  let error_str = match format_js_error_fn {
+    Some(format_js_error_fn) => match error.downcast_ref::<JsError>() {
+      Some(js_error) => format_js_error_fn(js_error),
+      None => error.to_string(),
+    },
+    None => error.to_string(),
+  };
   eprintln!(
     "{}: Uncaught (in worker \"{}\") {}",
     colors::red_bold("error"),
@@ -662,6 +676,7 @@ pub fn run_web_worker(
   specifier: ModuleSpecifier,
   maybe_source_code: Option<String>,
   preload_module_cb: Arc<ops::worker_host::PreloadModuleCb>,
+  format_js_error_fn: Option<Arc<FormatJsErrorFn>>,
 ) -> Result<(), AnyError> {
   let name = worker.name.to_string();
 
@@ -675,7 +690,7 @@ pub fn run_web_worker(
     let mut worker = match result {
       Ok(worker) => worker,
       Err(e) => {
-        print_worker_error(e.to_string(), &name);
+        print_worker_error(&e, &name, format_js_error_fn.as_deref());
         internal_handle
           .post_event(WorkerControlEvent::TerminalError(e))
           .expect("Failed to post message to host");
@@ -715,7 +730,7 @@ pub fn run_web_worker(
     };
 
     if let Err(e) = result {
-      print_worker_error(e.to_string(), &name);
+      print_worker_error(&e, &name, format_js_error_fn.as_deref());
       internal_handle
         .post_event(WorkerControlEvent::TerminalError(e))
         .expect("Failed to post message to host");
diff --git a/runtime/worker.rs b/runtime/worker.rs
index 15f41fe56..23fe8e4c0 100644
--- a/runtime/worker.rs
+++ b/runtime/worker.rs
@@ -8,12 +8,12 @@ use crate::permissions::Permissions;
 use crate::BootstrapOptions;
 use deno_broadcast_channel::InMemoryBroadcastChannel;
 use deno_core::error::AnyError;
+use deno_core::error::JsError;
 use deno_core::futures::Future;
 use deno_core::located_script_name;
 use deno_core::CompiledWasmModuleStore;
 use deno_core::Extension;
 use deno_core::GetErrorClassFn;
-use deno_core::JsErrorCreateFn;
 use deno_core::JsRuntime;
 use deno_core::LocalInspectorSession;
 use deno_core::ModuleId;
@@ -33,6 +33,8 @@ use std::sync::Arc;
 use std::task::Context;
 use std::task::Poll;
 
+pub type FormatJsErrorFn = dyn Fn(&JsError) -> String + Sync + Send;
+
 /// This worker is created and used by almost all
 /// subcommands in Deno executable.
 ///
@@ -56,8 +58,8 @@ pub struct WorkerOptions {
   // Callbacks invoked when creating new instance of WebWorker
   pub create_web_worker_cb: Arc<ops::worker_host::CreateWebWorkerCb>,
   pub web_worker_preload_module_cb: Arc<ops::worker_host::PreloadModuleCb>,
+  pub format_js_error_fn: Option<Arc<FormatJsErrorFn>>,
   pub source_map_getter: Option<Box<dyn SourceMapGetter>>,
-  pub js_error_create_fn: Option<Rc<JsErrorCreateFn>>,
   pub maybe_inspector_server: Option<Arc<InspectorServer>>,
   pub should_break_on_first_statement: bool,
   pub get_error_class_fn: Option<GetErrorClassFn>,
@@ -133,6 +135,7 @@ impl MainWorker {
       ops::worker_host::init(
         options.create_web_worker_cb.clone(),
         options.web_worker_preload_module_cb.clone(),
+        options.format_js_error_fn.clone(),
       ),
       ops::spawn::init(),
       ops::fs_events::init(),
@@ -161,7 +164,6 @@ impl MainWorker {
       module_loader: Some(options.module_loader.clone()),
       startup_snapshot: Some(js::deno_isolate_init()),
       source_map_getter: options.source_map_getter,
-      js_error_create_fn: options.js_error_create_fn.clone(),
       get_error_class_fn: options.get_error_class_fn,
       shared_array_buffer_store: options.shared_array_buffer_store.clone(),
       compiled_wasm_module_store: options.compiled_wasm_module_store.clone(),
@@ -379,8 +381,8 @@ mod tests {
       unsafely_ignore_certificate_errors: None,
       root_cert_store: None,
       seed: None,
+      format_js_error_fn: None,
       source_map_getter: None,
-      js_error_create_fn: None,
       web_worker_preload_module_cb: Arc::new(|_| unreachable!()),
       create_web_worker_cb: Arc::new(|_| unreachable!()),
       maybe_inspector_server: None,