feat(cli/source_map): Use top user frame for error source lines (#9604)

This commit changes formatting of JS errors; by not showing 
source lines for internal code. Where possible, instead using 
the top stack frame associated with user code i.e. the first 
location that is colourful and not a "deno:" URL.

cli/ops/errors.rs

@@ -42,7 +42,6 @@ fn op_apply_source_map(
       args.file_name,
       args.line_number.into(),
       args.column_number.into(),
-      None,
       &mut mappings_map,
       program_state,
     );

cli/source_maps.rs

@@ -39,9 +39,8 @@ pub fn apply_source_map<G: SourceMapGetter>(
       js_error.line_number,
       // start_column is 0-based, we need 1-based here.
       js_error.start_column.map(|n| n + 1),
-      js_error.source_line.clone(),
       &mut mappings_map,
-      getter,
+      getter.clone(),
     );
   let start_column = start_column.map(|n| n - 1);
   // It is better to just move end_column to be the same distance away from
@@ -58,6 +57,33 @@ pub fn apply_source_map<G: SourceMapGetter>(
     _ => None,
   };
 
+  let mut script_resource_name = script_resource_name;
+  let mut line_number = line_number;
+  let mut start_column = start_column;
+  let mut end_column = end_column;
+  let mut source_line = source_line;
+  if script_resource_name
+    .as_ref()
+    .map_or(true, |s| s.starts_with("deno:"))
+  {
+    for frame in &js_error.frames {
+      if let (Some(file_name), Some(line_number_)) =
+        (&frame.file_name, frame.line_number)
+      {
+        if !file_name.starts_with("deno:") {
+          script_resource_name = Some(file_name.clone());
+          line_number = Some(line_number_);
+          start_column = frame.column_number.map(|n| n - 1);
+          end_column = frame.column_number;
+          // Lookup expects 0-based line and column numbers, but ours are 1-based.
+          source_line =
+            getter.get_source_line(file_name, (line_number_ - 1) as usize);
+          break;
+        }
+      }
+    }
+  }
+
   JsError {
     message: js_error.message.clone(),
     source_line,
@@ -74,21 +100,13 @@ fn get_maybe_orig_position<G: SourceMapGetter>(
   file_name: Option<String>,
   line_number: Option<i64>,
   column_number: Option<i64>,
-  source_line: Option<String>,
   mappings_map: &mut CachedMaps,
   getter: Arc<G>,
 ) -> (Option<String>, Option<i64>, Option<i64>, Option<String>) {
   match (file_name, line_number, column_number) {
     (Some(file_name_v), Some(line_v), Some(column_v)) => {
       let (file_name, line_number, column_number, source_line) =
-        get_orig_position(
+        get_orig_position(file_name_v, line_v, column_v, mappings_map, getter);
-          file_name_v,
-          line_v,
-          column_v,
-          source_line,
-          mappings_map,
-          getter,
-        );
       (
         Some(file_name),
         Some(line_number),
@@ -96,7 +114,7 @@ fn get_maybe_orig_position<G: SourceMapGetter>(
         source_line,
       )
     }
-    _ => (None, None, None, source_line),
+    _ => (None, None, None, None),
   }
 }
 
@@ -104,62 +122,55 @@ pub fn get_orig_position<G: SourceMapGetter>(
   file_name: String,
   line_number: i64,
   column_number: i64,
-  source_line: Option<String>,
   mappings_map: &mut CachedMaps,
   getter: Arc<G>,
 ) -> (String, i64, i64, Option<String>) {
-  let maybe_source_map = get_mappings(&file_name, mappings_map, getter.clone());
-  let default_pos =
-    (file_name.clone(), line_number, column_number, source_line);
-
   // Lookup expects 0-based line and column numbers, but ours are 1-based.
   let line_number = line_number - 1;
   let column_number = column_number - 1;
 
-  match maybe_source_map {
+  let default_pos = (file_name.clone(), line_number, column_number, None);
-    None => default_pos,
+  let maybe_source_map = get_mappings(&file_name, mappings_map, getter.clone());
-    Some(source_map) => {
+  let (file_name, line_number, column_number, source_line) =
-      match source_map.lookup_token(line_number as u32, column_number as u32) {
+    match maybe_source_map {
-        None => default_pos,
+      None => default_pos,
-        Some(token) => match token.get_source() {
+      Some(source_map) => {
+        match source_map.lookup_token(line_number as u32, column_number as u32)
+        {
           None => default_pos,
-          Some(source_file_name) => {
+          Some(token) => match token.get_source() {
-            // The `source_file_name` written by tsc in the source map is
+            None => default_pos,
-            // sometimes only the basename of the URL, or has unwanted `<`/`>`
+            Some(source_file_name) => {
-            // around it. Use the `file_name` we get from V8 if
+              // The `source_file_name` written by tsc in the source map is
-            // `source_file_name` does not parse as a URL.
+              // sometimes only the basename of the URL, or has unwanted `<`/`>`
-            let file_name = match deno_core::resolve_url(source_file_name) {
+              // around it. Use the `file_name` we get from V8 if
-              Ok(m) => m.to_string(),
+              // `source_file_name` does not parse as a URL.
-              Err(_) => file_name,
+              let file_name = match deno_core::resolve_url(source_file_name) {
-            };
+                Ok(m) => m.to_string(),
-            let maybe_source_line =
+                Err(_) => file_name,
-              if let Some(source_view) = token.get_source_view() {
-                source_view.get_line(token.get_src_line())
-              } else {
-                None
               };
-
+              let source_line =
-            let source_line = if let Some(source_line) = maybe_source_line {
+                if let Some(source_view) = token.get_source_view() {
-              Some(source_line.to_string())
+                  source_view
-            } else if let Some(source_line) =
+                    .get_line(token.get_src_line())
-              getter.get_source_line(&file_name, token.get_src_line() as usize)
+                    .map(|s| s.to_string())
-            {
+                } else {
-              Some(source_line)
+                  None
-            } else {
+                };
-              None
+              (
-            };
+                file_name,
-
+                i64::from(token.get_src_line()),
-            (
+                i64::from(token.get_src_col()),
-              file_name,
+                source_line,
-              i64::from(token.get_src_line()) + 1,
+              )
-              i64::from(token.get_src_col()) + 1,
+            }
-              source_line,
+          },
-            )
+        }
-          }
-        },
       }
-    }
+    };
-  }
+  let source_line = source_line
+    .or_else(|| getter.get_source_line(&file_name, line_number as usize));
+  (file_name, line_number + 1, column_number + 1, source_line)
 }
 
 fn get_mappings<'a, G: SourceMapGetter>(

cli/tests/059_fs_relative_path_perm.ts.out

@@ -1,4 +1,4 @@
 [WILDCARD]error: Uncaught PermissionDenied: read access to "non-existent", run again with the --allow-read flag
-    throw new ErrorClass(res.err.message, ...args);
+Deno.readFileSync("non-existent");
-          ^
+     ^
     at [WILDCARD]

cli/tests/error_009_op_crates_error.js.out

@@ -1,3 +1,6 @@
-[WILDCARD]error: Uncaught TypeError: Failed to construct 'Event': 1 argument, but only 0 present.[WILDCARD]
+[WILDCARD]error: Uncaught TypeError: Failed to construct 'Event': 1 argument, but only 0 present.
+new Event();
+^
+    at [WILDCARD]
     at new Event (deno:op_crates/web/[WILDCARD])
     at [WILDCARD]

cli/tests/error_019_stack_function.ts.out

@@ -2,5 +2,7 @@
     at foo ([WILDCARD]tests/error_019_stack_function.ts:[WILDCARD])
     at [WILDCARD]tests/error_019_stack_function.ts:[WILDCARD]
 error: Uncaught Error: function
+  throw new Error("function");
+        ^
     at foo ([WILDCARD]tests/error_019_stack_function.ts:[WILDCARD])
     at [WILDCARD]tests/error_019_stack_function.ts:[WILDCARD]

cli/tests/error_020_stack_constructor.ts.out

@@ -2,5 +2,7 @@
     at new A ([WILDCARD]tests/error_020_stack_constructor.ts:[WILDCARD])
     at [WILDCARD]tests/error_020_stack_constructor.ts:[WILDCARD]
 error: Uncaught Error: constructor
+    throw new Error("constructor");
+          ^
     at new A ([WILDCARD]tests/error_020_stack_constructor.ts:[WILDCARD])
     at [WILDCARD]tests/error_020_stack_constructor.ts:[WILDCARD]

cli/tests/error_021_stack_method.ts.out

@@ -2,5 +2,7 @@
     at A.m ([WILDCARD]tests/error_021_stack_method.ts:[WILDCARD])
     at [WILDCARD]tests/error_021_stack_method.ts:[WILDCARD]
 error: Uncaught Error: method
+    throw new Error("method");
+          ^
     at A.m ([WILDCARD]tests/error_021_stack_method.ts:[WILDCARD])
     at [WILDCARD]tests/error_021_stack_method.ts:[WILDCARD]

cli/tests/error_022_stack_custom_error.ts.out

@@ -1,4 +1,6 @@
 [WILDCARD]CustomError: custom error
     at [WILDCARD]tests/error_022_stack_custom_error.ts:[WILDCARD]
 error: Uncaught CustomError: custom error
+const error = new CustomError();
+              ^
     at [WILDCARD]tests/error_022_stack_custom_error.ts:[WILDCARD]

cli/tests/error_023_stack_async.ts.out

@@ -3,6 +3,8 @@
     at async [WILDCARD]tests/error_023_stack_async.ts:[WILDCARD]
     at async [WILDCARD]tests/error_023_stack_async.ts:[WILDCARD]
 error: Uncaught Error: async
+    throw new Error("async");
+          ^
     at [WILDCARD]tests/error_023_stack_async.ts:[WILDCARD]
     at async [WILDCARD]tests/error_023_stack_async.ts:[WILDCARD]
     at async [WILDCARD]tests/error_023_stack_async.ts:[WILDCARD]

cli/tests/error_024_stack_promise_all.ts.out

@@ -3,6 +3,8 @@
     at async Promise.all (index 1)
     at async [WILDCARD]tests/error_024_stack_promise_all.ts:[WILDCARD]
 error: Uncaught Error: Promise.all()
+    throw new Error("Promise.all()");
+          ^
     at [WILDCARD]tests/error_024_stack_promise_all.ts:[WILDCARD]
     at async Promise.all (index 1)
     at async [WILDCARD]tests/error_024_stack_promise_all.ts:[WILDCARD]

cli/tests/workers/worker_error.ts.out

@@ -2,6 +2,4 @@
     at foo ([WILDCARD])
     at [WILDCARD]
 error: Uncaught (in promise) Error: Unhandled error event reached main worker.
-              throw new Error("Unhandled error event reached main worker.");
-                    ^
     at Worker.#poll ([WILDCARD])

cli/tests/workers/worker_nested_error.ts.out

@@ -2,6 +2,4 @@
     at foo ([WILDCARD])
     at [WILDCARD]
 error: Uncaught (in promise) Error: Unhandled error event reached main worker.
-              throw new Error("Unhandled error event reached main worker.");
-                    ^
     at Worker.#poll ([WILDCARD])