refactor: add core.formatLocationFilename, remove op_format_filename (#14474)

This commit moves "op_format_location" to "core/ops_builtin.rs"
and removes "Deno.core.createPrepareStackTrace" in favor of
"Deno.core.prepareStackTrace".

Co-authored-by: Aaron O'Mullan <aaron.omullan@gmail.com>

.github/workflows/ci.yml

@@ -236,7 +236,7 @@ jobs:
             ~/.cargo/registry/index
             ~/.cargo/registry/cache
             ~/.cargo/git/db
-          key: 9-cargo-home-${{ matrix.os }}-${{ hashFiles('Cargo.lock') }}
+          key: 1-cargo-home-${{ matrix.os }}-${{ hashFiles('Cargo.lock') }}
 
       # In main branch, always creates fresh cache
       - name: Cache build output (main)
@@ -252,7 +252,7 @@ jobs:
             !./target/*/*.zip
             !./target/*/*.tar.gz
           key: |
-            9-cargo-target-${{ matrix.os }}-${{ matrix.profile }}-${{ github.sha }}
+            1-cargo-target-${{ matrix.os }}-${{ matrix.profile }}-${{ github.sha }}
 
       # Restore cache from the latest 'main' branch build.
       - name: Cache build output (PR)
@@ -268,7 +268,7 @@ jobs:
             !./target/*/*.tar.gz
           key: never_saved
           restore-keys: |
-            9-cargo-target-${{ matrix.os }}-${{ matrix.profile }}-
+            1-cargo-target-${{ matrix.os }}-${{ matrix.profile }}-
 
       # Don't save cache after building PRs or branches other than 'main'.
       - name: Skip save cache (PR)

cli/fmt_errors.rs

@@ -4,36 +4,11 @@ use crate::colors::cyan;
 use crate::colors::italic_bold;
 use crate::colors::red;
 use crate::colors::yellow;
-use deno_core::error::{JsError, JsStackFrame};
-use deno_core::url::Url;
+use deno_core::error::format_file_name;
+use deno_core::error::JsError;
+use deno_core::error::JsStackFrame;
 
 const SOURCE_ABBREV_THRESHOLD: usize = 150;
-const DATA_URL_ABBREV_THRESHOLD: usize = 150;
-
-pub fn format_file_name(file_name: &str) -> String {
-  if file_name.len() > DATA_URL_ABBREV_THRESHOLD {
-    if let Ok(url) = Url::parse(file_name) {
-      if url.scheme() == "data" {
-        let data_path = url.path();
-        if let Some(data_pieces) = data_path.split_once(',') {
-          let data_length = data_pieces.1.len();
-          if let Some(data_start) = data_pieces.1.get(0..20) {
-            if let Some(data_end) = data_pieces.1.get(data_length - 20..) {
-              return format!(
-                "{}:{},{}......{}",
-                url.scheme(),
-                data_pieces.0,
-                data_start,
-                data_end
-              );
-            }
-          }
-        }
-      }
-    }
-  }
-  file_name.to_string()
-}
 
 // Keep in sync with `/core/error.js`.
 pub fn format_location(frame: &JsStackFrame) -> String {

cli/ops/errors.rs

@@ -1,7 +1,6 @@
 // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license.
 
 use crate::diagnostics::Diagnostics;
-use crate::fmt_errors::format_file_name;
 use deno_core::error::AnyError;
 use deno_core::op;
 use deno_core::serde_json;
@@ -11,10 +10,7 @@ use deno_core::Extension;
 
 pub fn init() -> Extension {
   Extension::builder()
-    .ops(vec![
-      op_format_diagnostic::decl(),
-      op_format_file_name::decl(),
-    ])
+    .ops(vec![op_format_diagnostic::decl()])
     .build()
 }
 
@@ -23,8 +19,3 @@ fn op_format_diagnostic(args: Value) -> Result<Value, AnyError> {
   let diagnostic: Diagnostics = serde_json::from_value(args)?;
   Ok(json!(diagnostic.to_string()))
 }
-
-#[op]
-fn op_format_file_name(file_name: String) -> Result<String, AnyError> {
-  Ok(format_file_name(&file_name))
-}

core/02_error.js

@@ -71,7 +71,7 @@
   }
 
   // Keep in sync with `cli/fmt_errors.rs`.
-  function formatLocation(callSite, formatFileNameFn) {
+  function formatLocation(callSite) {
     if (callSite.isNative()) {
       return "native";
     }
@@ -81,7 +81,7 @@
     const fileName = callSite.getFileName();
 
     if (fileName) {
-      result += formatFileNameFn(fileName);
+      result += core.opSync("op_format_file_name", fileName);
     } else {
       if (callSite.isEval()) {
         const evalOrigin = callSite.getEvalOrigin();
@@ -107,7 +107,7 @@
   }
 
   // Keep in sync with `cli/fmt_errors.rs`.
-  function formatCallSite(callSite, formatFileNameFn) {
+  function formatCallSite(callSite) {
     let result = "";
     const functionName = callSite.getFunctionName();
 
@@ -160,11 +160,11 @@
     } else if (functionName) {
       result += functionName;
     } else {
-      result += formatLocation(callSite, formatFileNameFn);
+      result += formatLocation(callSite);
       return result;
     }
 
-    result += ` (${formatLocation(callSite, formatFileNameFn)})`;
+    result += ` (${formatLocation(callSite)})`;
     return result;
   }
 
@@ -189,57 +189,55 @@
     };
   }
 
-  /** Returns a function that can be used as `Error.prepareStackTrace`. */
-  function createPrepareStackTrace(formatFileNameFn) {
-    return function prepareStackTrace(
-      error,
-      callSites,
-    ) {
-      const mappedCallSites = ArrayPrototypeMap(callSites, (callSite) => {
-        const fileName = callSite.getFileName();
-        const lineNumber = callSite.getLineNumber();
-        const columnNumber = callSite.getColumnNumber();
-        if (fileName && lineNumber != null && columnNumber != null) {
-          return patchCallSite(
-            callSite,
-            core.applySourceMap({
-              fileName,
-              lineNumber,
-              columnNumber,
-            }),
-          );
-        }
-        return callSite;
-      });
-      ObjectDefineProperties(error, {
-        __callSiteEvals: { value: [], configurable: true },
-      });
-      const formattedCallSites = [];
-      for (const callSite of mappedCallSites) {
-        ArrayPrototypePush(error.__callSiteEvals, evaluateCallSite(callSite));
-        ArrayPrototypePush(
-          formattedCallSites,
-          formatCallSite(callSite, formatFileNameFn),
+  /** A function that can be used as `Error.prepareStackTrace`. */
+  function prepareStackTrace(
+    error,
+    callSites,
+  ) {
+    const mappedCallSites = ArrayPrototypeMap(callSites, (callSite) => {
+      const fileName = callSite.getFileName();
+      const lineNumber = callSite.getLineNumber();
+      const columnNumber = callSite.getColumnNumber();
+      if (fileName && lineNumber != null && columnNumber != null) {
+        return patchCallSite(
+          callSite,
+          core.applySourceMap({
+            fileName,
+            lineNumber,
+            columnNumber,
+          }),
         );
       }
-      const message = error.message !== undefined ? error.message : "";
-      const name = error.name !== undefined ? error.name : "Error";
-      let messageLine;
-      if (name != "" && message != "") {
-        messageLine = `${name}: ${message}`;
-      } else if ((name || message) != "") {
-        messageLine = name || message;
-      } else {
-        messageLine = "";
-      }
-      return messageLine +
-        ArrayPrototypeJoin(
-          ArrayPrototypeMap(formattedCallSites, (s) => `\n    at ${s}`),
-          "",
-        );
-    };
+      return callSite;
+    });
+    ObjectDefineProperties(error, {
+      __callSiteEvals: { value: [], configurable: true },
+    });
+    const formattedCallSites = [];
+    for (const callSite of mappedCallSites) {
+      ArrayPrototypePush(error.__callSiteEvals, evaluateCallSite(callSite));
+      ArrayPrototypePush(
+        formattedCallSites,
+        formatCallSite(callSite),
+      );
+    }
+    const message = error.message !== undefined ? error.message : "";
+    const name = error.name !== undefined ? error.name : "Error";
+    let messageLine;
+    if (name != "" && message != "") {
+      messageLine = `${name}: ${message}`;
+    } else if ((name || message) != "") {
+      messageLine = name || message;
+    } else {
+      messageLine = "";
+    }
+    return messageLine +
+      ArrayPrototypeJoin(
+        ArrayPrototypeMap(formattedCallSites, (s) => `\n    at ${s}`),
+        "",
+      );
   }
 
-  ObjectAssign(globalThis.__bootstrap.core, { createPrepareStackTrace });
+  ObjectAssign(globalThis.__bootstrap.core, { prepareStackTrace });
   ObjectFreeze(globalThis.__bootstrap.core);
 })(this);

core/bindings.rs

@@ -243,6 +243,7 @@ pub fn initialize_context<'s>(
   set_func(scope, core_val, "destructureError", destructure_error);
   set_func(scope, core_val, "terminate", terminate);
   set_func(scope, core_val, "applySourceMap", apply_source_map);
+
   // Direct bindings on `window`.
   set_func(scope, global, "queueMicrotask", queue_microtask);
 

core/error.rs

@@ -2,6 +2,7 @@
 
 use crate::runtime::JsRuntime;
 use crate::source_map::apply_source_map;
+use crate::url::Url;
 use anyhow::Error;
 use std::borrow::Cow;
 use std::collections::HashMap;
@@ -431,6 +432,27 @@ pub(crate) fn is_instance_of_error<'s>(
   false
 }
 
+const DATA_URL_ABBREV_THRESHOLD: usize = 150;
+
+pub fn format_file_name(file_name: &str) -> String {
+  abbrev_file_name(file_name).unwrap_or_else(|| file_name.to_string())
+}
+
+fn abbrev_file_name(file_name: &str) -> Option<String> {
+  if file_name.len() <= DATA_URL_ABBREV_THRESHOLD {
+    return None;
+  }
+  let url = Url::parse(file_name).ok()?;
+  if url.scheme() != "data" {
+    return None;
+  }
+  let (head, tail) = url.path().split_once(',')?;
+  let len = tail.len();
+  let start = tail.get(0..20)?;
+  let end = tail.get(len - 20..)?;
+  Some(format!("{}:{},{}......{}", url.scheme(), head, start, end))
+}
+
 #[cfg(test)]
 mod tests {
   use super::*;

core/ops_builtin.rs

@@ -1,3 +1,4 @@
+use crate::error::format_file_name;
 use crate::error::type_error;
 use crate::include_js_files;
 use crate::ops_metrics::OpMetrics;
@@ -34,6 +35,7 @@ pub(crate) fn init_builtins() -> Extension {
       op_write::decl(),
       op_shutdown::decl(),
       op_metrics::decl(),
+      op_format_file_name::decl(),
     ])
     .build()
 }
