Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Output cause on JS runtime errors #12308

Closed
petamoriken opened this issue Oct 3, 2021 · 1 comment · Fixed by #13209
Closed

Output cause on JS runtime errors #12308

petamoriken opened this issue Oct 3, 2021 · 1 comment · Fixed by #13209
Labels
deno_core Changes in "deno_core" crate are needed feat new feature (which has been agreed to/accepted) help wanted community help requested runtime Relates to code in the runtime crate

Comments

@petamoriken
Copy link
Contributor

petamoriken commented Oct 3, 2021

Since Deno 1.13.0, Error Cause Proposal is supported, so the Error object can hold inner errors. However, their contents are usually not displayed and I need to explicitly access the cause property.

function bar() {
  throw new Error("bar");
}

function foo() {
  try {
    bar();
  } catch (e) {
    throw new Error("foo", { cause: e });
  }
}

foo();

terminal

I think it should also show inner errors, just like Firefox does.

Firefox console

@kitsonk kitsonk added runtime Relates to code in the runtime crate suggestion suggestions for new features (yet to be agreed) labels Oct 3, 2021
@kitsonk kitsonk changed the title Display internal errors Output cause on JS runtime errors Oct 3, 2021
@bartlomieju
Copy link
Member

bartlomieju commented Dec 3, 2021

Showing Error.cause when inspecting in console landed in v1.16 (https://deno.com/blog/v1.16#errorcause-is-now-displayed-in-the-console), we should add the same feature for errors that were returned by the runtime. It will require to modify deno_core and cli to also account for cause property in errors.

Most likely these functions need to be updated:

deno/core/error.rs

Lines 173 to 200 in 1947f89

pub fn from_v8_exception(
scope: &mut v8::HandleScope,
exception: v8::Local<v8::Value>,
) -> Self {
// Create a new HandleScope because we're creating a lot of new local
// handles below.
let scope = &mut v8::HandleScope::new(scope);
let msg = v8::Exception::create_message(scope, exception);
let (message, frames, stack) = if is_instance_of_error(scope, exception) {
// The exception is a JS Error object.
let exception: v8::Local<v8::Object> = exception.try_into().unwrap();
let e: NativeJsError =
serde_v8::from_v8(scope, exception.into()).unwrap();
// Get the message by formatting error.name and error.message.
let name = e.name.unwrap_or_else(|| "Error".to_string());
let message_prop = e.message.unwrap_or_else(|| "".to_string());
let 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()
};

deno/cli/fmt_errors.rs

Lines 245 to 280 in 1947f89

impl fmt::Display for PrettyJsError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let mut frames = self.0.frames.clone();
// When the stack frame array is empty, but the source location given by
// (script_resource_name, line_number, start_column + 1) exists, this is
// likely a syntax error. For the sake of formatting we treat it like it was
// given as a single stack frame.
if frames.is_empty()
&& self.0.script_resource_name.is_some()
&& self.0.line_number.is_some()
&& self.0.start_column.is_some()
{
frames = vec![JsStackFrame::from_location(
self.0.script_resource_name.clone(),
self.0.line_number,
self.0.start_column.map(|n| n + 1),
)];
}
write!(
f,
"{}",
&format_stack(
true,
&self.0.message,
self.0.source_line.as_deref(),
self.0.start_column,
self.0.end_column,
&frames,
0
)
)?;
Ok(())
}
}

@bartlomieju bartlomieju added deno_core Changes in "deno_core" crate are needed debug feat new feature (which has been agreed to/accepted) help wanted community help requested and removed suggestion suggestions for new features (yet to be agreed) labels Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deno_core Changes in "deno_core" crate are needed feat new feature (which has been agreed to/accepted) help wanted community help requested runtime Relates to code in the runtime crate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants