Skip to content

Commit

Permalink
WIP: get better frame info from prepareStackTrace
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinkassimo committed Mar 3, 2020
1 parent eafd40f commit 93aed08
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 21 deletions.
17 changes: 16 additions & 1 deletion cli/fmt_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,21 @@ fn format_stack_frame(frame: &JSStackFrame, is_internal_frame: bool) -> String {
source_loc = colors::gray(source_loc).to_string();
}
if !frame.function_name.is_empty() {
format!("{} {} {}", at_prefix, function_name, source_loc)
if frame.is_async {
format!(
"{} {} {} {}",
at_prefix,
colors::gray("async".to_owned()).to_string(),
function_name,
source_loc
)
} else {
format!("{} {} {}", at_prefix, function_name, source_loc)
}
} else if frame.is_eval {
format!("{} eval {}", at_prefix, source_loc)
} else if frame.is_async {
format!("{} async {}", at_prefix, source_loc)
} else {
format!("{} {}", at_prefix, source_loc)
}
Expand Down Expand Up @@ -270,6 +282,7 @@ mod tests {
function_name: "foo".to_string(),
is_eval: false,
is_constructor: false,
is_async: false,
},
JSStackFrame {
line_number: 5,
Expand All @@ -278,6 +291,7 @@ mod tests {
function_name: "qat".to_string(),
is_eval: false,
is_constructor: false,
is_async: false,
},
JSStackFrame {
line_number: 1,
Expand All @@ -286,6 +300,7 @@ mod tests {
function_name: "".to_string(),
is_eval: false,
is_constructor: false,
is_async: false,
},
],
};
Expand Down
14 changes: 10 additions & 4 deletions cli/js/error_stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,9 @@ function prepareStackTrace(
error: Error,
structuredStackTrace: CallSite[]
): string {
return (
// @ts-ignore
error["__callSites"] = [];
const errorString =
`${error.name}: ${error.message}\n` +
structuredStackTrace
.map(
Expand All @@ -256,9 +258,13 @@ function prepareStackTrace(
return callSite;
}
)
.map((callSite): string => ` at ${callSiteToString(callSite)}`)
.join("\n")
);
.map((callSite): string => {
// @ts-ignore
error["__callSites"].push(callSite); // Save callsites to self
return ` at ${callSiteToString(callSite)}`;
})
.join("\n");
return errorString;
}

