feat: Better formatting for AggregateError (#14285)

This commit adds "aggregated" field to "deno_core::JsError" that stores
instances of "JsError" recursively to properly handle "AggregateError"
formatting. Appropriate logics was added to "PrettyJsError" and
"console" API to format AggregateErrors.

Co-authored-by: Nayeem Rahman <nayeemrmn99@gmail.com>

cli/fmt_errors.rs

@@ -128,45 +128,6 @@ fn format_frame(frame: &JsStackFrame) -> String {
   result
 }
 
-#[allow(clippy::too_many_arguments)]
-fn format_stack(
-  is_error: bool,
-  message_line: &str,
-  cause: Option<&str>,
-  source_line: Option<&str>,
-  source_line_frame_index: Option<usize>,
-  frames: &[JsStackFrame],
-  level: usize,
-) -> String {
-  let mut s = String::new();
-  s.push_str(&format!("{:indent$}{}", "", message_line, indent = level));
-  let column_number =
-    source_line_frame_index.and_then(|i| frames.get(i).unwrap().column_number);
-  s.push_str(&format_maybe_source_line(
-    source_line,
-    column_number,
-    is_error,
-    level,
-  ));
-  for frame in frames {
-    s.push_str(&format!(
-      "\n{:indent$}    at {}",
-      "",
-      format_frame(frame),
-      indent = level
-    ));
-  }
-  if let Some(cause) = cause {
-    s.push_str(&format!(
-      "\n{:indent$}Caused by: {}",
-      "",
-      cause,
-      indent = level
-    ));
-  }
-  s
-}
-
 /// Take an optional source line and associated information to format it into
 /// a pretty printed version of that line.
 fn format_maybe_source_line(
@@ -219,6 +180,43 @@ 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 {
+  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);
+      for line in error_string.trim_start_matches("Uncaught ").lines() {
+        s.push_str(&format!("\n    {}", line));
+      }
+    }
+  }
+  let column_number = js_error
+    .source_line_frame_index
+    .and_then(|i| js_error.frames.get(i).unwrap().column_number);
+  s.push_str(&format_maybe_source_line(
+    if is_child {
+      None
+    } else {
+      js_error.source_line.as_deref()
+    },
+    column_number,
+    true,
+    0,
+  ));
+  for frame in &js_error.frames {
+    s.push_str(&format!("\n    at {}", format_frame(frame)));
+  }
+  if let Some(cause) = &js_error.cause {
+    let error_string = format_js_error(cause, true);
+    s.push_str(&format!(
+      "\nCaused by: {}",
+      error_string.trim_start_matches("Uncaught ")
+    ));
+  }
+  s
+}
+
 /// Wrapper around deno_core::JsError which provides colorful
 /// string representation.
 #[derive(Debug)]
@@ -240,26 +238,7 @@ impl Deref for PrettyJsError {
 
 impl fmt::Display for PrettyJsError {
   fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-    let cause = self
+    write!(f, "{}", &format_js_error(&self.0, false))
-      .0
-      .cause
-      .clone()
-      .map(|cause| format!("{}", PrettyJsError(*cause)));
-
-    write!(
-      f,
-      "{}",
-      &format_stack(
-        true,
-        &self.0.exception_message,
-        cause.as_deref(),
-        self.0.source_line.as_deref(),
-        self.0.source_line_frame_index,
-        &self.0.frames,
-        0
-      )
-    )?;
-    Ok(())
   }
 }
 

cli/tests/integration/run_tests.rs

@@ -2714,3 +2714,15 @@ itest!(set_timeout_error_handled {
   args: "run --quiet set_timeout_error_handled.ts",
   output: "set_timeout_error_handled.ts.out",
 });
+
+itest!(aggregate_error {
+  args: "run --quiet aggregate_error.ts",
+  output: "aggregate_error.out",
+  exit_code: 1,
+});
+
+itest!(complex_error {
+  args: "run --quiet complex_error.ts",
+  output: "complex_error.ts.out",
+  exit_code: 1,
+});

cli/tests/testdata/aggregate_error.out

