fix: Format non-error exceptions (#14604)

This commit adds "Deno.core.setFormatExceptionCallback" which
can be used to provide custom formatting for errors. It is useful
in cases when user throws something that is non-Error (eg.
a string, plain object, etc).

cli/tests/integration/test_tests.rs

@@ -387,3 +387,9 @@ itest!(check_local_by_default2 {
   http_server: true,
   exit_code: 1,
 });
+
+itest!(non_error_thrown {
+  args: "test --quiet test/non_error_thrown.ts",
+  output: "test/non_error_thrown.out",
+  exit_code: 1,
+});

cli/tests/testdata/error_007_any.ts

@@ -1 +1 @@
-throw {};
+throw { foo: "bar" };

cli/tests/testdata/error_007_any.ts.out

@@ -1 +1 @@
-[WILDCARD]error: Uncaught #<Object>
+[WILDCARD]error: Uncaught { foo: "bar" }

cli/tests/testdata/error_cause.ts.out

@@ -11,5 +11,5 @@ Caused by: Error: bar
     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: deno
+Caused by: "deno"
 [WILDCARD]

cli/tests/testdata/test/non_error_thrown.out

@@ -0,0 +1,40 @@
+running 6 tests from [WILDCARD]/non_error_thrown.ts
+foo ... FAILED ([WILDCARD])
+bar ... FAILED ([WILDCARD])
+baz ... FAILED ([WILDCARD])
+qux ... FAILED ([WILDCARD])
+quux ... FAILED ([WILDCARD])
+quuz ... FAILED ([WILDCARD])
+
+ ERRORS 
+
+foo => [WILDCARD]/non_error_thrown.ts:1:6
+error: undefined
+
+bar => [WILDCARD]/non_error_thrown.ts:5:6
+error: null
+
+baz => [WILDCARD]/non_error_thrown.ts:9:6
+error: 123
+
+qux => [WILDCARD]/non_error_thrown.ts:13:6
+error: "Hello, world!"
+
+quux => [WILDCARD]/non_error_thrown.ts:17:6
+error: [ 1, 2, 3 ]
+
+quuz => [WILDCARD]/non_error_thrown.ts:21:6
+error: { a: "Hello, world!", b: [ 1, 2, 3 ] }
+
+ FAILURES 
+
+foo => [WILDCARD]/non_error_thrown.ts:1:6
+bar => [WILDCARD]/non_error_thrown.ts:5:6
+baz => [WILDCARD]/non_error_thrown.ts:9:6
+qux => [WILDCARD]/non_error_thrown.ts:13:6
+quux => [WILDCARD]/non_error_thrown.ts:17:6
+quuz => [WILDCARD]/non_error_thrown.ts:21:6
+
+test result: FAILED. 0 passed; 6 failed; 0 ignored; 0 measured; 0 filtered out ([WILDCARD])
+
+error: Test failed

cli/tests/testdata/test/non_error_thrown.ts

@@ -0,0 +1,23 @@
+Deno.test("foo", () => {
+  throw undefined;
+});
+
+Deno.test("bar", () => {
+  throw null;
+});
+
+Deno.test("baz", () => {
+  throw 123;
+});
+
+Deno.test("qux", () => {
+  throw "Hello, world!";
+});
+
+Deno.test("quux", () => {
+  throw [1, 2, 3];
+});
+
+Deno.test("quuz", () => {
+  throw { a: "Hello, world!", b: [1, 2, 3] };
+});

cli/tests/testdata/test/ops_sanitizer_missing_details.out

@@ -5,11 +5,12 @@ test 1 ... FAILED [WILDCARD]
  ERRORS 
 
 test 1 => ./test/ops_sanitizer_missing_details.ts:[WILDCARD]
-error: Test case is leaking async ops.
+error: AssertionError: Test case is leaking async ops.
 
-- 1 async operation to op_write was started in this test, but never completed.
+ - 1 async operation to op_write was started in this test, but never completed.
 
 To get more details where ops were leaked, run again with --trace-ops flag.
+    at [WILDCARD]
 
  FAILURES 
 

cli/tests/testdata/test/ops_sanitizer_multiple_timeout_tests.out

@@ -6,9 +6,9 @@ test 2 ... FAILED ([WILDCARD])
  ERRORS 
 
 test 1 => ./test/ops_sanitizer_multiple_timeout_tests.ts:[WILDCARD]
-error: Test case is leaking async ops.
+error: AssertionError: Test case is leaking async ops.
 
-- 2 async operations to sleep for a duration were started in this test, but never completed. This is often caused by not cancelling a `setTimeout` or `setInterval` call. The operations were started here:
+ - 2 async operations to sleep for a duration were started in this test, but never completed. This is often caused by not cancelling a `setTimeout` or `setInterval` call. The operations were started here:
     at [WILDCARD]
     at setTimeout ([WILDCARD])
     at test ([WILDCARD]/testdata/test/ops_sanitizer_multiple_timeout_tests.ts:4:3)
@@ -21,10 +21,12 @@ error: Test case is leaking async ops.
     at [WILDCARD]/testdata/test/ops_sanitizer_multiple_timeout_tests.ts:8:27
     at [WILDCARD]
 
+    at [WILDCARD]
+
 test 2 => ./test/ops_sanitizer_multiple_timeout_tests.ts:[WILDCARD]
-error: Test case is leaking async ops.
+error: AssertionError: Test case is leaking async ops.
 
-- 2 async operations to sleep for a duration were started in this test, but never completed. This is often caused by not cancelling a `setTimeout` or `setInterval` call. The operations were started here:
+ - 2 async operations to sleep for a duration were started in this test, but never completed. This is often caused by not cancelling a `setTimeout` or `setInterval` call. The operations were started here:
     at [WILDCARD]
     at setTimeout ([WILDCARD])
     at test ([WILDCARD]/testdata/test/ops_sanitizer_multiple_timeout_tests.ts:4:3)
@@ -37,6 +39,8 @@ error: Test case is leaking async ops.
     at [WILDCARD]/testdata/test/ops_sanitizer_multiple_timeout_tests.ts:10:27
     at [WILDCARD]
 
+    at [WILDCARD]
+
  FAILURES 
 
 test 1 => ./test/ops_sanitizer_multiple_timeout_tests.ts:[WILDCARD]

cli/tests/testdata/test/ops_sanitizer_multiple_timeout_tests_no_trace.out

@@ -6,18 +6,20 @@ test 2 ... FAILED ([WILDCARD])
  ERRORS 
 
 test 1 => ./test/ops_sanitizer_multiple_timeout_tests.ts:[WILDCARD]
-error: Test case is leaking async ops.
+error: AssertionError: Test case is leaking async ops.
 
-- 2 async operations to sleep for a duration were started in this test, but never completed. This is often caused by not cancelling a `setTimeout` or `setInterval` call.
+ - 2 async operations to sleep for a duration were started in this test, but never completed. This is often caused by not cancelling a `setTimeout` or `setInterval` call.
 
 To get more details where ops were leaked, run again with --trace-ops flag.
+    at [WILDCARD]
 
 test 2 => ./test/ops_sanitizer_multiple_timeout_tests.ts:[WILDCARD]
-error: Test case is leaking async ops.
+error: AssertionError: Test case is leaking async ops.
 
-- 2 async operations to sleep for a duration were started in this test, but never completed. This is often caused by not cancelling a `setTimeout` or `setInterval` call.
+ - 2 async operations to sleep for a duration were started in this test, but never completed. This is often caused by not cancelling a `setTimeout` or `setInterval` call.
 
 To get more details where ops were leaked, run again with --trace-ops flag.
+    at [WILDCARD]
 
  FAILURES 
 

cli/tests/testdata/test/ops_sanitizer_unstable.out

@@ -6,14 +6,16 @@ leak interval ... FAILED ([WILDCARD])
  ERRORS 
 
 leak interval => ./test/ops_sanitizer_unstable.ts:[WILDCARD]
-error: Test case is leaking async ops.
+error: AssertionError: Test case is leaking async ops.
 
-- 1 async operation to sleep for a duration was started in this test, but never completed. This is often caused by not cancelling a `setTimeout` or `setInterval` call. The operation was started here:
+ - 1 async operation to sleep for a duration was started in this test, but never completed. This is often caused by not cancelling a `setTimeout` or `setInterval` call. The operation was started here:
     at [WILDCARD]
     at setInterval ([WILDCARD])
     at [WILDCARD]/testdata/test/ops_sanitizer_unstable.ts:3:3
     at [WILDCARD]
 
+    at [WILDCARD]
+
  FAILURES 
 
 leak interval => ./test/ops_sanitizer_unstable.ts:[WILDCARD]

core/bindings.rs

@@ -54,6 +54,9 @@ pub static EXTERNAL_REFERENCES: Lazy<v8::ExternalReferences> =
       v8::ExternalReference {
         function: set_uncaught_exception_callback.map_fn_to(),
       },
+      v8::ExternalReference {
+        function: set_format_exception_callback.map_fn_to(),
+      },
       v8::ExternalReference {
         function: run_microtasks.map_fn_to(),
       },
@@ -214,6 +217,12 @@ pub fn initialize_context<'s>(
     "setUncaughtExceptionCallback",
     set_uncaught_exception_callback,
   );
