foster/denoland-deno
1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-12-22 15:24:46 -05:00
Code Issues Packages 1 Wiki Activity

refactor(core/js_error): Align JSStackFrame with CallSite (#4715)

Browse source 
Renames and adds missing fields to JSStackFrame from CallSite. Fixes #4705.

Cleans up base changes for line and column numbers.
This commit is contained in:
Nayeem Rahman 2020-04-13 15:54:16 +01:00 committed by GitHub
parent 5105c68399
commit 0ea6eb83a9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 271 additions and 192 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
cli/diagnostics.rs
View file

@ -232,9 +232,10 @@ impl DisplayFormatter for DiagnosticItem {
} }
fn format_source_line(&self, level: usize) -> String { fn format_source_line(&self, level: usize) -> String {
// Formatter expects 1-based line numbers, but ours are 0-based.
format_maybe_source_line( format_maybe_source_line(
self.source_line.clone(), self.source_line.clone(),
self.line_number, self.line_number.map(|n| n + 1),
self.start_column, self.start_column,
self.end_column, self.end_column,
match self.category { match self.category {
@ -246,10 +247,11 @@ impl DisplayFormatter for DiagnosticItem {
} }
fn format_source_name(&self) -> String { fn format_source_name(&self) -> String {
// Formatter expects 1-based line and column numbers, but ours are 0-based.
format_maybe_source_name( format_maybe_source_name(
self.script_resource_name.clone(), self.script_resource_name.clone(),
self.line_number, self.line_number.map(|n| n + 1),
self.start_column, self.start_column.map(|n| n + 1),
) )
} }
} }

47
cli/fmt_errors.rs
View file

@ -21,34 +21,34 @@ pub trait DisplayFormatter {
} }
fn format_source_name( fn format_source_name(
script_name: String, file_name: String,
line_number: i64, line_number: i64,
column: i64, column_number: i64,
) -> String { ) -> String {
let line_number = line_number + 1; let line_number = line_number;
let column = column + 1; let column_number = column_number;
let script_name_c = colors::cyan(script_name); let file_name_c = colors::cyan(file_name);
let line_c = colors::yellow(line_number.to_string()); let line_c = colors::yellow(line_number.to_string());
let column_c = colors::yellow(column.to_string()); let column_c = colors::yellow(column_number.to_string());
format!("{}:{}:{}", script_name_c, line_c, column_c) format!("{}:{}:{}", file_name_c, line_c, column_c)
} }
/// Formats optional source, line number and column into a single string. /// Formats optional source, line and column numbers into a single string.
pub fn format_maybe_source_name( pub fn format_maybe_source_name(
script_name: Option<String>, file_name: Option<String>,
line_number: Option<i64>, line_number: Option<i64>,
column: Option<i64>, column_number: Option<i64>,
) -> String { ) -> String {
if script_name.is_none() { if file_name.is_none() {
return "".to_string(); return "".to_string();
} }
assert!(line_number.is_some()); assert!(line_number.is_some());
assert!(column.is_some()); assert!(column_number.is_some());
format_source_name( format_source_name(
script_name.unwrap(), file_name.unwrap(),
line_number.unwrap(), line_number.unwrap(),
column.unwrap(), column_number.unwrap(),
) )
} }
@ -76,7 +76,7 @@ pub fn format_maybe_source_line(
assert!(start_column.is_some()); assert!(start_column.is_some());
assert!(end_column.is_some()); assert!(end_column.is_some());
let line_number = (1 + line_number.unwrap()).to_string(); let line_number = line_number.unwrap().to_string();
let line_color = colors::black_on_white(line_number.to_string()); let line_color = colors::black_on_white(line_number.to_string());
let line_number_len = line_number.len(); let line_number_len = line_number.len();
let line_padding = let line_padding =
@ -93,12 +93,11 @@ pub fn format_maybe_source_line(
} else { } else {
'~' '~'
}; };
for i in 0..end_column { for _i in 0..start_column {
if i >= start_column { s.push(' ');
s.push(underline_char); }
} else { for _i in 0..(end_column - start_column) {
s.push(' '); s.push(underline_char);
}
} }
let color_underline = if is_error { let color_underline = if is_error {
colors::red(s).to_string() colors::red(s).to_string()
@ -181,7 +180,7 @@ impl DisplayFormatter for JSError {
format_maybe_source_name( format_maybe_source_name(
e.script_resource_name.clone(), e.script_resource_name.clone(),
e.line_number, e.line_number,
e.start_column, e.start_column.map(|n| n + 1),
) )
) )
} }
@ -223,7 +222,7 @@ mod tests {
Some(1), Some(1),
Some(2), Some(2),
); );
assert_eq!(strip_ansi_codes(&actual), "file://foo/bar.ts:2:3"); assert_eq!(strip_ansi_codes(&actual), "file://foo/bar.ts:1:2");
} }
#[test] #[test]
@ -244,7 +243,7 @@ mod tests {
); );
assert_eq!( assert_eq!(
strip_ansi_codes(&actual), strip_ansi_codes(&actual),
"\n\n9 console.log(\'foo\');\n ~~~\n" "\n\n8 console.log(\'foo\');\n ~~~\n"
); );
} }