/** Sets the `prepareStackTrace` method on the Error constructor which will
Expand Down
7 changes: 7 additions & 0 deletions cli/source_maps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ fn frame_apply_source_map<G: SourceMapGetter>(
column,
is_eval: frame.is_eval,
is_constructor: frame.is_constructor,
is_async: frame.is_async,
}
}

Expand Down Expand Up @@ -308,6 +309,7 @@ mod tests {
function_name: "foo".to_string(),
is_eval: false,
is_constructor: false,
is_async: false,
},
JSStackFrame {
line_number: 5,
Expand All @@ -316,6 +318,7 @@ mod tests {
function_name: "qat".to_string(),
is_eval: false,
is_constructor: false,
is_async: false,
},
JSStackFrame {
line_number: 1,
Expand All @@ -324,6 +327,7 @@ mod tests {
function_name: "".to_string(),
is_eval: false,
is_constructor: false,
is_async: false,
},
],
};
Expand All @@ -344,6 +348,7 @@ mod tests {
function_name: "foo".to_string(),
is_eval: false,
is_constructor: false,
is_async: false,
},
JSStackFrame {
line_number: 4,
Expand All @@ -352,6 +357,7 @@ mod tests {
function_name: "qat".to_string(),
is_eval: false,
is_constructor: false,
is_async: false,
},
JSStackFrame {
line_number: 1,
Expand All @@ -360,6 +366,7 @@ mod tests {
function_name: "".to_string(),
is_eval: false,
is_constructor: false,
is_async: false,
},
],
};
Expand Down
138 changes: 122 additions & 16 deletions core/js_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::fmt;

/// A `JSError` represents an exception coming from V8, with stack frames and
/// line numbers. The deno_cli crate defines another `JSError` type, which wraps
/// the one defined here, that adds source map support and colorful formatting.
/// the one defined here, that adds source map support and colorful formatting.
#[derive(Debug, PartialEq, Clone)]
pub struct JSError {
pub message: String,
Expand All @@ -38,6 +38,23 @@ pub struct JSStackFrame {
pub function_name: String,
pub is_eval: bool,
pub is_constructor: bool,
pub is_async: bool,
}

fn call_js_method<'s>(
scope: &mut impl v8::ToLocal<'s>,
context: v8::Local<v8::Context>,
o: v8::Local<v8::Object>,
name: &str,
) -> Option<v8::Local<'s, v8::Value>> {
let global: v8::Local<v8::Value> = context.global(scope).into();
let name_str = v8::String::new(scope, name).unwrap();
let func: v8::Local<v8::Function> = o
.get(scope, context, name_str.into())
.unwrap()
.try_into()
.unwrap();
return func.call(scope, context, global, &[o.clone().into()]);
}

impl JSError {
Expand All @@ -53,23 +70,22 @@ impl JSError {
// handles below.
let mut hs = v8::HandleScope::new(scope);
let scope = hs.enter();
let context = scope.get_current_context().unwrap();
let context = { scope.get_current_context().unwrap() };

let exception2: Result<v8::Local<v8::Object>, _> =
exception.clone().try_into();
let exception2 = exception2.unwrap();
let stack_str = v8::String::new(scope, "stack").unwrap();
let _ = exception2.get(scope, context, stack_str.into());

let error_call_sites_str = v8::String::new(scope, "__callSites").unwrap();
let error_call_sites =
exception2.get(scope, context, error_call_sites_str.into());

let msg = v8::Exception::create_message(scope, exception);

Self {
message: msg.get(scope).to_rust_string_lossy(scope),
script_resource_name: msg
.get_script_resource_name(scope)
.and_then(|v| v8::Local::<v8::String>::try_from(v).ok())
.map(|v| v.to_rust_string_lossy(scope)),
source_line: msg
.get_source_line(scope, context)
.map(|v| v.to_rust_string_lossy(scope)),
line_number: msg.get_line_number(context).and_then(|v| v.try_into().ok()),
start_column: msg.get_start_column().try_into().ok(),
end_column: msg.get_end_column().try_into().ok(),
frames: msg
let frames = if error_call_sites.is_none() {
msg
.get_stack_trace(scope)
.map(|stack_trace| {
(0..stack_trace.get_frame_count())
Expand All @@ -96,11 +112,96 @@ impl JSError {
.unwrap_or_else(|| "".to_owned()),
is_constructor: frame.is_constructor(),
is_eval: frame.is_eval(),
is_async: false,
}
})
.collect::<Vec<_>>()
})
.unwrap_or_else(Vec::<_>::new),
.unwrap_or_else(Vec::<_>::new)
} else {
let cs_arr: v8::Local<v8::Array> =
error_call_sites.unwrap().try_into().unwrap();
let mut output: Vec<JSStackFrame> = vec![];
for i in 0..cs_arr.length() {
let nth: v8::Local<v8::Object> = cs_arr
.get_index(scope, context, i)
.unwrap()
.try_into()
.unwrap();
let line_number_v8: v8::Local<v8::Integer> =
call_js_method(scope, context, nth, "getLineNumber")
.unwrap()
.try_into()
.unwrap();
let line_number = line_number_v8.value();
let column_v8: v8::Local<v8::Integer> =
call_js_method(scope, context, nth, "getColumnNumber")
.unwrap()
.try_into()
.unwrap();
let column = column_v8.value();
let script_name_v8: Result<v8::Local<v8::String>, _> =
call_js_method(scope, context, nth, "getFileName")
.unwrap()
.try_into();
let script_name = if let Ok(sname) = script_name_v8 {
sname.to_rust_string_lossy(scope)
} else {
String::new()
};
let function_name_v8: Result<v8::Local<v8::String>, _> =
call_js_method(scope, context, nth, "getFunctionName")
.unwrap()
.try_into();
let function_name = if let Ok(fname) = function_name_v8 {
fname.to_rust_string_lossy(scope)
} else {
String::new()
};
let is_constructor_v8: v8::Local<v8::Boolean> =
call_js_method(scope, context, nth, "isConstructor")
.unwrap()
.try_into()
.unwrap();
let is_constructor = is_constructor_v8.is_true();
let is_eval_v8: v8::Local<v8::Boolean> =
call_js_method(scope, context, nth, "isEval")
.unwrap()
.try_into()
.unwrap();
let is_eval = is_eval_v8.is_true();
let is_async_v8: v8::Local<v8::Boolean> =
call_js_method(scope, context, nth, "isAsync")
.unwrap()
.try_into()
.unwrap();
let is_async = is_async_v8.is_true();
output.push(JSStackFrame {
line_number,
column,
script_name,
function_name,
is_constructor,
is_eval,
is_async,
});
}
output
};

Self {
message: msg.get(scope).to_rust_string_lossy(scope),
script_resource_name: msg
.get_script_resource_name(scope)
.and_then(|v| v8::Local::<v8::String>::try_from(v).ok())
.map(|v| v.to_rust_string_lossy(scope)),
source_line: msg
.get_source_line(scope, context)
.map(|v| v.to_rust_string_lossy(scope)),
line_number: msg.get_line_number(context).and_then(|v| v.try_into().ok()),
start_column: msg.get_start_column().try_into().ok(),
end_column: msg.get_end_column().try_into().ok(),
frames,
}
}
}
Expand All @@ -127,6 +228,8 @@ fn format_stack_frame(frame: &JSStackFrame) -> String {
format!(" at {} ({})", frame.function_name, source_loc)
} else if frame.is_eval {
format!(" at eval ({})", source_loc)
} else if frame.is_async {
format!(" at async ({})", source_loc)
} else {
format!(" at {}", source_loc)
}
Expand Down Expand Up @@ -190,6 +293,7 @@ mod tests {
function_name: "foo".to_string(),
is_eval: false,
is_constructor: false,
is_async: false,
},
JSStackFrame {
line_number: 5,
Expand All @@ -198,6 +302,7 @@ mod tests {
function_name: "qat".to_string(),
is_eval: false,
is_constructor: false,
is_async: false,
},
JSStackFrame {
line_number: 1,
Expand All @@ -206,6 +311,7 @@ mod tests {
function_name: "".to_string(),
is_eval: false,
is_constructor: false,
is_async: false,
},
],
};
Expand Down

0 comments on commit 93aed08

Please sign in to comment.