diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 1d70a9cb74..b5bb04fdd9 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -4352,6 +4352,12 @@ itest!(node_prefix_missing { exit_code: 1, }); +itest!(dynamic_import_syntax_error { + args: "run -A run/dynamic_import_syntax_error.js", + output: "run/dynamic_import_syntax_error.js.out", + exit_code: 1, +}); + itest!(extension_import { args: "run run/extension_import.ts", output: "run/extension_import.ts.out", diff --git a/cli/tests/testdata/run/dynamic_import_syntax_error.js b/cli/tests/testdata/run/dynamic_import_syntax_error.js new file mode 100644 index 0000000000..be8ec132f9 --- /dev/null +++ b/cli/tests/testdata/run/dynamic_import_syntax_error.js @@ -0,0 +1 @@ +await import("./dynamic_import_syntax_error_import.js"); diff --git a/cli/tests/testdata/run/dynamic_import_syntax_error.js.out b/cli/tests/testdata/run/dynamic_import_syntax_error.js.out new file mode 100644 index 0000000000..91e69eac70 --- /dev/null +++ b/cli/tests/testdata/run/dynamic_import_syntax_error.js.out @@ -0,0 +1,4 @@ +error: Uncaught (in promise) SyntaxError: Unexpected reserved word at [WILDCARD]/dynamic_import_syntax_error_import.js:3:3 +await import("./dynamic_import_syntax_error_import.js"); +^ + at async [WILDCARD]/dynamic_import_syntax_error.js:1:1 diff --git a/cli/tests/testdata/run/dynamic_import_syntax_error_import.js b/cli/tests/testdata/run/dynamic_import_syntax_error_import.js new file mode 100644 index 0000000000..bcf0756385 --- /dev/null +++ b/cli/tests/testdata/run/dynamic_import_syntax_error_import.js @@ -0,0 +1,5 @@ +// deno-lint-ignore-file +function foo() { + await Promise.resolve(); +} +foo(); diff --git a/core/bindings.rs b/core/bindings.rs index 8e701c9034..5650b78f36 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -7,6 +7,7 @@ use log::debug; use v8::MapFnTo; use crate::error::is_instance_of_error; +use crate::error::JsStackFrame; use crate::modules::get_asserted_module_type_from_assertions; use crate::modules::parse_import_assertions; use crate::modules::resolve_helper; @@ -436,26 +437,26 @@ fn catch_dynamic_import_promise_error( if is_instance_of_error(scope, arg) { let e: crate::error::NativeJsError = serde_v8::from_v8(scope, arg).unwrap(); let name = e.name.unwrap_or_else(|| "Error".to_string()); - let message = v8::Exception::create_message(scope, arg); - if message.get_stack_trace(scope).unwrap().get_frame_count() == 0 { + let msg = v8::Exception::create_message(scope, arg); + if msg.get_stack_trace(scope).unwrap().get_frame_count() == 0 { let arg: v8::Local = arg.try_into().unwrap(); let message_key = v8::String::new_external_onebyte_static(scope, b"message").unwrap(); let message = arg.get(scope, message_key.into()).unwrap(); + let mut message: v8::Local = message.try_into().unwrap(); + if let Some(stack_frame) = JsStackFrame::from_v8_message(scope, msg) { + if let Some(location) = stack_frame.maybe_format_location() { + let str = + format!("{} at {location}", message.to_rust_string_lossy(scope)); + message = v8::String::new(scope, &str).unwrap(); + } + } let exception = match name.as_str() { - "RangeError" => { - v8::Exception::range_error(scope, message.try_into().unwrap()) - } - "TypeError" => { - v8::Exception::type_error(scope, message.try_into().unwrap()) - } - "SyntaxError" => { - v8::Exception::syntax_error(scope, message.try_into().unwrap()) - } - "ReferenceError" => { - v8::Exception::reference_error(scope, message.try_into().unwrap()) - } - _ => v8::Exception::error(scope, message.try_into().unwrap()), + "RangeError" => v8::Exception::range_error(scope, message), + "TypeError" => v8::Exception::type_error(scope, message), + "SyntaxError" => v8::Exception::syntax_error(scope, message), + "ReferenceError" => v8::Exception::reference_error(scope, message), + _ => v8::Exception::error(scope, message), }; let code_key = v8::String::new_external_onebyte_static(scope, b"code").unwrap(); diff --git a/core/error.rs b/core/error.rs index b35544bb1e..64898678ae 100644 --- a/core/error.rs +++ b/core/error.rs @@ -195,6 +195,44 @@ impl JsStackFrame { promise_index: None, } } + + /// Gets the source mapped stack frame corresponding to the + /// (script_resource_name, line_number, column_number) from a v8 message. + /// For non-syntax errors, it should also correspond to the first stack frame. + pub fn from_v8_message<'a>( + scope: &'a mut v8::HandleScope, + message: v8::Local<'a, v8::Message>, + ) -> Option { + let f = message.get_script_resource_name(scope)?; + let f: v8::Local = f.try_into().ok()?; + let f = f.to_rust_string_lossy(scope); + let l = message.get_line_number(scope)? as i64; + // V8's column numbers are 0-based, we want 1-based. + let c = message.get_start_column() as i64 + 1; + let state_rc = JsRuntime::state(scope); + let state = &mut *state_rc.borrow_mut(); + if let Some(source_map_getter) = &state.source_map_getter { + let (f, l, c) = apply_source_map( + f, + l, + c, + &mut state.source_map_cache, + source_map_getter.as_ref(), + ); + Some(JsStackFrame::from_location(Some(f), Some(l), Some(c))) + } else { + Some(JsStackFrame::from_location(Some(f), Some(l), Some(c))) + } + } + + pub fn maybe_format_location(&self) -> Option { + Some(format!( + "{}:{}:{}", + self.file_name.as_ref()?, + self.line_number?, + self.column_number? + )) + } } fn get_property<'a>( @@ -231,42 +269,18 @@ impl JsError { let scope = &mut v8::HandleScope::new(scope); let exception_message = msg.get(scope).to_rust_string_lossy(scope); - let state_rc = JsRuntime::state(scope); // Convert them into Vec let mut frames: Vec = vec![]; - let mut source_line = None; let mut source_line_frame_index = None; + + if let Some(stack_frame) = JsStackFrame::from_v8_message(scope, msg) { + frames = vec![stack_frame]; + } { + let state_rc = JsRuntime::state(scope); let state = &mut *state_rc.borrow_mut(); - - let script_resource_name = msg - .get_script_resource_name(scope) - .and_then(|v| v8::Local::::try_from(v).ok()) - .map(|v| v.to_rust_string_lossy(scope)); - let line_number: Option = - msg.get_line_number(scope).and_then(|v| v.try_into().ok()); - let column_number: Option = msg.get_start_column().try_into().ok(); - if let (Some(f), Some(l), Some(c)) = - (script_resource_name, line_number, column_number) - { - // V8's column numbers are 0-based, we want 1-based. - let c = c + 1; - if let Some(source_map_getter) = &state.source_map_getter { - let (f, l, c) = apply_source_map( - f, - l, - c, - &mut state.source_map_cache, - source_map_getter.as_ref(), - ); - frames = vec![JsStackFrame::from_location(Some(f), Some(l), Some(c))]; - } else { - frames = vec![JsStackFrame::from_location(Some(f), Some(l), Some(c))]; - } - } - if let Some(source_map_getter) = &state.source_map_getter { for (i, frame) in frames.iter().enumerate() { if let (Some(file_name), Some(line_number)) = @@ -377,48 +391,21 @@ impl JsError { Some(frames_v8) => serde_v8::from_v8(scope, frames_v8.into()).unwrap(), None => vec![], }; - let mut source_line = None; let mut source_line_frame_index = None; + + // 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 frames.is_empty() { + if let Some(stack_frame) = JsStackFrame::from_v8_message(scope, msg) { + frames = vec![stack_frame]; + } + } { let state_rc = JsRuntime::state(scope); let state = &mut *state_rc.borrow_mut(); - - // 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 frames.is_empty() { - let script_resource_name = msg - .get_script_resource_name(scope) - .and_then(|v| v8::Local::::try_from(v).ok()) - .map(|v| v.to_rust_string_lossy(scope)); - let line_number: Option = - msg.get_line_number(scope).and_then(|v| v.try_into().ok()); - let column_number: Option = - msg.get_start_column().try_into().ok(); - if let (Some(f), Some(l), Some(c)) = - (script_resource_name, line_number, column_number) - { - // V8's column numbers are 0-based, we want 1-based. - let c = c + 1; - if let Some(source_map_getter) = &state.source_map_getter { - let (f, l, c) = apply_source_map( - f, - l, - c, - &mut state.source_map_cache, - source_map_getter.as_ref(), - ); - frames = - vec![JsStackFrame::from_location(Some(f), Some(l), Some(c))]; - } else { - frames = - vec![JsStackFrame::from_location(Some(f), Some(l), Some(c))]; - } - } - } - if let Some(source_map_getter) = &state.source_map_getter { for (i, frame) in frames.iter().enumerate() { if let (Some(file_name), Some(line_number)) = @@ -502,16 +489,6 @@ impl JsError { impl std::error::Error for JsError {} -fn format_source_loc( - file_name: &str, - line_number: i64, - column_number: i64, -) -> String { - let line_number = line_number; - let column_number = column_number; - format!("{file_name}:{line_number}:{column_number}") -} - impl Display for JsError { fn fmt(&self, f: &mut Formatter) -> fmt::Result { if let Some(stack) = &self.stack { @@ -521,14 +498,9 @@ impl Display for JsError { } } write!(f, "{}", self.exception_message)?; - let frame = self.frames.first(); - if let Some(frame) = frame { - if let (Some(f_), Some(l), Some(c)) = - (&frame.file_name, frame.line_number, frame.column_number) - { - let source_loc = format_source_loc(f_, l, c); - write!(f, "\n at {source_loc}")?; - } + let location = self.frames.first().and_then(|f| f.maybe_format_location()); + if let Some(location) = location { + write!(f, "\n at {location}")?; } Ok(()) }