From 642118fdeb06817636fe03d0d8428fbd7927094d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0pa=C4=8Dek?= Date: Wed, 26 Oct 2022 17:30:44 +0200 Subject: [PATCH] fix(core) Include causes when converting anyhow errors to JS exceptions (#16397) When an op returns an `anyhow` error with a cause (usually added using the `.context()` method), the `Error` thrown into JavaScript contains only the message of the outernmost error in the chain. This PR simply changes the formatting of `anyhow::Error` from `"{}"` to `"{:#}"`: This significantly improves errors for code that embeds Deno and defines custom ops. For example, in [chiselstrike/chiselstrike](https://github.com/chiselstrike/chiselstrike), this PR improves an error message like ``` Error: could not plan migration ``` to ``` Error: could not plan migration: could not migrate table for entity "E": could not add column for field "title": the field does not have a default value ``` --- core/error.rs | 2 +- core/ops.rs | 2 +- core/runtime.rs | 67 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/core/error.rs b/core/error.rs index 5ee3355c44..9ad99c6da2 100644 --- a/core/error.rs +++ b/core/error.rs @@ -105,7 +105,7 @@ pub fn to_v8_error<'a>( let cb = cb.open(scope); let this = v8::undefined(scope).into(); let class = v8::String::new(scope, get_class(error)).unwrap(); - let message = v8::String::new(scope, &error.to_string()).unwrap(); + let message = v8::String::new(scope, &format!("{:#}", error)).unwrap(); let mut args = vec![class.into(), message.into()]; if let Some(code) = crate::error_codes::get_error_code(error) { args.push(v8::String::new(scope, code).unwrap().into()); diff --git a/core/ops.rs b/core/ops.rs index b339b7d013..8694324adf 100644 --- a/core/ops.rs +++ b/core/ops.rs @@ -133,7 +133,7 @@ impl OpError { pub fn new(get_class: GetErrorClassFn, err: Error) -> Self { Self { class_name: (get_class)(&err), - message: err.to_string(), + message: format!("{:#}", err), code: crate::error_codes::get_error_code(&err), } } diff --git a/core/runtime.rs b/core/runtime.rs index 516d519a3f..e574bf9d38 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -3094,6 +3094,73 @@ main(); }) } + #[test] + fn test_error_context() { + use anyhow::anyhow; + + #[op] + fn op_err_sync() -> Result<(), Error> { + Err(anyhow!("original sync error").context("higher-level sync error")) + } + + #[op] + async fn op_err_async() -> Result<(), Error> { + Err(anyhow!("original async error").context("higher-level async error")) + } + + run_in_task(|cx| { + let ext = Extension::builder() + .ops(vec![op_err_sync::decl(), op_err_async::decl()]) + .build(); + let mut runtime = JsRuntime::new(RuntimeOptions { + extensions: vec![ext], + ..Default::default() + }); + + runtime + .execute_script( + "test_error_context_sync.js", + r#" +let errMessage; +try { + Deno.core.ops.op_err_sync(); +} catch (err) { + errMessage = err.message; +} +if (errMessage !== "higher-level sync error: original sync error") { + throw new Error("unexpected error message from op_err_sync: " + errMessage); +} +"#, + ) + .unwrap(); + + let promise = runtime + .execute_script( + "test_error_context_async.js", + r#" +(async () => { + let errMessage; + try { + await Deno.core.opAsync("op_err_async"); + } catch (err) { + errMessage = err.message; + } + if (errMessage !== "higher-level async error: original async error") { + throw new Error("unexpected error message from op_err_async: " + errMessage); + } +})() +"#, + ) + .unwrap(); + + match runtime.poll_value(&promise, cx) { + Poll::Ready(Ok(_)) => {} + Poll::Ready(Err(err)) => panic!("{:?}", err), + _ => panic!(), + } + }) + } + #[test] fn test_pump_message_loop() { run_in_task(|cx| {