+  set_func(
+    scope,
+    core_val,
+    "setFormatExceptionCallback",
+    set_format_exception_callback,
+  );
   set_func(scope, core_val, "runMicrotasks", run_microtasks);
   set_func(scope, core_val, "hasTickScheduled", has_tick_scheduled);
   set_func(
@@ -646,6 +655,27 @@ fn set_uncaught_exception_callback(
   }
 }
 
+/// Set a callback which formats exception messages as stored in
+/// `JsError::exception_message`. The callback is passed the error value and
+/// should return a string or `null`. If no callback is set or the callback
+/// returns `null`, the built-in default formatting will be used.
+fn set_format_exception_callback(
+  scope: &mut v8::HandleScope,
+  args: v8::FunctionCallbackArguments,
+  mut rv: v8::ReturnValue,
+) {
+  if let Ok(new) = arg0_to_cb(scope, args) {
+    if let Some(old) = JsRuntime::state(scope)
+      .borrow_mut()
+      .js_format_exception_cb
+      .replace(new)
+    {
+      let old = v8::Local::new(scope, old);
+      rv.set(old.into());
+    }
+  }
+}
+
 fn arg0_to_cb(
   scope: &mut v8::HandleScope,
   args: v8::FunctionCallbackArguments,

core/error.rs

@@ -191,6 +191,20 @@ impl JsError {
 
     let msg = v8::Exception::create_message(scope, exception);
 
+    let mut exception_message = None;
+    let state_rc = JsRuntime::state(scope);
+    let state = state_rc.borrow();
+    if let Some(format_exception_cb) = &state.js_format_exception_cb {
+      let format_exception_cb = format_exception_cb.open(scope);
+      let this = v8::undefined(scope).into();
+      let formatted = format_exception_cb.call(scope, this, &[exception]);
+      if let Some(formatted) = formatted {
+        if formatted.is_string() {
+          exception_message = Some(formatted.to_rust_string_lossy(scope));
+        }
+      }
+    }
+
     if is_instance_of_error(scope, exception) {
       // The exception is a JS Error object.
       let exception: v8::Local<v8::Object> = exception.try_into().unwrap();
@@ -200,15 +214,17 @@ impl JsError {
       // Get the message by formatting error.name and error.message.
       let name = e.name.clone().unwrap_or_else(|| "Error".to_string());
       let message_prop = e.message.clone().unwrap_or_else(|| "".to_string());
-      let exception_message = if !name.is_empty() && !message_prop.is_empty() {
-        format!("Uncaught {}: {}", name, message_prop)
-      } else if !name.is_empty() {
-        format!("Uncaught {}", name)
-      } else if !message_prop.is_empty() {
-        format!("Uncaught {}", message_prop)
-      } else {
-        "Uncaught".to_string()
-      };
+      let exception_message = exception_message.unwrap_or_else(|| {
+        if !name.is_empty() && !message_prop.is_empty() {
+          format!("Uncaught {}: {}", name, message_prop)
+        } else if !name.is_empty() {
+          format!("Uncaught {}", name)
+        } else if !message_prop.is_empty() {
+          format!("Uncaught {}", message_prop)
+        } else {
+          "Uncaught".to_string()
+        }
+      });
       let cause = cause.and_then(|cause| {
         if cause.is_undefined() || seen.contains(&cause) {
           None
@@ -334,13 +350,15 @@ impl JsError {
         aggregated,
       }
     } else {
+      let exception_message = exception_message
+        .unwrap_or_else(|| msg.get(scope).to_rust_string_lossy(scope));
       // The exception is not a JS Error object.
       // Get the message given by V8::Exception::create_message(), and provide
       // empty frames.
       Self {
         name: None,
         message: None,
-        exception_message: msg.get(scope).to_rust_string_lossy(scope),
+        exception_message,
         cause: None,
         source_line: None,
         source_line_frame_index: None,

core/runtime.rs

@@ -152,6 +152,7 @@ pub(crate) struct JsRuntimeState {
   pub(crate) js_nexttick_cbs: Vec<v8::Global<v8::Function>>,
   pub(crate) js_promise_reject_cb: Option<v8::Global<v8::Function>>,
   pub(crate) js_uncaught_exception_cb: Option<v8::Global<v8::Function>>,
+  pub(crate) js_format_exception_cb: Option<v8::Global<v8::Function>>,
   pub(crate) has_tick_scheduled: bool,
   pub(crate) js_wasm_streaming_cb: Option<v8::Global<v8::Function>>,
   pub(crate) pending_promise_exceptions:
@@ -386,6 +387,7 @@ impl JsRuntime {
       js_nexttick_cbs: vec![],
       js_promise_reject_cb: None,
       js_uncaught_exception_cb: None,
+      js_format_exception_cb: None,
       has_tick_scheduled: false,
       js_wasm_streaming_cb: None,
       source_map_getter: options.source_map_getter,

ext/console/02_console.js

@@ -2322,5 +2322,6 @@
     inspect,
     wrapConsole,
     createFilteredInspectProxy,
+    quoteString,
   };
 })(this);

runtime/js/40_testing.js

@@ -217,14 +217,16 @@
 
       let msg = `Test case is leaking async ops.
 
-- ${ArrayPrototypeJoin(details, "\n - ")}`;
+ - ${ArrayPrototypeJoin(details, "\n - ")}`;
 
       if (!core.isOpCallTracingEnabled()) {
         msg +=
           `\n\nTo get more details where ops were leaked, run again with --trace-ops flag.`;
+      } else {
+        msg += "\n";
       }
 
-      throw msg;
+      throw assert(false, msg);
     };
   }
 

runtime/js/99_main.js

@@ -38,6 +38,8 @@ delete Object.prototype.__proto__;
   const encoding = window.__bootstrap.encoding;
   const colors = window.__bootstrap.colors;
   const Console = window.__bootstrap.console.Console;
+  const inspectArgs = window.__bootstrap.console.inspectArgs;
+  const quoteString = window.__bootstrap.console.quoteString;
   const compression = window.__bootstrap.compression;
   const worker = window.__bootstrap.worker;
   const internals = window.__bootstrap.internals;
@@ -210,9 +212,26 @@ delete Object.prototype.__proto__;
     return core.opSync("op_main_module");
   }
 
+  function formatException(error) {
+    if (error instanceof Error) {
+      return null;
+    } else if (typeof error == "string") {
+      return `Uncaught ${
+        inspectArgs([quoteString(error)], {
+          colors: !colors.getNoColor(),
+        })
+      }`;
+    } else {
+      return `Uncaught ${
+        inspectArgs([error], { colors: !colors.getNoColor() })
+      }`;
+    }
+  }
+
   function runtimeStart(runtimeOptions, source) {
     core.setMacrotaskCallback(timers.handleTimerMacrotask);
     core.setWasmStreamingCallback(fetch.handleWasmStreaming);
+    core.setFormatExceptionCallback(formatException);
     version.setVersions(
       runtimeOptions.denoVersion,
       runtimeOptions.v8Version,