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

feat: output cause on JS runtime errors #13209

Merged
merged 9 commits into from
Dec 29, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions cli/fmt_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,11 @@ fn format_frame(frame: &JsStackFrame) -> String {
result
}

#[allow(clippy::too_many_arguments)]
crowlKats marked this conversation as resolved.
Show resolved Hide resolved
fn format_stack(
is_error: bool,
message_line: &str,
cause: Option<&str>,
source_line: Option<&str>,
start_column: Option<i64>,
end_column: Option<i64>,
Expand All @@ -154,6 +156,14 @@ fn format_stack(
indent = level
));
}
if let Some(cause) = cause {
s.push_str(&format!(
"\n{:indent$}Caused by: {}",
"",
cause,
indent = level
));
}
s
}

Expand Down Expand Up @@ -262,12 +272,19 @@ impl fmt::Display for PrettyJsError {
)];
}

let cause = self
.0
.cause
.as_ref()
.map(|cause| format!("{}", PrettyJsError(*cause.clone())));

write!(
f,
"{}",
&format_stack(
true,
&self.0.message,
cause.as_deref(),
self.0.source_line.as_deref(),
self.0.start_column,
self.0.end_column,
Expand Down
2 changes: 2 additions & 0 deletions cli/source_maps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ pub fn apply_source_map<G: SourceMapGetter>(

JsError {
message: js_error.message.clone(),
cause: js_error.cause.clone(),
source_line,
script_resource_name,
line_number,
Expand Down Expand Up @@ -238,6 +239,7 @@ mod tests {
fn apply_source_map_line() {
let e = JsError {
message: "TypeError: baz".to_string(),
cause: None,
source_line: Some("foo".to_string()),
script_resource_name: Some("foo_bar.ts".to_string()),
line_number: Some(4),
Expand Down
6 changes: 6 additions & 0 deletions cli/tests/integration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,12 @@ fn broken_stdout() {
assert!(!stderr.contains("panic"));
}

itest!(error_cause {
args: "run error_cause.ts",
output: "error_cause.ts.out",
exit_code: 1,
});

itest_flaky!(cafile_url_imports {
args: "run --quiet --reload --cert tls/RootCA.pem cafile_url_imports.ts",
output: "cafile_url_imports.ts.out",
Expand Down
1 change: 1 addition & 0 deletions cli/tests/testdata/error_cause.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
throw new Error("foo", { cause: new Error("bar", { cause: "deno" }) });
11 changes: 11 additions & 0 deletions cli/tests/testdata/error_cause.ts.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[WILDCARD]
error: Uncaught Error: foo
throw new Error("foo", { cause: new Error("bar", { cause: "deno" }) });
^
at file:///[WILDCARD]/error_cause.ts:1:7
Caused by: Uncaught Error: bar
throw new Error("foo", { cause: new Error("bar", { cause: "deno" }) });
^
at file:///[WILDCARD]/error_cause.ts:1:33
Caused by: Uncaught deno
[WILDCARD]
101 changes: 59 additions & 42 deletions core/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ pub fn get_custom_error_class(error: &Error) -> Option<&'static str> {
#[derive(Debug, PartialEq, Clone)]
pub struct JsError {
pub message: String,
pub cause: Option<Box<JsError>>,
pub source_line: Option<String>,
pub script_resource_name: Option<String>,
pub line_number: Option<i64>,
Expand Down Expand Up @@ -180,53 +181,69 @@ impl JsError {

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)
let (message, frames, stack, cause) =
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 cause = get_property(scope, exception, "cause");
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()
};
let cause = cause.and_then(|cause| {
if cause.is_undefined() {
None
} else {
Some(Box::new(JsError::from_v8_exception(scope, cause)))
crowlKats marked this conversation as resolved.
Show resolved Hide resolved
}
});

// Access error.stack to ensure that prepareStackTrace() has been called.
// This should populate error.__callSiteEvals.
let stack = get_property(scope, exception, "stack");
let stack: Option<v8::Local<v8::String>> =
stack.and_then(|s| s.try_into().ok());
let stack = stack.map(|s| s.to_rust_string_lossy(scope));

// Read an array of structured frames from error.__callSiteEvals.
let frames_v8 = get_property(scope, exception, "__callSiteEvals");
// Ignore non-array values
let frames_v8: Option<v8::Local<v8::Array>> =
frames_v8.and_then(|a| a.try_into().ok());

// Convert them into Vec<JsStackFrame>
let frames: Vec<JsStackFrame> = match frames_v8 {
Some(frames_v8) => {
serde_v8::from_v8(scope, frames_v8.into()).unwrap()
}
None => vec![],
};
(message, frames, stack, cause)
} else {
"Uncaught".to_string()
// The exception is not a JS Error object.
// Get the message given by V8::Exception::create_message(), and provide
// empty frames.
(
msg.get(scope).to_rust_string_lossy(scope),
vec![],
None,
None,
)
};

// Access error.stack to ensure that prepareStackTrace() has been called.
// This should populate error.__callSiteEvals.
let stack = get_property(scope, exception, "stack");
let stack: Option<v8::Local<v8::String>> =
stack.and_then(|s| s.try_into().ok());
let stack = stack.map(|s| s.to_rust_string_lossy(scope));

// Read an array of structured frames from error.__callSiteEvals.
let frames_v8 = get_property(scope, exception, "__callSiteEvals");
// Ignore non-array values
let frames_v8: Option<v8::Local<v8::Array>> =
frames_v8.and_then(|a| a.try_into().ok());

// Convert them into Vec<JsStackFrame>
let frames: Vec<JsStackFrame> = match frames_v8 {
Some(frames_v8) => serde_v8::from_v8(scope, frames_v8.into()).unwrap(),
None => vec![],
};
(message, frames, stack)
} else {
// The exception is not a JS Error object.
// Get the message given by V8::Exception::create_message(), and provide
// empty frames.
(msg.get(scope).to_rust_string_lossy(scope), vec![], None)
};

Self {
message,
cause,
script_resource_name: msg
.get_script_resource_name(scope)
.and_then(|v| v8::Local::<v8::String>::try_from(v).ok())
Expand Down