Skip to content

Commit

Permalink
refactor(core/error): Clarify JsError message fields (denoland#14269)
Browse files Browse the repository at this point in the history
  • Loading branch information
nayeemrmn committed Apr 13, 2022
1 parent c03fbb3 commit 7d50a5f
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 17 deletions.
2 changes: 1 addition & 1 deletion cli/fmt_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ impl fmt::Display for PrettyJsError {
"{}",
&format_stack(
true,
&self.0.message,
&self.0.exception_message,
cause.as_deref(),
self.0.source_line.as_deref(),
self.0.start_column,
Expand Down
6 changes: 5 additions & 1 deletion cli/source_maps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ pub fn apply_source_map<G: SourceMapGetter>(
.map(|cause| Box::new(apply_source_map(&*cause, getter)));

JsError {
name: js_error.name.clone(),
message: js_error.message.clone(),
exception_message: js_error.exception_message.clone(),
cause,
source_line,
script_resource_name,
Expand Down Expand Up @@ -243,7 +245,9 @@ mod tests {
#[test]
fn apply_source_map_line() {
let e = JsError {
message: "TypeError: baz".to_string(),
name: Some("TypeError".to_string()),
message: Some("baz".to_string()),
exception_message: "TypeError: baz".to_string(),
cause: None,
source_line: Some("foo".to_string()),
script_resource_name: Some("foo_bar.ts".to_string()),
Expand Down
20 changes: 13 additions & 7 deletions core/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ pub fn get_custom_error_class(error: &Error) -> Option<&'static str> {
#[derive(Debug, PartialEq, Clone, serde::Deserialize, serde::Serialize)]
#[serde(rename_all = "camelCase")]
pub struct JsError {
pub message: String,
pub name: Option<String>,
pub message: Option<String>,
pub exception_message: String,
pub cause: Option<Box<JsError>>,
pub source_line: Option<String>,
pub script_resource_name: Option<String>,
Expand Down Expand Up @@ -198,9 +200,9 @@ impl JsError {
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() {
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)
Expand Down Expand Up @@ -239,7 +241,9 @@ impl JsError {
None => vec![],
};
Self {
message,
name: e.name,
message: e.message,
exception_message,
cause,
script_resource_name: msg
.get_script_resource_name(scope)
Expand All @@ -259,7 +263,9 @@ impl JsError {
// Get the message given by V8::Exception::create_message(), and provide
// empty frames.
Self {
message: msg.get(scope).to_rust_string_lossy(scope),
name: None,
message: None,
exception_message: msg.get(scope).to_rust_string_lossy(scope),
cause: None,
script_resource_name: None,
source_line: None,
Expand Down Expand Up @@ -294,7 +300,7 @@ impl Display for JsError {
}
}

write!(f, "{}", self.message)?;
write!(f, "{}", self.exception_message)?;
if let Some(script_resource_name) = &self.script_resource_name {
if self.line_number.is_some() && self.start_column.is_some() {
let source_loc = format_source_loc(
Expand Down
12 changes: 6 additions & 6 deletions core/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1053,9 +1053,9 @@ pub(crate) fn exception_to_err_result<'s, T>(

let mut js_error = JsError::from_v8_exception(scope, exception);
if in_promise {
js_error.message = format!(
js_error.exception_message = format!(
"Uncaught (in promise) {}",
js_error.message.trim_start_matches("Uncaught ")
js_error.exception_message.trim_start_matches("Uncaught ")
);
}
let js_error = (state.js_error_create_fn)(js_error);
Expand Down Expand Up @@ -2044,7 +2044,7 @@ pub mod tests {
.unwrap();
let v = runtime.poll_value(&value_global, cx);
assert!(
matches!(v, Poll::Ready(Err(e)) if e.downcast_ref::<JsError>().unwrap().message == "Uncaught Error: fail")
matches!(v, Poll::Ready(Err(e)) if e.downcast_ref::<JsError>().unwrap().exception_message == "Uncaught Error: fail")
);

let value_global = runtime
Expand Down Expand Up @@ -2087,7 +2087,7 @@ pub mod tests {
let err = runtime.resolve_value(value_global).await.unwrap_err();
assert_eq!(
"Uncaught Error: fail",
err.downcast::<JsError>().unwrap().message
err.downcast::<JsError>().unwrap().exception_message
);

let value_global = runtime
Expand Down Expand Up @@ -2377,7 +2377,7 @@ pub mod tests {
.expect_err("script should fail");
assert_eq!(
"Uncaught Error: execution terminated",
err.downcast::<JsError>().unwrap().message
err.downcast::<JsError>().unwrap().exception_message
);
assert!(callback_invoke_count.load(Ordering::SeqCst) > 0)
}
Expand Down Expand Up @@ -2430,7 +2430,7 @@ pub mod tests {
.expect_err("script should fail");
assert_eq!(
"Uncaught Error: execution terminated",
err.downcast::<JsError>().unwrap().message
err.downcast::<JsError>().unwrap().exception_message
);
assert_eq!(0, callback_invoke_count_first.load(Ordering::SeqCst));
assert!(callback_invoke_count_second.load(Ordering::SeqCst) > 0);
Expand Down
2 changes: 1 addition & 1 deletion ext/web/02_event.js
Original file line number Diff line number Diff line change
Expand Up @@ -1329,7 +1329,7 @@
function reportException(error) {
reportExceptionStackedCalls++;
const jsError = core.destructureError(error);
const message = jsError.message;
const message = jsError.exceptionMessage;
let filename = "";
let lineno = 0;
let colno = 0;
Expand Down
2 changes: 1 addition & 1 deletion runtime/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ impl Serialize for WorkerControlEvent {
| WorkerControlEvent::Error(error) => {
let value = match error.downcast_ref::<JsError>() {
Some(js_error) => json!({
"message": js_error.message,
"message": js_error.exception_message,
"fileName": js_error.script_resource_name,
"lineNumber": js_error.line_number,
"columnNumber": js_error.start_column,
Expand Down

0 comments on commit 7d50a5f

Please sign in to comment.