55
cli/js/error_stack.ts
View file

@ -11,31 +11,31 @@ function patchCallSite(callSite: CallSite, location: Location): CallSite {
getThis(): unknown { getThis(): unknown {
return callSite.getThis(); return callSite.getThis();
}, },
getTypeName(): string { getTypeName(): string | null {
return callSite.getTypeName(); return callSite.getTypeName();
}, },
getFunction(): Function { getFunction(): Function | null {
return callSite.getFunction(); return callSite.getFunction();
}, },
getFunctionName(): string { getFunctionName(): string | null {
return callSite.getFunctionName(); return callSite.getFunctionName();
}, },
getMethodName(): string { getMethodName(): string | null {
return callSite.getMethodName(); return callSite.getMethodName();
}, },
getFileName(): string { getFileName(): string | null {
return location.filename; return location.fileName;
}, },
getLineNumber(): number { getLineNumber(): number {
return location.line; return location.lineNumber;
}, },
getColumnNumber(): number { getColumnNumber(): number {
return location.column; return location.columnNumber;
}, },
getEvalOrigin(): string | null { getEvalOrigin(): string | null {
return callSite.getEvalOrigin(); return callSite.getEvalOrigin();
}, },
isToplevel(): boolean { isToplevel(): boolean | null {
return callSite.isToplevel(); return callSite.isToplevel();
}, },
isEval(): boolean { isEval(): boolean {
@ -176,15 +176,15 @@ function callSiteToString(callSite: CallSite, isInternal = false): string {
interface CallSiteEval { interface CallSiteEval {
this: unknown; this: unknown;
typeName: string; typeName: string | null;
function: Function; function: Function | null;
functionName: string; functionName: string | null;
methodName: string; methodName: string | null;
fileName: string; fileName: string | null;
lineNumber: number | null; lineNumber: number | null;
columnNumber: number | null; columnNumber: number | null;
evalOrigin: string | null; evalOrigin: string | null;
isToplevel: boolean; isToplevel: boolean | null;
isEval: boolean; isEval: boolean;
isNative: boolean; isNative: boolean;
isConstructor: boolean; isConstructor: boolean;
@ -227,16 +227,16 @@ function prepareStackTrace(
structuredStackTrace structuredStackTrace
.map( .map(
(callSite): CallSite => { (callSite): CallSite => {
const filename = callSite.getFileName(); const fileName = callSite.getFileName();
const line = callSite.getLineNumber(); const lineNumber = callSite.getLineNumber();
const column = callSite.getColumnNumber(); const columnNumber = callSite.getColumnNumber();
if (filename && line != null && column != null) { if (fileName && lineNumber != null && columnNumber != null) {
return patchCallSite( return patchCallSite(
callSite, callSite,
applySourceMap({ applySourceMap({
filename, fileName,
line, lineNumber,
column, columnNumber,
}) })
); );
} }
@ -244,16 +244,13 @@ function prepareStackTrace(
} }
) )
.map((callSite): string => { .map((callSite): string => {
// @ts-ignore
error.__callSiteEvals.push(Object.freeze(evaluateCallSite(callSite)));
const isInternal = const isInternal =
callSite.getFileName()?.startsWith("$deno$") ?? false; callSite.getFileName()?.startsWith("$deno$") ?? false;
const string = callSiteToString(callSite, isInternal); const string = callSiteToString(callSite, isInternal);
const callSiteEv = Object.freeze(evaluateCallSite(callSite)); // @ts-ignore
if (callSiteEv.lineNumber != null && callSiteEv.columnNumber != null) { error.__formattedFrames.push(string);
// @ts-ignore
error.__callSiteEvals.push(callSiteEv);
// @ts-ignore
error.__formattedFrames.push(string);
}
return ` at ${colors.stripColor(string)}`; return ` at ${colors.stripColor(string)}`;
}) })
.join("\n"); .join("\n");

12
cli/js/globals.ts
View file

@ -31,15 +31,15 @@ import { core } from "./core.ts";
declare global { declare global {
interface CallSite { interface CallSite {
getThis(): unknown; getThis(): unknown;
getTypeName(): string; getTypeName(): string | null;
getFunction(): Function; getFunction(): Function | null;
getFunctionName(): string; getFunctionName(): string | null;
getMethodName(): string; getMethodName(): string | null;
getFileName(): string; getFileName(): string | null;
getLineNumber(): number | null; getLineNumber(): number | null;
getColumnNumber(): number | null; getColumnNumber(): number | null;
getEvalOrigin(): string | null; getEvalOrigin(): string | null;
isToplevel(): boolean; isToplevel(): boolean | null;
isEval(): boolean; isEval(): boolean;
isNative(): boolean; isNative(): boolean;
isConstructor(): boolean; isConstructor(): boolean;

12
cli/js/lib.deno.ns.d.ts vendored
View file

@ -1612,11 +1612,11 @@ declare namespace Deno {
interface Location { interface Location {
/** The full url for the module, e.g. `file://some/file.ts` or /** The full url for the module, e.g. `file://some/file.ts` or
* `https://some/file.ts`. */ * `https://some/file.ts`. */
filename: string; fileName: string;
/** The line number in the file. It is assumed to be 1-indexed. */ /** The line number in the file. It is assumed to be 1-indexed. */
line: number; lineNumber: number;
/** The column number in the file. It is assumed to be 1-indexed. */ /** The column number in the file. It is assumed to be 1-indexed. */
column: number; columnNumber: number;
} }
/** UNSTABLE: new API, yet to be vetted. /** UNSTABLE: new API, yet to be vetted.
@ -1636,9 +1636,9 @@ declare namespace Deno {
* An example: * An example:
* *
* const orig = Deno.applySourceMap({ * const orig = Deno.applySourceMap({
* location: "file://my/module.ts", * fileName: "file://my/module.ts",
* line: 5, * lineNumber: 5,
* column: 15 * columnNumber: 15
* }); * });
* console.log(`${orig.filename}:${orig.line}:${orig.column}`); * console.log(`${orig.filename}:${orig.line}:${orig.column}`);
*/ */

24
cli/js/ops/errors.ts
View file

@ -7,25 +7,21 @@ export function formatDiagnostics(items: DiagnosticItem[]): string {
} }
export interface Location { export interface Location {
filename: string; fileName: string;
lineNumber: number;
line: number; columnNumber: number;
column: number;
} }
export function applySourceMap(location: Location): Location { export function applySourceMap(location: Location): Location {
const { filename, line, column } = location; const { fileName, lineNumber, columnNumber } = location;
// On this side, line/column are 1 based, but in the source maps, they are
// 0 based, so we have to convert back and forth
const res = sendSync("op_apply_source_map", { const res = sendSync("op_apply_source_map", {
filename, fileName,
line: line - 1, lineNumber: lineNumber,
column: column - 1, columnNumber: columnNumber,
}); });
return { return {
filename: res.filename, fileName: res.fileName,
line: res.line + 1, lineNumber: res.lineNumber,
column: res.column + 1, columnNumber: res.columnNumber,
}; };
} }

36
cli/js/tests/error_stack_test.ts
View file

@ -6,15 +6,15 @@ const { setPrepareStackTrace } = Deno[Deno.symbols.internal];
interface CallSite { interface CallSite {
getThis(): unknown; getThis(): unknown;
getTypeName(): string; getTypeName(): string | null;
getFunction(): Function; getFunction(): Function | null;
getFunctionName(): string; getFunctionName(): string | null;
getMethodName(): string; getMethodName(): string | null;
getFileName(): string; getFileName(): string | null;
getLineNumber(): number | null; getLineNumber(): number | null;
getColumnNumber(): number | null; getColumnNumber(): number | null;
getEvalOrigin(): string | null; getEvalOrigin(): string | null;
isToplevel(): boolean; isToplevel(): boolean | null;
isEval(): boolean; isEval(): boolean;
isNative(): boolean; isNative(): boolean;
isConstructor(): boolean; isConstructor(): boolean;
@ -24,9 +24,9 @@ interface CallSite {
} }
function getMockCallSite( function getMockCallSite(
filename: string, fileName: string,
line: number | null, lineNumber: number | null,
column: number | null columnNumber: number | null
): CallSite { ): CallSite {
return { return {
getThis(): unknown { getThis(): unknown {
@ -45,13 +45,13 @@ function getMockCallSite(
return ""; return "";
}, },
getFileName(): string { getFileName(): string {
return filename; return fileName;
}, },
getLineNumber(): number | null { getLineNumber(): number | null {
return line; return lineNumber;
}, },
getColumnNumber(): number | null { getColumnNumber(): number | null {
return column; return columnNumber;
}, },
getEvalOrigin(): null { getEvalOrigin(): null {
return null; return null;
@ -98,11 +98,11 @@ unitTest(function prepareStackTrace(): void {
unitTest(function applySourceMap(): void { unitTest(function applySourceMap(): void {
const result = Deno.applySourceMap({ const result = Deno.applySourceMap({
filename: "CLI_SNAPSHOT.js", fileName: "CLI_SNAPSHOT.js",
line: 23, lineNumber: 23,
column: 0, columnNumber: 0,
}); });
assert(result.filename.endsWith(".ts")); assert(result.fileName.endsWith(".ts"));
assert(result.line != null); assert(result.lineNumber != null);
assert(result.column != null); assert(result.columnNumber != null);
}); });

28
cli/ops/errors.rs
View file

@ -20,10 +20,11 @@ pub fn init(i: &mut Isolate, s: &State) {
} }
#[derive(Deserialize)] #[derive(Deserialize)]
#[serde(rename_all = "camelCase")]
struct ApplySourceMap { struct ApplySourceMap {
filename: String, file_name: String,
line: i32, line_number: i32,
column: i32, column_number: i32,
} }
fn op_apply_source_map( fn op_apply_source_map(
@ -34,18 +35,19 @@ fn op_apply_source_map(
let args: ApplySourceMap = serde_json::from_value(args)?; let args: ApplySourceMap = serde_json::from_value(args)?;
let mut mappings_map: CachedMaps = HashMap::new(); let mut mappings_map: CachedMaps = HashMap::new();
let (orig_filename, orig_line, orig_column) = get_orig_position( let (orig_file_name, orig_line_number, orig_column_number) =
args.filename, get_orig_position(
args.line.into(), args.file_name,
args.column.into(), args.line_number.into(),
&mut mappings_map, args.column_number.into(),
&state.borrow().global_state.ts_compiler, &mut mappings_map,
); &state.borrow().global_state.ts_compiler,
);
Ok(JsonOp::Sync(json!({ Ok(JsonOp::Sync(json!({
"filename": orig_filename, "fileName": orig_file_name,
"line": orig_line as u32, "lineNumber": orig_line_number as u32,
"column": orig_column as u32, "columnNumber": orig_column_number as u32,
}))) })))
} }

74
cli/source_maps.rs
View file

@ -6,10 +6,10 @@ use std::str;
pub trait SourceMapGetter { pub trait SourceMapGetter {
/// Returns the raw source map file. /// Returns the raw source map file.
fn get_source_map(&self, script_name: &str) -> Option<Vec<u8>>; fn get_source_map(&self, file_name: &str) -> Option<Vec<u8>>;
fn get_source_line( fn get_source_line(
&self, &self,
script_name: &str, file_name: &str,
line_number: usize, line_number: usize,
) -> Option<String>; ) -> Option<String>;
} }
@ -29,10 +29,10 @@ fn builtin_source_map(_: &str) -> Option<Vec<u8>> {
} }
#[cfg(not(feature = "check-only"))] #[cfg(not(feature = "check-only"))]
fn builtin_source_map(script_name: &str) -> Option<Vec<u8>> { fn builtin_source_map(file_name: &str) -> Option<Vec<u8>> {
if script_name.ends_with("CLI_SNAPSHOT.js") { if file_name.ends_with("CLI_SNAPSHOT.js") {
Some(crate::js::CLI_SNAPSHOT_MAP.to_vec()) Some(crate::js::CLI_SNAPSHOT_MAP.to_vec())
} else if script_name.ends_with("COMPILER_SNAPSHOT.js") { } else if file_name.ends_with("COMPILER_SNAPSHOT.js") {
Some(crate::js::COMPILER_SNAPSHOT_MAP.to_vec()) Some(crate::js::COMPILER_SNAPSHOT_MAP.to_vec())
} else { } else {
None None
@ -54,10 +54,12 @@ pub fn apply_source_map<G: SourceMapGetter>(
get_maybe_orig_position( get_maybe_orig_position(
js_error.script_resource_name.clone(), js_error.script_resource_name.clone(),
js_error.line_number, js_error.line_number,
js_error.start_column, // start_column is 0-based, we need 1-based here.
js_error.start_column.map(|n| n + 1),
&mut mappings_map, &mut mappings_map,
getter, getter,
); );
let start_column = start_column.map(|n| n - 1);
// It is better to just move end_column to be the same distance away from // It is better to just move end_column to be the same distance away from
// start column because sometimes the code point is not available in the // start column because sometimes the code point is not available in the
// source file map. // source file map.
@ -79,7 +81,8 @@ pub fn apply_source_map<G: SourceMapGetter>(
{ {
getter.get_source_line( getter.get_source_line(
&js_error.script_resource_name.clone().unwrap(), &js_error.script_resource_name.clone().unwrap(),
ln as usize, // Getter expects 0-based line numbers, but ours are 1-based.
ln as usize - 1,
) )
} }
_ => js_error.source_line.clone(), _ => js_error.source_line.clone(),
@ -98,48 +101,47 @@ pub fn apply_source_map<G: SourceMapGetter>(
} }
fn get_maybe_orig_position<G: SourceMapGetter>( fn get_maybe_orig_position<G: SourceMapGetter>(
script_name: Option<String>, file_name: Option<String>,
line_number: Option<i64>, line_number: Option<i64>,
column: Option<i64>, column_number: Option<i64>,
mappings_map: &mut CachedMaps, mappings_map: &mut CachedMaps,
getter: &G, getter: &G,
) -> (Option<String>, Option<i64>, Option<i64>) { ) -> (Option<String>, Option<i64>, Option<i64>) {
match (script_name, line_number, column) { match (file_name, line_number, column_number) {
(Some(script_name_v), Some(line_v), Some(column_v)) => { (Some(file_name_v), Some(line_v), Some(column_v)) => {
let (script_name, line_number, column) = get_orig_position( let (file_name, line_number, column_number) =
script_name_v, get_orig_position(file_name_v, line_v, column_v, mappings_map, getter);
line_v - 1, (Some(file_name), Some(line_number), Some(column_number))
column_v,
mappings_map,
getter,
);
(Some(script_name), Some(line_number), Some(column))
} }
_ => (None, None, None), _ => (None, None, None),
} }
} }
pub fn get_orig_position<G: SourceMapGetter>( pub fn get_orig_position<G: SourceMapGetter>(
script_name: String, file_name: String,
line_number: i64, line_number: i64,
column: i64, column_number: i64,
mappings_map: &mut CachedMaps, mappings_map: &mut CachedMaps,
getter: &G, getter: &G,
) -> (String, i64, i64) { ) -> (String, i64, i64) {
let maybe_source_map = get_mappings(&script_name, mappings_map, getter); let maybe_source_map = get_mappings(&file_name, mappings_map, getter);
let default_pos = (script_name, line_number, column); let default_pos = (file_name, line_number, column_number);
// Lookup expects 0-based line and column numbers, but ours are 1-based.
let line_number = line_number - 1;
let column_number = column_number - 1;
match maybe_source_map { match maybe_source_map {
None => default_pos, None => default_pos,
Some(source_map) => { Some(source_map) => {
match source_map.lookup_token(line_number as u32, column as u32) { match source_map.lookup_token(line_number as u32, column_number as u32) {
None => default_pos, None => default_pos,
Some(token) => match token.get_source() { Some(token) => match token.get_source() {
None => default_pos, None => default_pos,
Some(original) => ( Some(original) => (
original.to_string(), original.to_string(),
i64::from(token.get_src_line()), i64::from(token.get_src_line()) + 1,
i64::from(token.get_src_col()), i64::from(token.get_src_col()) + 1,
), ),
}, },
} }
@ -148,23 +150,23 @@ pub fn get_orig_position<G: SourceMapGetter>(
} }
fn get_mappings<'a, G: SourceMapGetter>( fn get_mappings<'a, G: SourceMapGetter>(
script_name: &str, file_name: &str,
mappings_map: &'a mut CachedMaps, mappings_map: &'a mut CachedMaps,
getter: &G, getter: &G,
) -> &'a Option<SourceMap> { ) -> &'a Option<SourceMap> {
mappings_map mappings_map
.entry(script_name.to_string()) .entry(file_name.to_string())
.or_insert_with(|| parse_map_string(script_name, getter)) .or_insert_with(|| parse_map_string(file_name, getter))
} }
// TODO(kitsonk) parsed source maps should probably be cached in state in // TODO(kitsonk) parsed source maps should probably be cached in state in
// the module meta data. // the module meta data.
fn parse_map_string<G: SourceMapGetter>( fn parse_map_string<G: SourceMapGetter>(
script_name: &str, file_name: &str,
getter: &G, getter: &G,
) -> Option<SourceMap> { ) -> Option<SourceMap> {
builtin_source_map(script_name) builtin_source_map(file_name)
.or_else(|| getter.get_source_map(script_name)) .or_else(|| getter.get_source_map(file_name))
.and_then(|raw_source_map| SourceMap::from_slice(&raw_source_map).ok()) .and_then(|raw_source_map| SourceMap::from_slice(&raw_source_map).ok())
} }
@ -175,8 +177,8 @@ mod tests {
struct MockSourceMapGetter {} struct MockSourceMapGetter {}
impl SourceMapGetter for MockSourceMapGetter { impl SourceMapGetter for MockSourceMapGetter {
fn get_source_map(&self, script_name: &str) -> Option<Vec<u8>> { fn get_source_map(&self, file_name: &str) -> Option<Vec<u8>> {
let s = match script_name { let s = match file_name {
"foo_bar.ts" => { "foo_bar.ts" => {
r#"{"sources": ["foo_bar.ts"], "mappings":";;;IAIA,OAAO,CAAC,GAAG,CAAC,qBAAqB,EAAE,EAAE,CAAC,OAAO,CAAC,CAAC;IAC/C,OAAO,CAAC,GAAG,CAAC,eAAe,EAAE,IAAI,CAAC,QAAQ,CAAC,IAAI,CAAC,CAAC;IACjD,OAAO,CAAC,GAAG,CAAC,WAAW,EAAE,IAAI,CAAC,QAAQ,CAAC,EAAE,CAAC,CAAC;IAE3C,OAAO,CAAC,GAAG,CAAC,GAAG,CAAC,CAAC"}"# r#"{"sources": ["foo_bar.ts"], "mappings":";;;IAIA,OAAO,CAAC,GAAG,CAAC,qBAAqB,EAAE,EAAE,CAAC,OAAO,CAAC,CAAC;IAC/C,OAAO,CAAC,GAAG,CAAC,eAAe,EAAE,IAAI,CAAC,QAAQ,CAAC,IAAI,CAAC,CAAC;IACjD,OAAO,CAAC,GAAG,CAAC,WAAW,EAAE,IAAI,CAAC,QAAQ,CAAC,EAAE,CAAC,CAAC;IAE3C,OAAO,CAAC,GAAG,CAAC,GAAG,CAAC,CAAC"}"#
} }
@ -190,10 +192,10 @@ mod tests {
fn get_source_line( fn get_source_line(
&self, &self,
script_name: &str, file_name: &str,
line_number: usize, line_number: usize,
) -> Option<String> { ) -> Option<String> {
let s = match script_name { let s = match file_name {
"foo_bar.ts" => vec![ "foo_bar.ts" => vec![
"console.log('foo');", "console.log('foo');",
"console.log('foo');", "console.log('foo');",

13
cli/tests/error_024_stack_promise_all.ts Normal file
View file

@ -0,0 +1,13 @@
const p = Promise.all([
(async (): Promise<never> => {
await Promise.resolve();
throw new Error("Promise.all()");
})(),
]);
try {
await p;
} catch (error) {
console.log(error.stack);
throw error;
}

8
cli/tests/error_024_stack_promise_all.ts.out Normal file
View file

@ -0,0 +1,8 @@
[WILDCARD]Error: Promise.all()
at [WILDCARD]tests/error_024_stack_promise_all.ts:[WILDCARD]
at async Promise.all (index 0)
at async [WILDCARD]tests/error_024_stack_promise_all.ts:[WILDCARD]
error: Uncaught Error: Promise.all()
at [WILDCARD]tests/error_024_stack_promise_all.ts:[WILDCARD]
at async Promise.all (index 0)
at async [WILDCARD]tests/error_024_stack_promise_all.ts:[WILDCARD]

7
cli/tests/integration_tests.rs
View file

@ -1406,6 +1406,13 @@ itest!(error_023_stack_async {
exit_code: 1, exit_code: 1,
}); });
itest!(error_024_stack_promise_all {
args: "error_024_stack_promise_all.ts",
output: "error_024_stack_promise_all.ts.out",
check_stderr: true,
exit_code: 1,
});
itest!(error_syntax { itest!(error_syntax {
args: "run --reload error_syntax.js", args: "run --reload error_syntax.js",
check_stderr: true, check_stderr: true,

139
core/js_errors.rs
View file

@ -1,8 +1,5 @@
// Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
// Note that source_map_mappings requires 0-indexed line and column numbers but
// V8 Exceptions are 1-indexed.
// TODO: This currently only applies to uncaught exceptions. It would be nice to // TODO: This currently only applies to uncaught exceptions. It would be nice to
// also have source maps for situations like this: // also have source maps for situations like this:
// const err = new Error("Boo!"); // const err = new Error("Boo!");
@ -25,22 +22,28 @@ pub struct JSError {
pub source_line: Option<String>, pub source_line: Option<String>,
pub script_resource_name: Option<String>, pub script_resource_name: Option<String>,
pub line_number: Option<i64>, pub line_number: Option<i64>,
pub start_column: Option<i64>, pub start_column: Option<i64>, // 0-based
pub end_column: Option<i64>, pub end_column: Option<i64>, // 0-based
pub frames: Vec<JSStackFrame>, pub frames: Vec<JSStackFrame>,
pub formatted_frames: Vec<String>, pub formatted_frames: Vec<String>,
} }
#[derive(Debug, PartialEq, Clone)] #[derive(Debug, PartialEq, Clone)]
pub struct JSStackFrame { pub struct JSStackFrame {
pub line_number: i64, // zero indexed pub type_name: Option<String>,
pub column: i64, // zero indexed pub function_name: Option<String>,
pub script_name: String, pub method_name: Option<String>,
pub function_name: String, pub file_name: Option<String>,
pub line_number: Option<i64>,
pub column_number: Option<i64>,
pub eval_origin: Option<String>,
pub is_top_level: Option<bool>,
pub is_eval: bool, pub is_eval: bool,
pub is_native: bool,
pub is_constructor: bool, pub is_constructor: bool,
pub is_async: bool, pub is_async: bool,
// TODO(nayeemrmn): Support more CallSite fields. pub is_promise_all: bool,
pub promise_index: Option<i64>,
} }
fn get_property<'a>( fn get_property<'a>(
@ -96,56 +99,106 @@ impl JSError {
.unwrap() .unwrap()
.try_into() .try_into()
.unwrap(); .unwrap();
let line_number: v8::Local<v8::Integer> = let type_name: Option<v8::Local<v8::String>> =
get_property(scope, context, call_site, "typeName")
.unwrap()
.try_into()
.ok();
let type_name = type_name.map(|s| s.to_rust_string_lossy(scope));
let function_name: Option<v8::Local<v8::String>> =
get_property(scope, context, call_site, "functionName")
.unwrap()
.try_into()
.ok();
let function_name =
function_name.map(|s| s.to_rust_string_lossy(scope));
let method_name: Option<v8::Local<v8::String>> =
get_property(scope, context, call_site, "methodName")
.unwrap()
.try_into()
.ok();
let method_name = method_name.map(|s| s.to_rust_string_lossy(scope));
let file_name: Option<v8::Local<v8::String>> =
get_property(scope, context, call_site, "fileName")
.unwrap()
.try_into()
.ok();
let file_name = file_name.map(|s| s.to_rust_string_lossy(scope));
let line_number: Option<v8::Local<v8::Integer>> =
get_property(scope, context, call_site, "lineNumber") get_property(scope, context, call_site, "lineNumber")
.unwrap() .unwrap()
.try_into() .try_into()
.unwrap(); .ok();
let line_number = line_number.value() - 1; let line_number = line_number.map(|n| n.value());
let column_number: v8::Local<v8::Integer> = let column_number: Option<v8::Local<v8::Integer>> =
get_property(scope, context, call_site, "columnNumber") get_property(scope, context, call_site, "columnNumber")
.unwrap() .unwrap()
.try_into() .try_into()
.unwrap(); .ok();
let column_number = column_number.value() - 1; let column_number = column_number.map(|n| n.value());
let file_name: Result<v8::Local<v8::String>, _> = let eval_origin: Option<v8::Local<v8::String>> =
get_property(scope, context, call_site, "fileName") get_property(scope, context, call_site, "evalOrigin")
.unwrap()
.try_into();
let file_name = file_name
.map_or_else(|_| String::new(), |s| s.to_rust_string_lossy(scope));
let function_name: Result<v8::Local<v8::String>, _> =
get_property(scope, context, call_site, "functionName")
.unwrap()
.try_into();
let function_name = function_name
.map_or_else(|_| String::new(), |s| s.to_rust_string_lossy(scope));
let is_constructor: v8::Local<v8::Boolean> =
get_property(scope, context, call_site, "isConstructor")
.unwrap() .unwrap()
.try_into() .try_into()
.unwrap(); .ok();
let is_constructor = is_constructor.is_true(); let eval_origin = eval_origin.map(|s| s.to_rust_string_lossy(scope));
let is_top_level: Option<v8::Local<v8::Boolean>> =
get_property(scope, context, call_site, "isTopLevel")
.unwrap()
.try_into()
.ok();
let is_top_level = is_top_level.map(|b| b.is_true());
let is_eval: v8::Local<v8::Boolean> = let is_eval: v8::Local<v8::Boolean> =
get_property(scope, context, call_site, "isEval") get_property(scope, context, call_site, "isEval")
.unwrap() .unwrap()
.try_into() .try_into()
.unwrap(); .unwrap();
let is_eval = is_eval.is_true(); let is_eval = is_eval.is_true();
let is_native: v8::Local<v8::Boolean> =
get_property(scope, context, call_site, "isNative")
.unwrap()
.try_into()
.unwrap();
let is_native = is_native.is_true();
let is_constructor: v8::Local<v8::Boolean> =
get_property(scope, context, call_site, "isConstructor")
.unwrap()
.try_into()
.unwrap();
let is_constructor = is_constructor.is_true();
let is_async: v8::Local<v8::Boolean> = let is_async: v8::Local<v8::Boolean> =
get_property(scope, context, call_site, "isAsync") get_property(scope, context, call_site, "isAsync")
.unwrap() .unwrap()
.try_into() .try_into()
.unwrap(); .unwrap();
let is_async = is_async.is_true(); let is_async = is_async.is_true();
let is_promise_all: v8::Local<v8::Boolean> =
get_property(scope, context, call_site, "isPromiseAll")
.unwrap()
.try_into()
.unwrap();
let is_promise_all = is_promise_all.is_true();
let promise_index: Option<v8::Local<v8::Integer>> =
get_property(scope, context, call_site, "columnNumber")
.unwrap()
.try_into()
.ok();
let promise_index = promise_index.map(|n| n.value());
frames.push(JSStackFrame { frames.push(JSStackFrame {
line_number, type_name,
column: column_number,
script_name: file_name,
function_name, function_name,
is_constructor, method_name,
file_name,
line_number,
column_number,
eval_origin,
is_top_level,
is_eval, is_eval,
is_native,
is_constructor,
is_async, is_async,
is_promise_all,
promise_index,
}); });
let formatted_frame: v8::Local<v8::String> = formatted_frames_v8 let formatted_frame: v8::Local<v8::String> = formatted_frames_v8
.get_index(scope, context, i) .get_index(scope, context, i)
@ -181,14 +234,14 @@ impl JSError {
impl Error for JSError {} impl Error for JSError {}
fn format_source_loc( fn format_source_loc(
script_name: &str, file_name: &str,
line_number: i64, line_number: i64,
column: i64, column_number: i64,
) -> String { ) -> String {
// TODO match this style with how typescript displays errors. // TODO match this style with how typescript displays errors.
let line_number = line_number + 1; let line_number = line_number;
let column = column + 1; let column_number = column_number;
format!("{}:{}:{}", script_name, line_number, column) format!("{}:{}:{}", file_name, line_number, column_number)
} }
impl fmt::Display for JSError { impl fmt::Display for JSError {
@ -200,8 +253,8 @@ impl fmt::Display for JSError {
assert!(self.start_column.is_some()); assert!(self.start_column.is_some());
let source_loc = format_source_loc( let source_loc = format_source_loc(
script_resource_name, script_resource_name,
self.line_number.unwrap() - 1, self.line_number.unwrap(),
self.start_column.unwrap() - 1, self.start_column.unwrap(),
); );
write!(f, "{}", source_loc)?; write!(f, "{}", source_loc)?;
} }