@@ -183,3 +185,8 @@ async fn op_shutdown(
   let resource = state.borrow().resource_table.get_any(rid)?;
   resource.shutdown().await
 }
+
+#[op]
+fn op_format_file_name(file_name: String) -> Result<String, Error> {
+  Ok(format_file_name(&file_name))
+}

runtime/js/40_error_stack.js

@@ -8,12 +8,7 @@
     return core.opSync("op_format_diagnostic", diagnostics);
   }
 
-  function opFormatFileName(location) {
-    return core.opSync("op_format_file_name", location);
-  }
-
   window.__bootstrap.errorStack = {
     opFormatDiagnostics,
-    opFormatFileName,
   };
 })(this);

runtime/js/99_main.js

@@ -32,7 +32,6 @@ delete Object.prototype.__proto__;
   const location = window.__bootstrap.location;
   const build = window.__bootstrap.build;
   const version = window.__bootstrap.version;
-  const errorStack = window.__bootstrap.errorStack;
   const os = window.__bootstrap.os;
   const timers = window.__bootstrap.timers;
   const base64 = window.__bootstrap.base64;
@@ -221,11 +220,8 @@ delete Object.prototype.__proto__;
     );
     build.setBuildInfo(runtimeOptions.target);
     util.setLogDebug(runtimeOptions.debugFlag, source);
-    const prepareStackTrace = core.createPrepareStackTrace(
-      errorStack.opFormatFileName,
-    );
     // deno-lint-ignore prefer-primordials
-    Error.prepareStackTrace = prepareStackTrace;
+    Error.prepareStackTrace = core.prepareStackTrace;
   }
 
   function registerErrors() {