From a43984c9cfcea852ca18e1c575f9acdb8f023a5a Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Tue, 22 Sep 2020 18:01:30 +0100 Subject: [PATCH] refactor(cli/fmt_errors): Color stack traces in Rust (#7628) --- cli/fmt_errors.rs | 178 +++++++++++++++++++++++++++++++------- cli/lint.rs | 13 ++- cli/rt/40_error_stack.js | 128 +++++++++++---------------- cli/source_maps.rs | 2 - cli/tsc/40_error_stack.js | 107 +++++++++-------------- core/error.rs | 60 ++++++------- 6 files changed, 279 insertions(+), 209 deletions(-) diff --git a/cli/fmt_errors.rs b/cli/fmt_errors.rs index 8432d36c2e..333c477567 100644 --- a/cli/fmt_errors.rs +++ b/cli/fmt_errors.rs @@ -3,30 +3,156 @@ use crate::colors; use crate::source_maps::apply_source_map; use crate::source_maps::SourceMapGetter; -use deno_core::error::AnyError; -use deno_core::error::JsError as CoreJsError; +use deno_core::error::{AnyError, JsError as CoreJsError, JsStackFrame}; use std::error::Error; use std::fmt; use std::ops::Deref; const SOURCE_ABBREV_THRESHOLD: usize = 150; -pub fn format_location(filename: &str, line: i64, col: i64) -> String { - format!( - "{}:{}:{}", - colors::cyan(filename), - colors::yellow(&line.to_string()), - colors::yellow(&col.to_string()) - ) +fn default_color(s: &str, internal: bool) -> String { + if internal { + colors::gray(s).to_string() + } else { + s.to_string() + } } -pub fn format_stack( +fn cyan(s: &str, internal: bool) -> String { + if internal { + colors::gray(s).to_string() + } else { + colors::cyan(s).to_string() + } +} + +fn yellow(s: &str, internal: bool) -> String { + if internal { + colors::gray(s).to_string() + } else { + colors::yellow(s).to_string() + } +} + +fn italic_bold(s: &str, internal: bool) -> String { + if internal { + colors::italic_bold_gray(s).to_string() + } else { + colors::italic_bold(s).to_string() + } +} + +// Keep in sync with `cli/rt/40_error_stack.js`. +pub fn format_location(frame: &JsStackFrame) -> String { + let internal = frame + .file_name + .as_ref() + .map_or(false, |f| f.starts_with("deno:")); + if frame.is_native { + return cyan("native", internal); + } + let mut result = String::new(); + if let Some(file_name) = &frame.file_name { + result += &cyan(&file_name, internal); + } else { + if frame.is_eval { + result += &(cyan(&frame.eval_origin.as_ref().unwrap(), internal) + ", "); + } + result += &cyan("", internal); + } + if let Some(line_number) = frame.line_number { + result += &format!( + "{}{}", + default_color(":", internal), + yellow(&line_number.to_string(), internal) + ); + if let Some(column_number) = frame.column_number { + result += &format!( + "{}{}", + default_color(":", internal), + yellow(&column_number.to_string(), internal) + ); + } + } + result +} + +// Keep in sync with `cli/rt/40_error_stack.js`. +fn format_frame(frame: &JsStackFrame) -> String { + let internal = frame + .file_name + .as_ref() + .map_or(false, |f| f.starts_with("deno:")); + let is_method_call = + !(frame.is_top_level.unwrap_or_default() || frame.is_constructor); + let mut result = String::new(); + if frame.is_async { + result += &colors::gray("async ").to_string(); + } + if frame.is_promise_all { + result += &italic_bold( + &format!( + "Promise.all (index {})", + frame.promise_index.unwrap_or_default().to_string() + ), + internal, + ); + return result; + } + if is_method_call { + let mut formatted_method = String::new(); + if let Some(function_name) = &frame.function_name { + if let Some(type_name) = &frame.type_name { + if !function_name.starts_with(type_name) { + formatted_method += &format!("{}.", type_name); + } + } + formatted_method += &function_name; + if let Some(method_name) = &frame.method_name { + if !function_name.ends_with(method_name) { + formatted_method += &format!(" [as {}]", method_name); + } + } + } else { + if let Some(type_name) = &frame.type_name { + formatted_method += &format!("{}.", type_name); + } + if let Some(method_name) = &frame.method_name { + formatted_method += &method_name + } else { + formatted_method += ""; + } + } + result += &italic_bold(&formatted_method, internal); + } else if frame.is_constructor { + result += &colors::gray("new ").to_string(); + if let Some(function_name) = &frame.function_name { + result += &italic_bold(&function_name, internal); + } else { + result += &cyan("", internal); + } + } else if let Some(function_name) = &frame.function_name { + result += &italic_bold(&function_name, internal); + } else { + result += &format_location(frame); + return result; + } + result += &format!( + " {}{}{}", + default_color("(", internal), + format_location(frame), + default_color(")", internal) + ); + result +} + +fn format_stack( is_error: bool, message_line: &str, source_line: Option<&str>, start_column: Option, end_column: Option, - formatted_frames: &[String], + frames: &[JsStackFrame], level: usize, ) -> String { let mut s = String::new(); @@ -38,11 +164,11 @@ pub fn format_stack( is_error, level, )); - for formatted_frame in formatted_frames { + for frame in frames { s.push_str(&format!( "\n{:indent$} at {}", "", - formatted_frame, + format_frame(frame), indent = level )); } @@ -128,31 +254,23 @@ impl Deref for JsError { impl fmt::Display for JsError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let mut formatted_frames = self.0.formatted_frames.clone(); - - // The formatted_frames passed from prepareStackTrace() are colored. - if !colors::use_color() { - formatted_frames = formatted_frames - .iter() - .map(|s| colors::strip_ansi_codes(s).to_string()) - .collect(); - } + let mut frames = self.0.frames.clone(); // When the stack frame array is empty, but the source location given by // (script_resource_name, line_number, start_column + 1) exists, this is // likely a syntax error. For the sake of formatting we treat it like it was // given as a single stack frame. - if formatted_frames.is_empty() + if frames.is_empty() && self.0.script_resource_name.is_some() && self.0.line_number.is_some() && self.0.start_column.is_some() { - formatted_frames = vec![format_location( - self.0.script_resource_name.as_ref().unwrap(), - self.0.line_number.unwrap(), - self.0.start_column.unwrap() + 1, - )] - }; + frames = vec![JsStackFrame::from_location( + self.0.script_resource_name.clone(), + self.0.line_number, + self.0.start_column.map(|n| n + 1), + )]; + } write!( f, @@ -163,7 +281,7 @@ impl fmt::Display for JsError { self.0.source_line.as_deref(), self.0.start_column, self.0.end_column, - &formatted_frames, + &frames, 0 ) )?; diff --git a/cli/lint.rs b/cli/lint.rs index 3b337fe7f4..882ec44145 100644 --- a/cli/lint.rs +++ b/cli/lint.rs @@ -12,8 +12,7 @@ use crate::fmt::collect_files; use crate::fmt::run_parallelized; use crate::fmt_errors; use crate::media_type::MediaType; -use deno_core::error::generic_error; -use deno_core::error::AnyError; +use deno_core::error::{generic_error, AnyError, JsStackFrame}; use deno_core::serde_json; use deno_lint::diagnostic::LintDiagnostic; use deno_lint::linter::Linter; @@ -223,11 +222,11 @@ impl LintReporter for PrettyLintReporter { &pretty_message, &source_lines, d.range.clone(), - &fmt_errors::format_location( - &d.filename, - d.range.start.line as i64, - d.range.start.col as i64, - ), + &fmt_errors::format_location(&JsStackFrame::from_location( + Some(d.filename.clone()), + Some(d.range.start.line as i64), + Some(d.range.start.col as i64), + )), ); eprintln!("{}\n", message); diff --git a/cli/rt/40_error_stack.js b/cli/rt/40_error_stack.js index 08647a8d85..5cc1897175 100644 --- a/cli/rt/40_error_stack.js +++ b/cli/rt/40_error_stack.js @@ -4,7 +4,6 @@ // 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 const core = window.Deno.core; - const colors = window.__bootstrap.colors; const assert = window.__bootstrap.util.assert; const internals = window.__bootstrap.internals; @@ -74,81 +73,42 @@ }; } - function getMethodCall(callSite) { - let result = ""; - - const typeName = callSite.getTypeName(); - const methodName = callSite.getMethodName(); - const functionName = callSite.getFunctionName(); - - if (functionName) { - if (typeName) { - const startsWithTypeName = functionName.startsWith(typeName); - if (!startsWithTypeName) { - result += `${typeName}.`; - } - } - result += functionName; - - if (methodName) { - if (!functionName.endsWith(methodName)) { - result += ` [as ${methodName}]`; - } - } - } else { - if (typeName) { - result += `${typeName}.`; - } - if (methodName) { - result += methodName; - } else { - result += ""; - } - } - - return result; - } - - function getFileLocation(callSite, internal = false) { - const cyan = internal ? colors.gray : colors.cyan; - const yellow = internal ? colors.gray : colors.yellow; - const black = internal ? colors.gray : (s) => s; + // Keep in sync with `cli/fmt_errors.rs`. + function formatLocation(callSite) { if (callSite.isNative()) { - return cyan("native"); + return "native"; } let result = ""; const fileName = callSite.getFileName(); - if (!fileName && callSite.isEval()) { - const evalOrigin = callSite.getEvalOrigin(); - assert(evalOrigin != null); - result += cyan(`${evalOrigin}, `); - } if (fileName) { - result += cyan(fileName); + result += fileName; } else { - result += cyan(""); + if (callSite.isEval()) { + const evalOrigin = callSite.getEvalOrigin(); + assert(evalOrigin != null); + result += `${evalOrigin}, `; + } + result += ""; } const lineNumber = callSite.getLineNumber(); if (lineNumber != null) { - result += `${black(":")}${yellow(lineNumber.toString())}`; + result += `:${lineNumber}`; const columnNumber = callSite.getColumnNumber(); if (columnNumber != null) { - result += `${black(":")}${yellow(columnNumber.toString())}`; + result += `:${columnNumber}`; } } return result; } - function callSiteToString(callSite, internal = false) { - const cyan = internal ? colors.gray : colors.cyan; - const black = internal ? colors.gray : (s) => s; - + // Keep in sync with `cli/fmt_errors.rs`. + function formatCallSite(callSite) { let result = ""; const functionName = callSite.getFunctionName(); @@ -159,35 +119,53 @@ const isMethodCall = !(isTopLevel || isConstructor); if (isAsync) { - result += colors.gray("async "); + result += "async "; } if (isPromiseAll) { - result += colors.bold( - colors.italic( - black(`Promise.all (index ${callSite.getPromiseIndex()})`), - ), - ); + result += `Promise.all (index ${callSite.getPromiseIndex()})`; return result; } if (isMethodCall) { - result += colors.bold(colors.italic(black(getMethodCall(callSite)))); - } else if (isConstructor) { - result += colors.gray("new "); + const typeName = callSite.getTypeName(); + const methodName = callSite.getMethodName(); + if (functionName) { - result += colors.bold(colors.italic(black(functionName))); + if (typeName) { + if (!functionName.startsWith(typeName)) { + result += `${typeName}.`; + } + } + result += functionName; + if (methodName) { + if (!functionName.endsWith(methodName)) { + result += ` [as ${methodName}]`; + } + } } else { - result += cyan(""); + if (typeName) { + result += `${typeName}.`; + } + if (methodName) { + result += methodName; + } else { + result += ""; + } + } + } else if (isConstructor) { + result += "new "; + if (functionName) { + result += functionName; + } else { + result += ""; } } else if (functionName) { - result += colors.bold(colors.italic(black(functionName))); + result += functionName; } else { - result += getFileLocation(callSite, internal); + result += formatLocation(callSite); return result; } - result += ` ${black("(")}${getFileLocation(callSite, internal)}${ - black(")") - }`; + result += ` (${formatLocation(callSite)})`; return result; } @@ -236,19 +214,17 @@ ); Object.defineProperties(error, { __callSiteEvals: { value: [], configurable: true }, - __formattedFrames: { value: [], configurable: true }, }); + const formattedCallSites = []; for (const callSite of mappedCallSites) { error.__callSiteEvals.push(Object.freeze(evaluateCallSite(callSite))); - const isInternal = callSite.getFileName()?.startsWith("deno:") ?? false; - error.__formattedFrames.push(callSiteToString(callSite, isInternal)); + formattedCallSites.push(formatCallSite(callSite)); } Object.freeze(error.__callSiteEvals); - Object.freeze(error.__formattedFrames); return ( `${error.name}: ${error.message}\n` + - error.__formattedFrames - .map((s) => ` at ${colors.stripColor(s)}`) + formattedCallSites + .map((s) => ` at ${s}`) .join("\n") ); } diff --git a/cli/source_maps.rs b/cli/source_maps.rs index 70b9faac63..1e7a14a0ec 100644 --- a/cli/source_maps.rs +++ b/cli/source_maps.rs @@ -78,7 +78,6 @@ pub fn apply_source_map( start_column, end_column, frames: js_error.frames.clone(), - formatted_frames: js_error.formatted_frames.clone(), } } @@ -205,7 +204,6 @@ mod tests { start_column: Some(16), end_column: None, frames: vec![], - formatted_frames: vec![], }; let getter = MockSourceMapGetter {}; let actual = apply_source_map(&e, &getter); diff --git a/cli/tsc/40_error_stack.js b/cli/tsc/40_error_stack.js index 77321eb126..7a91a15c8e 100644 --- a/cli/tsc/40_error_stack.js +++ b/cli/tsc/40_error_stack.js @@ -5,19 +5,6 @@ // style license available here: https://github.com/v8/v8/blob/24886f2d1c565287d33d71e4109a53bf0b54b75c/LICENSE.v8 const assert = window.__bootstrap.util.assert; - // 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", - ); - - function stripColor(string) { - return string.replace(ANSI_PATTERN, ""); - } - function patchCallSite(callSite, location) { return { getThis() { @@ -71,42 +58,8 @@ }; } - function getMethodCall(callSite) { - let result = ""; - - const typeName = callSite.getTypeName(); - const methodName = callSite.getMethodName(); - const functionName = callSite.getFunctionName(); - - if (functionName) { - if (typeName) { - const startsWithTypeName = functionName.startsWith(typeName); - if (!startsWithTypeName) { - result += `${typeName}.`; - } - } - result += functionName; - - if (methodName) { - if (!functionName.endsWith(methodName)) { - result += ` [as ${methodName}]`; - } - } - } else { - if (typeName) { - result += `${typeName}.`; - } - if (methodName) { - result += methodName; - } else { - result += ""; - } - } - - return result; - } - - function getFileLocation(callSite) { + // Keep in sync with `cli/fmt_errors.rs`. + function formatLocation(callSite) { if (callSite.isNative()) { return "native"; } @@ -114,32 +67,33 @@ let result = ""; const fileName = callSite.getFileName(); - if (!fileName && callSite.isEval()) { - const evalOrigin = callSite.getEvalOrigin(); - assert(evalOrigin != null); - result += `${evalOrigin}, `; - } if (fileName) { result += fileName; } else { + if (callSite.isEval()) { + const evalOrigin = callSite.getEvalOrigin(); + assert(evalOrigin != null); + result += `${evalOrigin}, `; + } result += ""; } const lineNumber = callSite.getLineNumber(); if (lineNumber != null) { - result += `:${lineNumber.toString()}`; + result += `:${lineNumber}`; const columnNumber = callSite.getColumnNumber(); if (columnNumber != null) { - result += `:${columnNumber.toString()}`; + result += `:${columnNumber}`; } } return result; } - function callSiteToString(callSite) { + // Keep in sync with `cli/fmt_errors.rs`. + function formatCallSite(callSite) { let result = ""; const functionName = callSite.getFunctionName(); @@ -157,7 +111,31 @@ return result; } if (isMethodCall) { - result += getMethodCall(callSite); + const typeName = callSite.getTypeName(); + const methodName = callSite.getMethodName(); + + if (functionName) { + if (typeName) { + if (!functionName.startsWith(typeName)) { + result += `${typeName}.`; + } + } + result += functionName; + if (methodName) { + if (!functionName.endsWith(methodName)) { + result += ` [as ${methodName}]`; + } + } + } else { + if (typeName) { + result += `${typeName}.`; + } + if (methodName) { + result += methodName; + } else { + result += ""; + } + } } else if (isConstructor) { result += "new "; if (functionName) { @@ -168,11 +146,11 @@ } else if (functionName) { result += functionName; } else { - result += getFileLocation(callSite); + result += formatLocation(callSite); return result; } - result += ` (${getFileLocation(callSite)})`; + result += ` (${formatLocation(callSite)})`; return result; } @@ -221,18 +199,17 @@ ); Object.defineProperties(error, { __callSiteEvals: { value: [], configurable: true }, - __formattedFrames: { value: [], configurable: true }, }); + const formattedCallSites = []; for (const callSite of mappedCallSites) { error.__callSiteEvals.push(Object.freeze(evaluateCallSite(callSite))); - error.__formattedFrames.push(callSiteToString(callSite)); + formattedCallSites.push(formatCallSite(callSite)); } Object.freeze(error.__callSiteEvals); - Object.freeze(error.__formattedFrames); return ( `${error.name}: ${error.message}\n` + - error.__formattedFrames - .map((s) => ` at ${stripColor(s)}`) + formattedCallSites + .map((s) => ` at ${s}`) .join("\n") ); } diff --git a/core/error.rs b/core/error.rs index bb23a10d11..25a7ec12bf 100644 --- a/core/error.rs +++ b/core/error.rs @@ -97,7 +97,6 @@ pub struct JsError { pub start_column: Option, // 0-based pub end_column: Option, // 0-based pub frames: Vec, - pub formatted_frames: Vec, } #[derive(Debug, PartialEq, Clone)] @@ -118,6 +117,31 @@ pub struct JsStackFrame { pub promise_index: Option, } +impl JsStackFrame { + pub fn from_location( + file_name: Option, + line_number: Option, + column_number: Option, + ) -> Self { + Self { + type_name: None, + function_name: None, + method_name: None, + file_name, + line_number, + column_number, + eval_origin: None, + is_top_level: None, + is_eval: false, + is_native: false, + is_constructor: false, + is_async: false, + is_promise_all: false, + promise_index: None, + } + } +} + fn get_property<'a>( scope: &mut v8::HandleScope<'a>, object: v8::Local, @@ -142,7 +166,7 @@ impl JsError { let msg = v8::Exception::create_message(scope, exception); - let (message, frames, formatted_frames) = if exception.is_native_error() { + let (message, frames) = if exception.is_native_error() { // The exception is a JS Error object. let exception: v8::Local = exception.clone().try_into().unwrap(); @@ -159,7 +183,7 @@ impl JsError { let message = format!("Uncaught {}: {}", name, message_prop); // Access error.stack to ensure that prepareStackTrace() has been called. - // This should populate error.__callSiteEvals and error.__formattedFrames. + // This should populate error.__callSiteEvals. let _ = get_property(scope, exception, "stack"); // Read an array of structured frames from error.__callSiteEvals. @@ -167,18 +191,9 @@ impl JsError { let frames_v8: Option> = frames_v8.and_then(|a| a.try_into().ok()); - // Read an array of pre-formatted frames from error.__formattedFrames. - let formatted_frames_v8 = - get_property(scope, exception, "__formattedFrames"); - let formatted_frames_v8: Option> = - formatted_frames_v8.and_then(|a| a.try_into().ok()); - // Convert them into Vec and Vec respectively. let mut frames: Vec = vec![]; - let mut formatted_frames: Vec = vec![]; - if let (Some(frames_v8), Some(formatted_frames_v8)) = - (frames_v8, formatted_frames_v8) - { + if let Some(frames_v8) = frames_v8 { for i in 0..frames_v8.length() { let call_site: v8::Local = frames_v8.get_index(scope, i).unwrap().try_into().unwrap(); @@ -226,7 +241,7 @@ impl JsError { .ok(); let eval_origin = eval_origin.map(|s| s.to_rust_string_lossy(scope)); let is_top_level: Option> = - get_property(scope, call_site, "isTopLevel") + get_property(scope, call_site, "isToplevel") .unwrap() .try_into() .ok(); @@ -283,21 +298,14 @@ impl JsError { is_promise_all, promise_index, }); - let formatted_frame: v8::Local = formatted_frames_v8 - .get_index(scope, i) - .unwrap() - .try_into() - .unwrap(); - let formatted_frame = formatted_frame.to_rust_string_lossy(scope); - formatted_frames.push(formatted_frame) } } - (message, frames, formatted_frames) + (message, frames) } 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![], vec![]) + (msg.get(scope).to_rust_string_lossy(scope), vec![]) }; Self { @@ -313,7 +321,6 @@ impl JsError { start_column: msg.get_start_column().try_into().ok(), end_column: msg.get_end_column().try_into().ok(), frames, - formatted_frames, } } } @@ -361,11 +368,6 @@ impl Display for JsError { } write!(f, "{}", self.message)?; - - for formatted_frame in &self.formatted_frames { - // TODO: Strip ANSI color from formatted_frame. - write!(f, "\n at {}", formatted_frame)?; - } Ok(()) } }