@@ -0,0 +1,18 @@
+AggregateError: Multiple errors.
+    at [WILDCARD]/aggregate_error.ts:1:24
+
+AggregateError: Multiple errors.
+    Error: Error message 1.
+        at [WILDCARD]/aggregate_error.ts:2:3
+    Error: Error message 2.
+        at [WILDCARD]/aggregate_error.ts:3:3
+    at [WILDCARD]/aggregate_error.ts:1:24
+
+error: Uncaught AggregateError: Multiple errors.
+    Error: Error message 1.
+        at [WILDCARD]/aggregate_error.ts:2:3
+    Error: Error message 2.
+        at [WILDCARD]/aggregate_error.ts:3:3
+const aggregateError = new AggregateError([
+                       ^
+    at [WILDCARD]/aggregate_error.ts:1:24

cli/tests/testdata/aggregate_error.ts

@@ -0,0 +1,9 @@
+const aggregateError = new AggregateError([
+  new Error("Error message 1."),
+  new Error("Error message 2."),
+], "Multiple errors.");
+console.log(aggregateError.stack);
+console.log();
+console.log(aggregateError);
+console.log();
+throw aggregateError;

cli/tests/testdata/complex_error.ts

@@ -0,0 +1,18 @@
+const error = new AggregateError(
+  [
+    new AggregateError([new Error("qux1"), new Error("quux1")]),
+    new Error("bar1", { cause: new Error("baz1") }),
+  ],
+  "foo1",
+  {
+    cause: new AggregateError([
+      new AggregateError([new Error("qux2"), new Error("quux2")]),
+      new Error("bar2", { cause: new Error("baz2") }),
+    ], "foo2"),
+  },
+);
+console.log(error.stack);
+console.log();
+console.log(error);
+console.log();
+throw error;

cli/tests/testdata/complex_error.ts.out

@@ -0,0 +1,44 @@
+AggregateError: foo1
+    at [WILDCARD]/complex_error.ts:1:15
+
+AggregateError: foo1
+    AggregateError
+        Error: qux1
+            at [WILDCARD]/complex_error.ts:3:25
+        Error: quux1
+            at [WILDCARD]/complex_error.ts:3:44
+        at [WILDCARD]/complex_error.ts:3:5
+    Error: bar1
+        at [WILDCARD]/complex_error.ts:4:5
+    Caused by Error: baz1
+        at [WILDCARD]/complex_error.ts:4:32
+    at [WILDCARD]/complex_error.ts:1:15
+Caused by AggregateError: foo2
+    at [WILDCARD]/complex_error.ts:8:12
+
+error: Uncaught AggregateError: foo1
+    AggregateError
+        Error: qux1
+            at [WILDCARD]/complex_error.ts:3:25
+        Error: quux1
+            at [WILDCARD]/complex_error.ts:3:44
+        at [WILDCARD]/complex_error.ts:3:5
+    Error: bar1
+        at [WILDCARD]/complex_error.ts:4:5
+    Caused by: Error: baz1
+        at [WILDCARD]/complex_error.ts:4:32
+const error = new AggregateError(
+              ^
+    at [WILDCARD]/complex_error.ts:1:15
+Caused by: AggregateError: foo2
+    AggregateError
+        Error: qux2
+            at [WILDCARD]/complex_error.ts:9:27
+        Error: quux2
+            at [WILDCARD]/complex_error.ts:9:46
+        at [WILDCARD]/complex_error.ts:9:7
+    Error: bar2
+        at [WILDCARD]/complex_error.ts:10:7
+    Caused by: Error: baz2
+        at [WILDCARD]/complex_error.ts:10:34
+    at [WILDCARD]/complex_error.ts:8:12

cli/tests/testdata/error_cause.ts.out

@@ -6,12 +6,10 @@ error: Uncaught Error: foo
     at b (file:///[WILDCARD]/error_cause.ts:7:3)
     at c (file:///[WILDCARD]/error_cause.ts:11:3)
     at file:///[WILDCARD]/error_cause.ts:14:1
-Caused by: Uncaught Error: bar
+Caused by: Error: bar
-  throw new Error("foo", { cause: new Error("bar", { cause: "deno" as any }) });
-                                  ^
     at a (file:///[WILDCARD]/error_cause.ts:3:35)
     at b (file:///[WILDCARD]/error_cause.ts:7:3)
     at c (file:///[WILDCARD]/error_cause.ts:11:3)
     at file:///[WILDCARD]/error_cause.ts:14:1
-Caused by: Uncaught deno
+Caused by: deno
-[WILDCARD]
+[WILDCARD]

cli/tests/testdata/error_cause_recursive.ts.out

@@ -3,12 +3,8 @@ error: Uncaught Error: bar
 const y = new Error("bar", { cause: x });
           ^
     at file:///[WILDCARD]/error_cause_recursive.ts:2:11
-Caused by: Uncaught Error: foo
+Caused by: Error: foo
-const x = new Error("foo");
-          ^
     at file:///[WILDCARD]/error_cause_recursive.ts:1:11
-Caused by: Uncaught Error: bar
+Caused by: Error: bar
-const y = new Error("bar", { cause: x });
-          ^
     at file:///[WILDCARD]/error_cause_recursive.ts:2:11
-[WILDCARD]
+[WILDCARD]

core/error.rs

@@ -104,6 +104,7 @@ pub struct JsError {
   pub frames: Vec<JsStackFrame>,
   pub source_line: Option<String>,
   pub source_line_frame_index: Option<usize>,
+  pub aggregated: Option<Vec<JsError>>,
 }
 
 #[derive(Debug, PartialEq, Clone, serde::Deserialize, serde::Serialize)]
@@ -305,6 +306,25 @@ impl JsError {
         }
       }
 
+      // Read an array of stored errors, this is only defined for `AggregateError`
+      let aggregated_errors = get_property(scope, exception, "errors");
+      let aggregated_errors: Option<v8::Local<v8::Array>> =
+        aggregated_errors.and_then(|a| a.try_into().ok());
+
+      let mut aggregated: Option<Vec<JsError>> = None;
+
+      if let Some(errors) = aggregated_errors {
+        if errors.length() > 0 {
+          let mut agg = vec![];
+          for i in 0..errors.length() {
+            let error = errors.get_index(scope, i).unwrap();
+            let js_error = Self::from_v8_exception(scope, error);
+            agg.push(js_error);
+          }
+          aggregated = Some(agg);
+        }
+      }
+
       Self {
         name: e.name,
         message: e.message,
@@ -314,6 +334,7 @@ impl JsError {
         source_line_frame_index,
         frames,
         stack,
+        aggregated,
       }
     } else {
       // The exception is not a JS Error object.
@@ -328,6 +349,7 @@ impl JsError {
         source_line_frame_index: None,
         frames: vec![],
         stack: None,
+        aggregated: None,
       }
     }
   }

ext/console/02_console.js

@@ -9,6 +9,8 @@
   const colors = window.__bootstrap.colors;
   const {
     ArrayBufferIsView,
+    AggregateErrorPrototype,
+    ArrayPrototypeUnshift,
     isNaN,
     DataViewPrototype,
     DatePrototype,
@@ -947,16 +949,50 @@
     }
     ArrayPrototypeShift(causes);
 
-    return (MapPrototypeGet(refMap, value) ?? "") + value.stack +
+    let finalMessage = (MapPrototypeGet(refMap, value) ?? "");
-      ArrayPrototypeJoin(
+
+    if (ObjectPrototypeIsPrototypeOf(AggregateErrorPrototype, value)) {
+      const stackLines = StringPrototypeSplit(value.stack, "\n");
+      while (true) {
+        const line = ArrayPrototypeShift(stackLines);
+        if (RegExpPrototypeTest(/\s+at/, line)) {
+          ArrayPrototypeUnshift(stackLines, line);
+          break;
+        }
+
+        finalMessage += line;
+        finalMessage += "\n";
+      }
+      const aggregateMessage = ArrayPrototypeJoin(
         ArrayPrototypeMap(
-          causes,
+          value.errors,
-          (cause) =>
+          (error) =>
-            "\nCaused by " + (MapPrototypeGet(refMap, cause) ?? "") +
+            StringPrototypeReplace(
-            (cause?.stack ?? cause),
+              inspectArgs([error]),
+              /^(?!\s*$)/gm,
+              StringPrototypeRepeat(" ", 4),
+            ),
         ),
-        "",
+        "\n",
       );
+      finalMessage += aggregateMessage;
+      finalMessage += "\n";
+      finalMessage += ArrayPrototypeJoin(stackLines, "\n");
+    } else {
+      finalMessage += value.stack;
+    }
+
+    finalMessage += ArrayPrototypeJoin(
+      ArrayPrototypeMap(
+        causes,
+        (cause) =>
+          "\nCaused by " + (MapPrototypeGet(refMap, cause) ?? "") +
+          (cause?.stack ?? cause),
+      ),
+      "",
+    );
+
+    return finalMessage;
   }
 
   function inspectStringObject(value, inspectOptions) {