From 56cb59462c587d640f104b9ee8613b9761e3938a Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Fri, 10 Apr 2020 23:50:17 +0100 Subject: [PATCH 1/3] refactor(cli/fmt_errors): Only format stack frames in prepareStackTrace() --- cli/fmt_errors.rs | 115 +------------ cli/js/colors.ts | 13 ++ cli/js/error_stack.ts | 63 ++++--- cli/source_maps.rs | 154 +----------------- cli/tests/044_bad_resource.ts.out | 2 +- cli/tests/error_004_missing_module.ts.out | 4 +- .../error_005_missing_dynamic_import.ts.out | 4 +- cli/tests/error_006_import_ext_failure.ts.out | 4 +- .../error_011_bad_module_specifier.ts.out | 4 +- ...or_012_bad_dynamic_import_specifier.ts.out | 4 +- cli/tests/error_type_definitions.ts.out | 4 +- core/js_errors.rs | 141 +++------------- 12 files changed, 105 insertions(+), 407 deletions(-) diff --git a/cli/fmt_errors.rs b/cli/fmt_errors.rs index b7cfbbce93951c..784bcc9c9a5acc 100644 --- a/cli/fmt_errors.rs +++ b/cli/fmt_errors.rs @@ -4,7 +4,6 @@ use crate::colors; use crate::source_maps::apply_source_map; use crate::source_maps::SourceMapGetter; use deno_core::ErrBox; -use deno_core::JSStackFrame; use std::error::Error; use std::fmt; use std::ops::Deref; @@ -25,18 +24,13 @@ fn format_source_name( script_name: String, line_number: i64, column: i64, - is_internal: bool, ) -> String { let line_number = line_number + 1; let column = column + 1; - if is_internal { - format!("{}:{}:{}", script_name, line_number, column) - } else { - let script_name_c = colors::cyan(script_name); - let line_c = colors::yellow(line_number.to_string()); - let column_c = colors::yellow(column.to_string()); - format!("{}:{}:{}", script_name_c, line_c, column_c) - } + let script_name_c = colors::cyan(script_name); + let line_c = colors::yellow(line_number.to_string()); + let column_c = colors::yellow(column.to_string()); + format!("{}:{}:{}", script_name_c, line_c, column_c) } /// Formats optional source, line number and column into a single string. @@ -55,7 +49,6 @@ pub fn format_maybe_source_name( script_name.unwrap(), line_number.unwrap(), column.unwrap(), - false, ) } @@ -127,54 +120,6 @@ pub fn format_error_message(msg: String) -> String { format!("{} {}", preamble, msg) } -fn format_stack_frame(frame: &JSStackFrame, is_internal_frame: bool) -> String { - // Note when we print to string, we change from 0-indexed to 1-indexed. - let function_name = if is_internal_frame { - colors::italic_bold_gray(frame.function_name.clone()).to_string() - } else { - colors::italic_bold(frame.function_name.clone()).to_string() - }; - let mut source_loc = format_source_name( - frame.script_name.clone(), - frame.line_number, - frame.column, - is_internal_frame, - ); - - // Each chunk of styled text is auto-resetted on end, - // which make nesting not working. - // Explicitly specify color for each section. - let mut at_prefix = " at".to_owned(); - if is_internal_frame { - at_prefix = colors::gray(at_prefix).to_string(); - } - if !frame.function_name.is_empty() || frame.is_eval { - source_loc = format!("({})", source_loc); // wrap then style - } - if is_internal_frame { - source_loc = colors::gray(source_loc).to_string(); - } - if !frame.function_name.is_empty() { - 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) - } -} - /// Wrapper around deno_core::JSError which provides color to_string. #[derive(Debug)] pub struct JSError(deno_core::JSError); @@ -251,10 +196,8 @@ impl fmt::Display for JSError { self.format_source_name(), self.format_source_line(0), )?; - - for frame in &self.0.frames { - let is_internal_frame = frame.script_name.starts_with("$deno$"); - write!(f, "\n{}", format_stack_frame(&frame, is_internal_frame))?; + for formatted_frame in &self.0.formatted_frames { + write!(f, "\n at {}", formatted_frame)?; } Ok(()) } @@ -267,52 +210,6 @@ mod tests { use super::*; use crate::colors::strip_ansi_codes; - #[test] - fn js_error_to_string() { - let core_js_error = deno_core::JSError { - message: "Error: foo bar".to_string(), - source_line: None, - script_resource_name: None, - line_number: None, - start_column: None, - end_column: None, - frames: vec![ - JSStackFrame { - line_number: 4, - column: 16, - script_name: "foo_bar.ts".to_string(), - function_name: "foo".to_string(), - is_eval: false, - is_constructor: false, - is_async: false, - }, - JSStackFrame { - line_number: 5, - column: 20, - script_name: "bar_baz.ts".to_string(), - function_name: "qat".to_string(), - is_eval: false, - is_constructor: false, - is_async: false, - }, - JSStackFrame { - line_number: 1, - column: 1, - script_name: "deno_main.js".to_string(), - function_name: "".to_string(), - is_eval: false, - is_constructor: false, - is_async: false, - }, - ], - already_source_mapped: true, - }; - let formatted_error = JSError(core_js_error).to_string(); - let actual = strip_ansi_codes(&formatted_error); - let expected = "error: Error: foo bar\n at foo (foo_bar.ts:5:17)\n at qat (bar_baz.ts:6:21)\n at deno_main.js:2:2"; - assert_eq!(actual, expected); - } - #[test] fn test_format_none_source_name() { let actual = format_maybe_source_name(None, None, None); diff --git a/cli/js/colors.ts b/cli/js/colors.ts index d0ecdd13962fc7..9b5e7be4ef79f3 100644 --- a/cli/js/colors.ts +++ b/cli/js/colors.ts @@ -62,3 +62,16 @@ export function white(str: string): string { export function gray(str: string): string { return run(str, code(90, 39)); } + +// https://github.com/chalk/ansi-regex/blob/2b56fb0c7a07108e5b54241e8faec160d393aedb/index.js +const ANSI_PATTERN = new RegExp( + [ + "[\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)", + "(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-ntqry=><~]))", + ].join("|"), + "g" +); + +export function stripColor(string: string): string { + return string.replace(ANSI_PATTERN, ""); +} diff --git a/cli/js/error_stack.ts b/cli/js/error_stack.ts index bab179e489f0ff..43723dff6c4fae 100644 --- a/cli/js/error_stack.ts +++ b/cli/js/error_stack.ts @@ -1,6 +1,7 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. // Some of the code here is adapted directly from V8 and licensed under a BSD // style license available here: https://github.com/v8/v8/blob/24886f2d1c565287d33d71e4109a53bf0b54b75c/LICENSE.v8 +import * as colors from "./colors.ts"; import { applySourceMap, Location } from "./ops/errors.ts"; import { assert } from "./util.ts"; import { exposeForTest } from "./internals.ts"; @@ -93,9 +94,12 @@ function getMethodCall(callSite: CallSite): string { return result; } -function getFileLocation(callSite: CallSite): string { +function getFileLocation(callSite: CallSite, isInternal = false): string { + const cyan = isInternal ? colors.gray : colors.cyan; + const yellow = isInternal ? colors.gray : colors.yellow; + const black = isInternal ? colors.gray : (s: string): string => s; if (callSite.isNative()) { - return "native"; + return cyan("native"); } let result = ""; @@ -104,29 +108,32 @@ function getFileLocation(callSite: CallSite): string { if (!fileName && callSite.isEval()) { const evalOrigin = callSite.getEvalOrigin(); assert(evalOrigin != null); - result += `${evalOrigin}, `; + result += cyan(`${evalOrigin}, `); } if (fileName) { - result += fileName; + result += cyan(fileName); } else { - result += ""; + result += cyan(""); } const lineNumber = callSite.getLineNumber(); if (lineNumber != null) { - result += `:${lineNumber}`; + result += `${black(":")}${yellow(lineNumber.toString())}`; const columnNumber = callSite.getColumnNumber(); if (columnNumber != null) { - result += `:${columnNumber}`; + result += `${black(":")}${yellow(columnNumber.toString())}`; } } return result; } -function callSiteToString(callSite: CallSite): string { +function callSiteToString(callSite: CallSite, isInternal = false): string { + const cyan = isInternal ? colors.gray : colors.cyan; + const black = isInternal ? colors.gray : (s: string): string => s; + let result = ""; const functionName = callSite.getFunctionName(); @@ -137,29 +144,33 @@ function callSiteToString(callSite: CallSite): string { const isMethodCall = !(isTopLevel || isConstructor); if (isAsync) { - result += "async "; + result += colors.gray("async "); } if (isPromiseAll) { - result += `Promise.all (index ${callSite.getPromiseIndex()})`; + result += colors.bold( + colors.italic(black(`Promise.all (index ${callSite.getPromiseIndex()})`)) + ); return result; } if (isMethodCall) { - result += getMethodCall(callSite); + result += colors.bold(colors.italic(black(getMethodCall(callSite)))); } else if (isConstructor) { - result += "new "; + result += colors.gray("new "); if (functionName) { - result += functionName; + result += colors.bold(colors.italic(black(functionName))); } else { - result += ""; + result += cyan(""); } } else if (functionName) { - result += functionName; + result += colors.bold(colors.italic(black(functionName))); } else { - result += getFileLocation(callSite); + result += getFileLocation(callSite, isInternal); return result; } - result += ` (${getFileLocation(callSite)})`; + result += ` ${black("(")}${getFileLocation(callSite, isInternal)}${black( + ")" + )}`; return result; } @@ -207,7 +218,10 @@ function prepareStackTrace( error: Error, structuredStackTrace: CallSite[] ): string { - Object.defineProperty(error, "__callSiteEvals", { value: [] }); + Object.defineProperties(error, { + __callSiteEvals: { value: [] }, + __formattedFrames: { value: [] }, + }); const errorString = `${error.name}: ${error.message}\n` + structuredStackTrace @@ -230,16 +244,23 @@ function prepareStackTrace( } ) .map((callSite): string => { + const isInternal = + callSite.getFileName()?.startsWith("$deno$") ?? false; + const string = callSiteToString(callSite, isInternal); const callSiteEv = Object.freeze(evaluateCallSite(callSite)); if (callSiteEv.lineNumber != null && callSiteEv.columnNumber != null) { // @ts-ignore - error["__callSiteEvals"].push(callSiteEv); + error.__callSiteEvals.push(callSiteEv); + // @ts-ignore + error.__formattedFrames.push(string); } - return ` at ${callSiteToString(callSite)}`; + return ` at ${colors.stripColor(string)}`; }) .join("\n"); // @ts-ignore - Object.freeze(error["__callSiteEvals"]); + Object.freeze(error.__callSiteEvals); + // @ts-ignore + Object.freeze(error.__formattedFrames); return errorString; } diff --git a/cli/source_maps.rs b/cli/source_maps.rs index b22cd44694f6cd..2d442b82341364 100644 --- a/cli/source_maps.rs +++ b/cli/source_maps.rs @@ -1,6 +1,5 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. //! This mod provides functions to remap a deno_core::deno_core::JSError based on a source map -use deno_core::JSStackFrame; use sourcemap::SourceMap; use std::collections::HashMap; use std::str; @@ -47,19 +46,10 @@ pub fn apply_source_map( js_error: &deno_core::JSError, getter: &G, ) -> deno_core::JSError { + // Note that js_error.frames has already been source mapped in + // prepareStackTrace(). let mut mappings_map: CachedMaps = HashMap::new(); - let frames = if !js_error.already_source_mapped { - let mut frames = Vec::::new(); - for frame in &js_error.frames { - let f = frame_apply_source_map(&frame, &mut mappings_map, getter); - frames.push(f); - } - frames - } else { - js_error.frames.clone() - }; - let (script_resource_name, line_number, start_column) = get_maybe_orig_position( js_error.script_resource_name.clone(), @@ -102,32 +92,8 @@ pub fn apply_source_map( line_number, start_column, end_column, - frames, - already_source_mapped: js_error.already_source_mapped, - } -} - -fn frame_apply_source_map( - frame: &JSStackFrame, - mappings_map: &mut CachedMaps, - getter: &G, -) -> JSStackFrame { - let (script_name, line_number, column) = get_orig_position( - frame.script_name.to_string(), - frame.line_number, - frame.column, - mappings_map, - getter, - ); - - JSStackFrame { - script_name, - function_name: frame.function_name.clone(), - line_number, - column, - is_eval: frame.is_eval, - is_constructor: frame.is_constructor, - is_async: frame.is_async, + frames: js_error.frames.clone(), + formatted_frames: js_error.formatted_frames.clone(), } } @@ -245,116 +211,6 @@ mod tests { } } - #[test] - fn apply_source_map_1() { - let core_js_error = deno_core::JSError { - message: "Error: foo bar".to_string(), - source_line: None, - script_resource_name: None, - line_number: None, - start_column: None, - end_column: None, - frames: vec![ - JSStackFrame { - line_number: 4, - column: 16, - script_name: "foo_bar.ts".to_string(), - function_name: "foo".to_string(), - is_eval: false, - is_constructor: false, - is_async: false, - }, - JSStackFrame { - line_number: 5, - column: 20, - script_name: "bar_baz.ts".to_string(), - function_name: "qat".to_string(), - is_eval: false, - is_constructor: false, - is_async: false, - }, - JSStackFrame { - line_number: 1, - column: 1, - script_name: "deno_main.js".to_string(), - function_name: "".to_string(), - is_eval: false, - is_constructor: false, - is_async: false, - }, - ], - already_source_mapped: false, - }; - let getter = MockSourceMapGetter {}; - let actual = apply_source_map(&core_js_error, &getter); - let expected = deno_core::JSError { - message: "Error: foo bar".to_string(), - source_line: None, - script_resource_name: None, - line_number: None, - start_column: None, - end_column: None, - frames: vec![ - JSStackFrame { - line_number: 5, - column: 12, - script_name: "foo_bar.ts".to_string(), - function_name: "foo".to_string(), - is_eval: false, - is_constructor: false, - is_async: false, - }, - JSStackFrame { - line_number: 4, - column: 14, - script_name: "bar_baz.ts".to_string(), - function_name: "qat".to_string(), - is_eval: false, - is_constructor: false, - is_async: false, - }, - JSStackFrame { - line_number: 1, - column: 1, - script_name: "deno_main.js".to_string(), - function_name: "".to_string(), - is_eval: false, - is_constructor: false, - is_async: false, - }, - ], - already_source_mapped: false, - }; - assert_eq!(actual, expected); - } - - #[test] - fn apply_source_map_2() { - let e = deno_core::JSError { - message: "TypeError: baz".to_string(), - source_line: None, - script_resource_name: None, - line_number: None, - start_column: None, - end_column: None, - frames: vec![JSStackFrame { - line_number: 11, - column: 12, - script_name: "CLI_SNAPSHOT.js".to_string(), - function_name: "setLogDebug".to_string(), - is_eval: false, - is_constructor: false, - is_async: false, - }], - already_source_mapped: false, - }; - let getter = MockSourceMapGetter {}; - let actual = apply_source_map(&e, &getter); - assert_eq!(actual.message, "TypeError: baz"); - // Because this is accessing the live bundle, this test might be more fragile - assert_eq!(actual.frames.len(), 1); - } - #[test] fn apply_source_map_line() { let e = deno_core::JSError { @@ -365,7 +221,7 @@ mod tests { start_column: Some(16), end_column: None, frames: vec![], - already_source_mapped: false, + formatted_frames: vec![], }; let getter = MockSourceMapGetter {}; let actual = apply_source_map(&e, &getter); diff --git a/cli/tests/044_bad_resource.ts.out b/cli/tests/044_bad_resource.ts.out index 026beb341771b9..a54c800f847565 100644 --- a/cli/tests/044_bad_resource.ts.out +++ b/cli/tests/044_bad_resource.ts.out @@ -2,5 +2,5 @@ error: Uncaught BadResource: Bad resource ID [WILDCARD]dispatch_json.ts:[WILDCARD] at unwrapResponse ([WILDCARD]dispatch_json.ts:[WILDCARD]) - at sendAsync ([WILDCARD]dispatch_json.ts:[WILDCARD]) + at Object.sendAsync ([WILDCARD]dispatch_json.ts:[WILDCARD]) at async main ([WILDCARD]tests/044_bad_resource.ts:[WILDCARD]) diff --git a/cli/tests/error_004_missing_module.ts.out b/cli/tests/error_004_missing_module.ts.out index 691d5ce5ad08a6..e82cb00da3f02f 100644 --- a/cli/tests/error_004_missing_module.ts.out +++ b/cli/tests/error_004_missing_module.ts.out @@ -1,9 +1,9 @@ [WILDCARD]error: Uncaught NotFound: Cannot resolve module "[WILDCARD]/bad-module.ts" from "[WILDCARD]/error_004_missing_module.ts" [WILDCARD]dispatch_json.ts:[WILDCARD] at unwrapResponse ([WILDCARD]dispatch_json.ts:[WILDCARD]) - at sendAsync ([WILDCARD]dispatch_json.ts:[WILDCARD]) - at async processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) + at Object.sendAsync ([WILDCARD]dispatch_json.ts:[WILDCARD]) at async processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) + at async Object.processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) at async compile ([WILDCARD]compiler.ts:[WILDCARD]) at async tsCompilerOnMessage ([WILDCARD]compiler.ts:[WILDCARD]) at async workerMessageRecvCallback ([WILDCARD]runtime_worker.ts:[WILDCARD]) diff --git a/cli/tests/error_005_missing_dynamic_import.ts.out b/cli/tests/error_005_missing_dynamic_import.ts.out index 4a7822e43146d2..a67241ec19288f 100644 --- a/cli/tests/error_005_missing_dynamic_import.ts.out +++ b/cli/tests/error_005_missing_dynamic_import.ts.out @@ -1,9 +1,9 @@ [WILDCARD]error: Uncaught NotFound: Cannot resolve module "[WILDCARD]/bad-module.ts" from "[WILDCARD]/error_005_missing_dynamic_import.ts" [WILDCARD]dispatch_json.ts:[WILDCARD] at unwrapResponse ([WILDCARD]dispatch_json.ts:[WILDCARD]) - at sendAsync ([WILDCARD]dispatch_json.ts:[WILDCARD]) - at async processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) + at Object.sendAsync ([WILDCARD]dispatch_json.ts:[WILDCARD]) at async processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) + at async Object.processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) at async compile ([WILDCARD]compiler.ts:[WILDCARD]) at async tsCompilerOnMessage ([WILDCARD]compiler.ts:[WILDCARD]) at async workerMessageRecvCallback ([WILDCARD]runtime_worker.ts:[WILDCARD]) diff --git a/cli/tests/error_006_import_ext_failure.ts.out b/cli/tests/error_006_import_ext_failure.ts.out index c4e6a10371ba58..57830c4058cea8 100644 --- a/cli/tests/error_006_import_ext_failure.ts.out +++ b/cli/tests/error_006_import_ext_failure.ts.out @@ -1,9 +1,9 @@ [WILDCARD]error: Uncaught NotFound: Cannot resolve module "[WILDCARD]/non-existent" from "[WILDCARD]/error_006_import_ext_failure.ts" [WILDCARD]dispatch_json.ts:[WILDCARD] at unwrapResponse ([WILDCARD]dispatch_json.ts:[WILDCARD]) - at sendAsync[WILDCARD] ([WILDCARD]dispatch_json.ts:[WILDCARD]) - at async processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) + at Object.sendAsync ([WILDCARD]dispatch_json.ts:[WILDCARD]) at async processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) + at async Object.processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) at async compile ([WILDCARD]compiler.ts:[WILDCARD]) at async tsCompilerOnMessage ([WILDCARD]compiler.ts:[WILDCARD]) at async workerMessageRecvCallback ([WILDCARD]runtime_worker.ts:[WILDCARD]) diff --git a/cli/tests/error_011_bad_module_specifier.ts.out b/cli/tests/error_011_bad_module_specifier.ts.out index 5ac95002ff8057..d85177dc1e172a 100644 --- a/cli/tests/error_011_bad_module_specifier.ts.out +++ b/cli/tests/error_011_bad_module_specifier.ts.out @@ -1,10 +1,10 @@ [WILDCARD]error: Uncaught URIError: relative import path "bad-module.ts" not prefixed with / or ./ or ../ Imported from "[WILDCARD]/error_011_bad_module_specifier.ts" [WILDCARD]dispatch_json.ts:[WILDCARD] at unwrapResponse ([WILDCARD]ops/dispatch_json.ts:[WILDCARD]) - at sendSync ([WILDCARD]ops/dispatch_json.ts:[WILDCARD]) + at Object.sendSync ([WILDCARD]ops/dispatch_json.ts:[WILDCARD]) at resolveModules ([WILDCARD]compiler/imports.ts:[WILDCARD]) at processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) - at processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) + at Object.processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) at async compile ([WILDCARD]compiler.ts:[WILDCARD]) at async tsCompilerOnMessage ([WILDCARD]compiler.ts:[WILDCARD]) at async workerMessageRecvCallback ([WILDCARD]runtime_worker.ts:[WILDCARD]) diff --git a/cli/tests/error_012_bad_dynamic_import_specifier.ts.out b/cli/tests/error_012_bad_dynamic_import_specifier.ts.out index 505f29f82ef9bb..59b968e81b7a88 100644 --- a/cli/tests/error_012_bad_dynamic_import_specifier.ts.out +++ b/cli/tests/error_012_bad_dynamic_import_specifier.ts.out @@ -1,10 +1,10 @@ [WILDCARD]error: Uncaught URIError: relative import path "bad-module.ts" not prefixed with / or ./ or ../ Imported from "[WILDCARD]/error_012_bad_dynamic_import_specifier.ts" [WILDCARD]dispatch_json.ts:[WILDCARD] at unwrapResponse ([WILDCARD]ops/dispatch_json.ts:[WILDCARD]) - at sendSync ([WILDCARD]ops/dispatch_json.ts:[WILDCARD]) + at Object.sendSync ([WILDCARD]ops/dispatch_json.ts:[WILDCARD]) at resolveModules ([WILDCARD]compiler/imports.ts:[WILDCARD]) at processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) - at processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) + at Object.processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) at async compile ([WILDCARD]compiler.ts:[WILDCARD]) at async tsCompilerOnMessage ([WILDCARD]compiler.ts:[WILDCARD]) at async workerMessageRecvCallback ([WILDCARD]runtime_worker.ts:[WILDCARD]) diff --git a/cli/tests/error_type_definitions.ts.out b/cli/tests/error_type_definitions.ts.out index 59773ac3532c3a..85679ee554bddd 100644 --- a/cli/tests/error_type_definitions.ts.out +++ b/cli/tests/error_type_definitions.ts.out @@ -1,10 +1,10 @@ [WILDCARD]error: Uncaught URIError: relative import path "baz" not prefixed with / or ./ or ../ Imported from "[WILDCARD]/type_definitions/bar.d.ts" [WILDCARD]dispatch_json.ts:[WILDCARD] at unwrapResponse ([WILDCARD]ops/dispatch_json.ts:[WILDCARD]) - at sendSync ([WILDCARD]ops/dispatch_json.ts:[WILDCARD]) + at Object.sendSync ([WILDCARD]ops/dispatch_json.ts:[WILDCARD]) at resolveModules ([WILDCARD]compiler/imports.ts:[WILDCARD]) at processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) - at processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) + at Object.processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) at async processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) at async compile ([WILDCARD]compiler.ts:[WILDCARD]) at async tsCompilerOnMessage ([WILDCARD]compiler.ts:[WILDCARD]) diff --git a/core/js_errors.rs b/core/js_errors.rs index 64009b9ee13d48..87f603a16552e0 100644 --- a/core/js_errors.rs +++ b/core/js_errors.rs @@ -28,12 +28,7 @@ pub struct JSError { pub start_column: Option, pub end_column: Option, pub frames: Vec, - // TODO: Remove this field. It is required because JSError::from_v8_exception - // will generally (but not always) return stack frames passed from - // `prepareStackTrace()` which have already been source-mapped, and we need a - // flag saying not to do it again. Note: applies to `frames` but not - // `source_line`. - pub already_source_mapped: bool, + pub formatted_frames: Vec, } #[derive(Debug, PartialEq, Clone)] @@ -84,10 +79,17 @@ impl JSError { let maybe_call_sites: Option> = maybe_call_sites.and_then(|a| a.try_into().ok()); - let already_source_mapped; - let frames = if let Some(call_sites) = maybe_call_sites { - already_source_mapped = true; - let mut output: Vec = vec![]; + let (frames, formatted_frames) = if let Some(call_sites) = maybe_call_sites + { + let mut frames: Vec = vec![]; + let mut formatted_frames: Vec = vec![]; + + let formatted_frames_v8 = + get_property(scope, context, exception.unwrap(), "__formattedFrames"); + let formatted_frames_v8: v8::Local = formatted_frames_v8 + .and_then(|a| a.try_into().ok()) + .expect("__formattedFrames should be defined if __callSiteEvals is."); + for i in 0..call_sites.length() { let call_site: v8::Local = call_sites .get_index(scope, context, i) @@ -136,7 +138,7 @@ impl JSError { .try_into() .unwrap(); let is_async = is_async.is_true(); - output.push(JSStackFrame { + frames.push(JSStackFrame { line_number, column: column_number, script_name: file_name, @@ -145,43 +147,17 @@ impl JSError { is_eval, is_async, }); + let formatted_frame: v8::Local = formatted_frames_v8 + .get_index(scope, context, i) + .unwrap() + .try_into() + .unwrap(); + let formatted_frame = formatted_frame.to_rust_string_lossy(scope); + formatted_frames.push(formatted_frame) } - output + (frames, formatted_frames) } else { - already_source_mapped = false; - msg - .get_stack_trace(scope) - .map(|stack_trace| { - (0..stack_trace.get_frame_count()) - .map(|i| { - let frame = stack_trace.get_frame(scope, i).unwrap(); - JSStackFrame { - line_number: frame - .get_line_number() - .checked_sub(1) - .and_then(|v| v.try_into().ok()) - .unwrap(), - column: frame - .get_column() - .checked_sub(1) - .and_then(|v| v.try_into().ok()) - .unwrap(), - script_name: frame - .get_script_name_or_source_url(scope) - .map(|v| v.to_rust_string_lossy(scope)) - .unwrap_or_else(|| "".to_owned()), - function_name: frame - .get_function_name(scope) - .map(|v| v.to_rust_string_lossy(scope)) - .unwrap_or_else(|| "".to_owned()), - is_constructor: frame.is_constructor(), - is_eval: frame.is_eval(), - is_async: false, - } - }) - .collect::>() - }) - .unwrap_or_else(Vec::<_>::new) + (vec![], vec![]) }; Self { @@ -197,7 +173,7 @@ impl JSError { start_column: msg.get_start_column().try_into().ok(), end_column: msg.get_end_column().try_into().ok(), frames, - already_source_mapped, + formatted_frames, } } } @@ -215,22 +191,6 @@ fn format_source_loc( format!("{}:{}:{}", script_name, line_number, column) } -fn format_stack_frame(frame: &JSStackFrame) -> String { - // Note when we print to string, we change from 0-indexed to 1-indexed. - let source_loc = - format_source_loc(&frame.script_name, frame.line_number, frame.column); - - if !frame.function_name.is_empty() { - 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) - } -} - impl fmt::Display for JSError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { if self.script_resource_name.is_some() { @@ -261,59 +221,10 @@ impl fmt::Display for JSError { write!(f, "{}", self.message)?; - for frame in &self.frames { - write!(f, "\n{}", format_stack_frame(frame))?; + for formatted_frame in &self.formatted_frames { + // TODO: Strip ANSI color from formatted_frame. + write!(f, "\n at {}", formatted_frame)?; } Ok(()) } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn js_error_to_string() { - let js_error = JSError { - message: "Error: foo bar".to_string(), - source_line: None, - script_resource_name: None, - line_number: None, - start_column: None, - end_column: None, - frames: vec![ - JSStackFrame { - line_number: 4, - column: 16, - script_name: "foo_bar.ts".to_string(), - function_name: "foo".to_string(), - is_eval: false, - is_constructor: false, - is_async: false, - }, - JSStackFrame { - line_number: 5, - column: 20, - script_name: "bar_baz.ts".to_string(), - function_name: "qat".to_string(), - is_eval: false, - is_constructor: false, - is_async: false, - }, - JSStackFrame { - line_number: 1, - column: 1, - script_name: "deno_main.js".to_string(), - function_name: "".to_string(), - is_eval: false, - is_constructor: false, - is_async: false, - }, - ], - already_source_mapped: true, - }; - let actual = js_error.to_string(); - let expected = "Error: foo bar\n at foo (foo_bar.ts:5:17)\n at qat (bar_baz.ts:6:21)\n at deno_main.js:2:2"; - assert_eq!(actual, expected); - } -} From 04335351858f3764944977449b0da9be5ac2860e Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Sat, 11 Apr 2020 00:26:26 +0100 Subject: [PATCH 2/3] fixup! refactor(cli/fmt_errors): Only format stack frames in prepareStackTrace() --- cli/tests/error_type_definitions.ts.out | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cli/tests/error_type_definitions.ts.out b/cli/tests/error_type_definitions.ts.out index 85679ee554bddd..55d0e4dad58794 100644 --- a/cli/tests/error_type_definitions.ts.out +++ b/cli/tests/error_type_definitions.ts.out @@ -1,11 +1,12 @@ [WILDCARD]error: Uncaught URIError: relative import path "baz" not prefixed with / or ./ or ../ Imported from "[WILDCARD]/type_definitions/bar.d.ts" -[WILDCARD]dispatch_json.ts:[WILDCARD] +[WILDCARD]ops/dispatch_json.ts:[WILDCARD] at unwrapResponse ([WILDCARD]ops/dispatch_json.ts:[WILDCARD]) at Object.sendSync ([WILDCARD]ops/dispatch_json.ts:[WILDCARD]) + at Object.resolveModules ([WILDCARD]ops/compiler.ts:[WILDCARD]) at resolveModules ([WILDCARD]compiler/imports.ts:[WILDCARD]) at processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) - at Object.processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) - at async processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) + at processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) + at async Object.processImports ([WILDCARD]compiler/imports.ts:[WILDCARD]) at async compile ([WILDCARD]compiler.ts:[WILDCARD]) at async tsCompilerOnMessage ([WILDCARD]compiler.ts:[WILDCARD]) at async workerMessageRecvCallback ([WILDCARD]runtime_worker.ts:[WILDCARD]) From fbe2a9b36953448e00cc1e340c6dd95a51424fcf Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Sat, 11 Apr 2020 04:51:42 +0100 Subject: [PATCH 3/3] Add tests --- cli/tests/error_019_stack_function.ts | 10 ++++++ cli/tests/error_019_stack_function.ts.out | 6 ++++ cli/tests/error_020_stack_constructor.ts | 12 +++++++ cli/tests/error_020_stack_constructor.ts.out | 6 ++++ cli/tests/error_021_stack_method.ts | 12 +++++++ cli/tests/error_021_stack_method.ts.out | 6 ++++ cli/tests/error_022_stack_custom_error.ts | 10 ++++++ cli/tests/error_022_stack_custom_error.ts.out | 4 +++ cli/tests/error_023_stack_async.ts | 12 +++++++ cli/tests/error_023_stack_async.ts.out | 8 +++++ cli/tests/error_stack.ts | 10 ------ cli/tests/error_stack.ts.out | 6 ---- cli/tests/integration_tests.rs | 34 +++++++++++++++++-- 13 files changed, 117 insertions(+), 19 deletions(-) create mode 100644 cli/tests/error_019_stack_function.ts create mode 100644 cli/tests/error_019_stack_function.ts.out create mode 100644 cli/tests/error_020_stack_constructor.ts create mode 100644 cli/tests/error_020_stack_constructor.ts.out create mode 100644 cli/tests/error_021_stack_method.ts create mode 100644 cli/tests/error_021_stack_method.ts.out create mode 100644 cli/tests/error_022_stack_custom_error.ts create mode 100644 cli/tests/error_022_stack_custom_error.ts.out create mode 100644 cli/tests/error_023_stack_async.ts create mode 100644 cli/tests/error_023_stack_async.ts.out delete mode 100644 cli/tests/error_stack.ts delete mode 100644 cli/tests/error_stack.ts.out diff --git a/cli/tests/error_019_stack_function.ts b/cli/tests/error_019_stack_function.ts new file mode 100644 index 00000000000000..c5eeae8f4ab4e5 --- /dev/null +++ b/cli/tests/error_019_stack_function.ts @@ -0,0 +1,10 @@ +function foo(): never { + throw new Error("function"); +} + +try { + foo(); +} catch (error) { + console.log(error.stack); + throw error; +} diff --git a/cli/tests/error_019_stack_function.ts.out b/cli/tests/error_019_stack_function.ts.out new file mode 100644 index 00000000000000..a360e7d8ed9e5f --- /dev/null +++ b/cli/tests/error_019_stack_function.ts.out @@ -0,0 +1,6 @@ +[WILDCARD]Error: function + at foo ([WILDCARD]tests/error_019_stack_function.ts:[WILDCARD]) + at [WILDCARD]tests/error_019_stack_function.ts:[WILDCARD] +error: Uncaught Error: function + at foo ([WILDCARD]tests/error_019_stack_function.ts:[WILDCARD]) + at [WILDCARD]tests/error_019_stack_function.ts:[WILDCARD] diff --git a/cli/tests/error_020_stack_constructor.ts b/cli/tests/error_020_stack_constructor.ts new file mode 100644 index 00000000000000..49988280b1f735 --- /dev/null +++ b/cli/tests/error_020_stack_constructor.ts @@ -0,0 +1,12 @@ +class A { + constructor() { + throw new Error("constructor"); + } +} + +try { + new A(); +} catch (error) { + console.log(error.stack); + throw error; +} diff --git a/cli/tests/error_020_stack_constructor.ts.out b/cli/tests/error_020_stack_constructor.ts.out new file mode 100644 index 00000000000000..a60e375d71b0bf --- /dev/null +++ b/cli/tests/error_020_stack_constructor.ts.out @@ -0,0 +1,6 @@ +[WILDCARD]Error: constructor + at new A ([WILDCARD]tests/error_020_stack_constructor.ts:[WILDCARD]) + at [WILDCARD]tests/error_020_stack_constructor.ts:[WILDCARD] +error: Uncaught Error: constructor + at new A ([WILDCARD]tests/error_020_stack_constructor.ts:[WILDCARD]) + at [WILDCARD]tests/error_020_stack_constructor.ts:[WILDCARD] diff --git a/cli/tests/error_021_stack_method.ts b/cli/tests/error_021_stack_method.ts new file mode 100644 index 00000000000000..a52d00deb6599d --- /dev/null +++ b/cli/tests/error_021_stack_method.ts @@ -0,0 +1,12 @@ +class A { + m(): never { + throw new Error("method"); + } +} + +try { + new A().m(); +} catch (error) { + console.log(error.stack); + throw error; +} diff --git a/cli/tests/error_021_stack_method.ts.out b/cli/tests/error_021_stack_method.ts.out new file mode 100644 index 00000000000000..fff3e193b06125 --- /dev/null +++ b/cli/tests/error_021_stack_method.ts.out @@ -0,0 +1,6 @@ +[WILDCARD]Error: method + at A.m ([WILDCARD]tests/error_021_stack_method.ts:[WILDCARD]) + at [WILDCARD]tests/error_021_stack_method.ts:[WILDCARD] +error: Uncaught Error: method + at A.m ([WILDCARD]tests/error_021_stack_method.ts:[WILDCARD]) + at [WILDCARD]tests/error_021_stack_method.ts:[WILDCARD] diff --git a/cli/tests/error_022_stack_custom_error.ts b/cli/tests/error_022_stack_custom_error.ts new file mode 100644 index 00000000000000..6f3c1b0bdf0799 --- /dev/null +++ b/cli/tests/error_022_stack_custom_error.ts @@ -0,0 +1,10 @@ +class CustomError extends Error { + constructor(message: string) { + super(message); + this.name = "CustomError"; + } +} + +const error = new CustomError("custom error"); +console.log(error.stack); +throw error; diff --git a/cli/tests/error_022_stack_custom_error.ts.out b/cli/tests/error_022_stack_custom_error.ts.out new file mode 100644 index 00000000000000..89c536ddb19ac8 --- /dev/null +++ b/cli/tests/error_022_stack_custom_error.ts.out @@ -0,0 +1,4 @@ +[WILDCARD]CustomError: custom error + at [WILDCARD]tests/error_022_stack_custom_error.ts:[WILDCARD] +error: Uncaught CustomError: custom error + at [WILDCARD]tests/error_022_stack_custom_error.ts:[WILDCARD] diff --git a/cli/tests/error_023_stack_async.ts b/cli/tests/error_023_stack_async.ts new file mode 100644 index 00000000000000..621933576be699 --- /dev/null +++ b/cli/tests/error_023_stack_async.ts @@ -0,0 +1,12 @@ +const p = (async (): Promise => { + await Promise.resolve().then((): never => { + throw new Error("async"); + }); +})(); + +try { + await p; +} catch (error) { + console.log(error.stack); + throw error; +} diff --git a/cli/tests/error_023_stack_async.ts.out b/cli/tests/error_023_stack_async.ts.out new file mode 100644 index 00000000000000..e5b707ce452545 --- /dev/null +++ b/cli/tests/error_023_stack_async.ts.out @@ -0,0 +1,8 @@ +[WILDCARD]Error: async + at [WILDCARD]tests/error_023_stack_async.ts:[WILDCARD] + at async [WILDCARD]tests/error_023_stack_async.ts:[WILDCARD] + at async [WILDCARD]tests/error_023_stack_async.ts:[WILDCARD] +error: Uncaught Error: async + at [WILDCARD]tests/error_023_stack_async.ts:[WILDCARD] + at async [WILDCARD]tests/error_023_stack_async.ts:[WILDCARD] + at async [WILDCARD]tests/error_023_stack_async.ts:[WILDCARD] diff --git a/cli/tests/error_stack.ts b/cli/tests/error_stack.ts deleted file mode 100644 index f2125d662fefaa..00000000000000 --- a/cli/tests/error_stack.ts +++ /dev/null @@ -1,10 +0,0 @@ -function foo(): never { - throw new Error("foo"); -} - -try { - foo(); -} catch (e) { - console.log(e); - throw e; -} diff --git a/cli/tests/error_stack.ts.out b/cli/tests/error_stack.ts.out deleted file mode 100644 index 2bb629e2d3ea26..00000000000000 --- a/cli/tests/error_stack.ts.out +++ /dev/null @@ -1,6 +0,0 @@ -[WILDCARD]Error: foo - at foo ([WILDCARD]tests/error_stack.ts:2:9) - at [WILDCARD]tests/error_stack.ts:6:3 -error: Uncaught Error: foo - at foo ([WILDCARD]tests/error_stack.ts:2:9) - at [WILDCARD]tests/error_stack.ts:6:3 diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index 31a4a9c66a3d49..d043958a91ec2f 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -1371,11 +1371,39 @@ itest!(error_018_hide_long_source_js { exit_code: 1, }); -itest!(error_stack { - args: "run --reload error_stack.ts", +itest!(error_019_stack_function { + args: "error_019_stack_function.ts", + output: "error_019_stack_function.ts.out", + check_stderr: true, + exit_code: 1, +}); + +itest!(error_020_stack_constructor { + args: "error_020_stack_constructor.ts", + output: "error_020_stack_constructor.ts.out", + check_stderr: true, + exit_code: 1, +}); + +itest!(error_021_stack_method { + args: "error_021_stack_method.ts", + output: "error_021_stack_method.ts.out", + check_stderr: true, + exit_code: 1, +}); + +itest!(error_022_stack_custom_error { + args: "error_022_stack_custom_error.ts", + output: "error_022_stack_custom_error.ts.out", + check_stderr: true, + exit_code: 1, +}); + +itest!(error_023_stack_async { + args: "error_023_stack_async.ts", + output: "error_023_stack_async.ts.out", check_stderr: true, exit_code: 1, - output: "error_stack.ts.out", }); itest!(error_syntax {