diff --git a/moli-benchmark/wpt-cross-current/failed-cases.txt b/moli-benchmark/wpt-cross-current/failed-cases.txt index 38476813b..bb6632938 100644 --- a/moli-benchmark/wpt-cross-current/failed-cases.txt +++ b/moli-benchmark/wpt-cross-current/failed-cases.txt @@ -2957,7 +2957,6 @@ html/semantics/scripting-1/the-script-element/microtasks/checkpoint-after-worker html/semantics/scripting-1/the-script-element/module/crossorigin.html html/semantics/scripting-1/the-script-element/module/dynamic-import/alpha/base-url-worker-importScripts.html html/semantics/scripting-1/the-script-element/module/dynamic-import/code-cache-base-url.html -html/semantics/scripting-1/the-script-element/module/evaluation-error-5.html html/semantics/scripting-1/the-script-element/module/inline-async-execorder.html html/semantics/scripting-1/the-script-element/moving-between-documents/ordering/delay-load-event-1.html html/semantics/scripting-1/the-script-element/moving-between-documents/ordering/delay-load-event-2.html diff --git a/moli-benchmark/wpt-cross-current/passed-cases.txt b/moli-benchmark/wpt-cross-current/passed-cases.txt index 01605fd4b..cffd0c0e3 100644 --- a/moli-benchmark/wpt-cross-current/passed-cases.txt +++ b/moli-benchmark/wpt-cross-current/passed-cases.txt @@ -7018,6 +7018,7 @@ html/semantics/scripting-1/the-script-element/module/evaluation-error-1.html html/semantics/scripting-1/the-script-element/module/evaluation-error-2.html html/semantics/scripting-1/the-script-element/module/evaluation-error-3.html html/semantics/scripting-1/the-script-element/module/evaluation-error-4.html +html/semantics/scripting-1/the-script-element/module/evaluation-error-5.html html/semantics/scripting-1/the-script-element/module/execorder.html html/semantics/scripting-1/the-script-element/module/fetch-error-1.html html/semantics/scripting-1/the-script-element/module/fetch-error-2.html diff --git a/moli-module-script-tree/src/types.rs b/moli-module-script-tree/src/types.rs index 372f650a4..92c8ed8f0 100644 --- a/moli-module-script-tree/src/types.rs +++ b/moli-module-script-tree/src/types.rs @@ -514,9 +514,22 @@ impl FetchedModuleSource { } } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ModuleSourceOrigin { + /// Diagnostic source identity, independent of the module resolution base. + pub url: Url, + /// Zero-based offsets into the containing source document. + pub line_offset: u32, + pub column_offset: u32, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum ModuleSource { Text(String), + TextWithOrigin { + source: String, + origin: Box, + }, Binary(Vec), } diff --git a/moli-renderer-v8/src/document_module_graph.rs b/moli-renderer-v8/src/document_module_graph.rs index d95b617b0..1507dd7bd 100644 --- a/moli-renderer-v8/src/document_module_graph.rs +++ b/moli-renderer-v8/src/document_module_graph.rs @@ -25,7 +25,7 @@ pub(crate) use key::{ pub(crate) use map::DocumentModuleMapCore; pub(crate) use record::{ ModuleGraphFetchedSource, ModuleImportPhase, ModuleRequestRecord, ModuleResolvedDependency, - ModuleSource, + ModuleSource, ModuleSourceOrigin, }; pub(crate) use terminal::{ ModuleMapFetchClient, ModuleMapTerminalClients, ModuleMapTerminalNotification, diff --git a/moli-renderer-v8/src/document_module_graph/record.rs b/moli-renderer-v8/src/document_module_graph/record.rs index c3232c71c..652d302c8 100644 --- a/moli-renderer-v8/src/document_module_graph/record.rs +++ b/moli-renderer-v8/src/document_module_graph/record.rs @@ -1,3 +1,4 @@ +pub(crate) use moli_module_script_tree::ModuleSourceOrigin; use url::Url; use super::{ModuleAttributesKey, ModuleKind, ModuleMapKey}; @@ -5,6 +6,10 @@ use super::{ModuleAttributesKey, ModuleKind, ModuleMapKey}; #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) enum ModuleSource { Text(String), + TextWithOrigin { + source: String, + origin: Box, + }, Binary(Vec), } @@ -86,20 +91,34 @@ impl ModuleSource { Self::Text(source) } + pub(crate) fn text_with_origin(source: String, origin: ModuleSourceOrigin) -> Self { + Self::TextWithOrigin { + source, + origin: Box::new(origin), + } + } + + pub(crate) fn origin(&self) -> Option<&ModuleSourceOrigin> { + match self { + Self::TextWithOrigin { origin, .. } => Some(origin), + Self::Text(_) | Self::Binary(_) => None, + } + } + pub(crate) fn binary(bytes: Vec) -> Self { Self::Binary(bytes) } pub(crate) fn text_source(&self) -> Option<&str> { match self { - Self::Text(source) => Some(source), + Self::Text(source) | Self::TextWithOrigin { source, .. } => Some(source), Self::Binary(_) => None, } } pub(crate) fn binary_source(&self) -> Option<&[u8]> { match self { - Self::Text(_) => None, + Self::Text(_) | Self::TextWithOrigin { .. } => None, Self::Binary(bytes) => Some(bytes), } } @@ -107,7 +126,7 @@ impl ModuleSource { #[cfg(test)] pub(crate) fn len(&self) -> usize { match self { - Self::Text(source) => source.len(), + Self::Text(source) | Self::TextWithOrigin { source, .. } => source.len(), Self::Binary(bytes) => bytes.len(), } } diff --git a/moli-renderer-v8/src/module_runtime/graph.rs b/moli-renderer-v8/src/module_runtime/graph.rs index 6b25b567f..76a4660ae 100644 --- a/moli-renderer-v8/src/module_runtime/graph.rs +++ b/moli-renderer-v8/src/module_runtime/graph.rs @@ -1736,6 +1736,9 @@ fn local_entry_id(entry: module_tree::ModuleEntryId) -> ModuleEntryId { fn chromium_source(source: ModuleSource) -> module_tree::ModuleSource { match source { ModuleSource::Text(source) => module_tree::ModuleSource::Text(source), + ModuleSource::TextWithOrigin { source, origin } => { + module_tree::ModuleSource::TextWithOrigin { source, origin } + } ModuleSource::Binary(bytes) => module_tree::ModuleSource::Binary(bytes), } } @@ -1814,6 +1817,9 @@ fn chromium_fetched_source_for_request( fn local_source(source: module_tree::ModuleSource) -> ModuleSource { match source { module_tree::ModuleSource::Text(source) => ModuleSource::Text(source), + module_tree::ModuleSource::TextWithOrigin { source, origin } => { + ModuleSource::TextWithOrigin { source, origin } + } module_tree::ModuleSource::Binary(bytes) => ModuleSource::Binary(bytes), } } @@ -2688,6 +2694,23 @@ mod tests { Url::parse(raw).expect("test url should parse") } + #[test] + fn module_source_origin_survives_the_tree_adapter() { + let source = ModuleSource::text_with_origin( + "export const value = 1;".to_owned(), + crate::document_module_graph::ModuleSourceOrigin { + url: url("https://example.test/source.html"), + line_offset: 17, + column_offset: 23, + }, + ); + let roundtrip = local_source(chromium_source(source.clone())); + assert_eq!(roundtrip, source); + assert_eq!(roundtrip.text_source(), Some("export const value = 1;")); + assert_eq!(roundtrip.origin().unwrap().line_offset, 17); + assert!(roundtrip.binary_source().is_none()); + } + fn new_test_vm(url: &str) -> StandaloneScriptVmHarness { let _js_runtime = crate::JsRuntime::initialize(); let page_task_queue = crate::page_task_queue::PageTaskQueueTestHarness::new(); @@ -2704,6 +2727,52 @@ mod tests { .expect("script vm finish should succeed") } + #[test] + fn module_source_origin_keeps_each_import_meta_base_when_first_accessed_later() { + let document_url = url("https://example.test/source.html"); + let mut vm = new_test_vm(document_url.as_str()); + vm.eval("globalThis.__moduleMetaReaders = [];").unwrap(); + for directory in ["first", "second"] { + let base_url = url(&format!("https://example.test/{directory}/")); + let source = ModuleSource::text_with_origin( + "__moduleMetaReaders.push(() => import.meta);".to_owned(), + crate::document_module_graph::ModuleSourceOrigin { + url: document_url.clone(), + line_offset: 7, + column_offset: 0, + }, + ); + let mut job = parser_owned_loaded_module_script_graph_job( + &mut vm, + source, + &base_url, + &document_url, + &ScriptFetchMetadata::default(), + false, + ) + .unwrap(); + let NativeModuleGraphJobAdvance::Complete(graph) = + job.advance_module_script_owner_lane(&mut vm).unwrap() + else { + panic!("an import-free inline module must compile without fetching"); + }; + vm.instantiate_native_module_graph(&graph).unwrap(); + vm.evaluate_native_module_graph(graph.root_entry).unwrap(); + } + assert_eq!( + vm.eval( + r#"JSON.stringify([1, 0].map(index => { + const meta = __moduleMetaReaders[index](); + const originalURL = meta.url; + meta.url = 'https://author-replacement.test/'; + return [originalURL, meta.resolve('./dependency.mjs')]; + }))"#, + ) + .unwrap(), + r#"[["https://example.test/second/","https://example.test/second/dependency.mjs"],["https://example.test/first/","https://example.test/first/dependency.mjs"]]"# + ); + } + #[test] fn native_json_module_fetch_request_uses_json_destination_metadata() { let fetch_request = NativeModuleGraphFetchRequest::new_for_test( diff --git a/moli-renderer-v8/src/module_runtime/host_callbacks.rs b/moli-renderer-v8/src/module_runtime/host_callbacks.rs index 7e9a4f83a..1937eb026 100644 --- a/moli-renderer-v8/src/module_runtime/host_callbacks.rs +++ b/moli-renderer-v8/src/module_runtime/host_callbacks.rs @@ -35,7 +35,15 @@ pub(crate) unsafe extern "C" fn initialize_import_meta_object_callback( meta: v8::Local<'_, v8::Object>, ) { v8::callback_scope!(unsafe scope, context); - let Ok(module_url) = v8::Local::::try_from(module.get_resource_name(scope)) else { + // V8 calls this while the module's import.meta expression is running. + // Its host-defined options retain the preparation-time resolution base; + // the resource name instead identifies the source document for diagnostics. + let module_url = scope + .get_current_host_defined_options() + .and_then(|options| script_base_url_from_host_defined_options(scope, options)) + .and_then(|url| v8_string(scope, url.as_str())) + .or_else(|| v8::Local::::try_from(module.get_resource_name(scope)).ok()); + let Some(module_url) = module_url else { return; }; let _ = ImportMetaDeclaration::new(module_url).initialize(scope, meta); diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_documents/commit.rs b/moli-renderer-v8/src/native_bridge/context_host/child_documents/commit.rs index 677a959ec..b170b4f5e 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_documents/commit.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_documents/commit.rs @@ -202,7 +202,7 @@ impl JsContextHost { }; let document_url = self.document_url_for_handle(document_handle); let document_base_url = self.document_base_url_for_handle(document_handle); - let parser_base_url = document_base_url.clone(); + let parser_document_url = document_url.clone(); let referrer_policy = self .child_browsing_context_referrer_policy_for_document_handle(document_handle) .map(str::to_owned); @@ -236,7 +236,7 @@ impl JsContextHost { document_handle, owner_local_window_id, owner_document_id, - parser_base_url, + parser_document_url, source.as_ref(), is_xml_document, ); diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_documents/live_parser.rs b/moli-renderer-v8/src/native_bridge/context_host/child_documents/live_parser.rs index 7aa0bc64f..b3f139b0e 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_documents/live_parser.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_documents/live_parser.rs @@ -1714,20 +1714,17 @@ impl JsContextHost { document_handle: DomHandle, owner_local_window_id: LocalWindowId, owner_document_id: DocumentId, - document_base_url: Url, + document_url: Url, markup: &str, is_xml_document: bool, ) -> ChildLiveDocumentParserStartResult { let owner = FrameDocumentOwner::new(owner_local_window_id, owner_document_id); self.child_document_parsers.clear(owner); let mut parser = if is_xml_document { - DocumentParserSession::start_finite_live_xml_document( - document_base_url, - document_handle, - ) + DocumentParserSession::start_finite_live_xml_document(document_url, document_handle) } else { DocumentParserSession::start_finite_live_document( - document_base_url, + document_url, document_handle, self.child_browsing_context_scripting_enabled(child_handle), ) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/child_module_error_reporting.rs b/moli-renderer-v8/src/runtime/page_vm/tests/child_module_error_reporting.rs index b5ac645ba..bdbb356e0 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/child_module_error_reporting.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/child_module_error_reporting.rs @@ -57,6 +57,7 @@ fn install_child_error_observers( page_vm.vm_mut().eval(&format!( r#" globalThis.__childErrors = []; + globalThis.__childErrorLocations = []; globalThis.__childOnerrors = []; globalThis.__childErrorOrder = []; globalThis.__parentErrors = 0; @@ -70,6 +71,7 @@ fn install_child_error_observers( const expectedReason = child.__reason; child.addEventListener('unhandledrejection', event => {{ ++__unhandled; event.preventDefault(); }}); child.addEventListener('error', event => {{ + __childErrorLocations.push([event.filename, event.lineno, event.colno]); __childErrors.push([ event.error, event instanceof child.ErrorEvent, event.target === child, event.isTrusted, @@ -91,6 +93,42 @@ fn install_child_error_observers( Ok(()) } +#[tokio::test(flavor = "current_thread")] +async fn child_module_error_reporting_preserves_inline_source_location() { + run_page_vm_async_test(async { + let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).unwrap(); + let (mut page_vm, _resource, _wake) = page_vm_with_bound_task_sources_and_owner_wake( + &loader, + Url::parse("https://example.com/child-module-location.html").unwrap(), + ); + queue_inline_child_error(&mut page_vm, &loader, "\n\nmissingModuleReference", "null") + .await?; + assert!( + page_vm + .run_exact_selected_page_task_for_test( + PageSelectedTaskTestSelector::ChildDocumentScriptReady, + &loader, + ) + .await? + ); + assert_eq!( + page_vm + .vm_mut() + .eval_without_microtask_checkpoint_for_test( + r#" + JSON.stringify([__childErrorLocations.length, + __childErrorLocations[0]?.[0] === child.document.URL, + __childErrorLocations[0]?.slice(1), __parentErrors, __scriptErrors, __scriptLoads]) + "# + )?, + "[1,true,[3,1],0,0,0]" + ); + Ok::<_, anyhow::Error>(()) + }) + .await + .unwrap(); +} + #[tokio::test(flavor = "current_thread")] async fn child_module_error_reporting_reports_root_parse_error_to_its_window() { run_page_vm_async_test(async { diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/module_error_reporting.rs b/moli-renderer-v8/src/runtime/page_vm/tests/module_error_reporting.rs index 49bdaddbf..97d0861fc 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/module_error_reporting.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/module_error_reporting.rs @@ -17,6 +17,75 @@ fn bound_parser_module(page_vm: &mut PageVm, position: u32, url: Url) -> Prepare script } +#[tokio::test(flavor = "current_thread")] +async fn parser_module_error_reporting_preserves_inline_source_origin() { + run_page_vm_async_test(async { + for (source, expected_line, expected_column) in [ + ("missingModuleReference", 17, 23), + ("\nmissingModuleReference", 18, 1), + ] { + let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).unwrap(); + let document_url = Url::parse("https://example.com/inline-module-location.html").unwrap(); + let mut page_vm = test_page_vm_with_loader_and_document_url(&loader, Vec::new(), document_url.clone()); + page_vm.vm_mut().eval(r#" + globalThis.__locations = []; + globalThis.__onerrorLocations = []; + addEventListener('error', event => { + __locations.push([event.filename, event.lineno, event.colno, event.error instanceof ReferenceError]); + event.preventDefault(); + }); + onerror = (message, filename, line, column, error) => { + __onerrorLocations.push([filename, line, column, error instanceof ReferenceError]); + return true; + }; + "#).unwrap(); + let mut script = bound_parser_module(&mut page_vm, 9201, document_url.clone()); + script.source_kind = ScriptSourceKind::Inline; + script.source = crate::planning::ScriptSource::Inline(source.to_owned()); + script.base_url = Url::parse("https://example.com/import-base/").unwrap(); + page_vm.vm_mut().document_runtime.note_parser_script_start_position(script.node_id, 17, 23); + let work = install_parser_module_defer_work(&mut page_vm, script); + page_vm.execute_post_parse_page_owned_task_on_named_owner_lane(&loader, work).await.unwrap(); + run_parser_module_completion_turns_for_test(&mut page_vm, &loader, 0, "inline module location").await; + let expected = format!(r#"[["{document_url}",{expected_line},{expected_column},true]]"#); + assert_eq!(page_vm.vm_mut().eval("JSON.stringify(__locations)").unwrap(), expected); + assert_eq!(page_vm.vm_mut().eval("JSON.stringify(__onerrorLocations)").unwrap(), expected); + } + }).await; +} + +#[tokio::test(flavor = "current_thread")] +async fn parser_module_error_reporting_preserves_deferred_source_location() { + run_page_vm_async_test(async { + let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).unwrap(); + let mut page_vm = test_page_vm_with_loader_and_document_url( + &loader, Vec::new(), Url::parse("https://example.com/tla-location.html").unwrap(), + ); + page_vm.vm_mut().eval(r#" + globalThis.__locations = []; + addEventListener('error', event => { + __locations.push([event.filename, event.lineno, event.colno, + event.error === __locatedOriginal]); + event.preventDefault(); + }); + "#).unwrap(); + let url = Url::parse("https://example.com/located-tla.mjs").unwrap(); + let script = bound_parser_module(&mut page_vm, 9202, url.clone()); + let work = install_parser_module_defer_work(&mut page_vm, script); + page_vm.execute_post_parse_page_owned_task_on_named_owner_lane(&loader, work).await.unwrap(); + let source = "\nglobalThis.__locatedOriginal = Object.freeze(new TypeError('original'));\nawait new Promise((_, reject) => { globalThis.__rejectLocatedModule = () => reject(__locatedOriginal); });"; + enqueue_parser_owned_module_script_fetch_completion_for_test(&mut page_vm, 0, &url, source); + assert!(run_next_main_module_fetch_terminal_for_test(&mut page_vm).unwrap().is_some()); + run_ready_parser_deferred_body_for_test(&mut page_vm, &loader, "located TLA module").await; + assert_eq!(page_vm.vm_mut().eval("__locations.length").unwrap(), "0"); + page_vm.vm_mut().eval("__rejectLocatedModule(); 'rejected'").unwrap(); + run_parser_module_completion_turns_for_test(&mut page_vm, &loader, 1, "located TLA module").await; + let column = source.lines().nth(1).unwrap().find("new TypeError").unwrap() + 1; + assert_eq!(page_vm.vm_mut().eval("JSON.stringify(__locations)").unwrap(), + format!(r#"[["{url}",2,{column},true]]"#)); + }).await; +} + async fn assert_parser_module_reports_original_value(value: &str, reject_later: bool) { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); diff --git a/moli-renderer-v8/src/script_vm/child_module_script_terminal.rs b/moli-renderer-v8/src/script_vm/child_module_script_terminal.rs index a8bcaea97..179b970f4 100644 --- a/moli-renderer-v8/src/script_vm/child_module_script_terminal.rs +++ b/moli-renderer-v8/src/script_vm/child_module_script_terminal.rs @@ -55,6 +55,14 @@ impl<'vm> ChildModuleScriptTerminalOwner<'vm> { ) -> FrameDocumentModuleScriptTerminalFollowup { self.vm .ensure_child_document_modulator_for_graph_start(task_owner.document_owner(), realm_id); + let source = if !client.source_is_external() + && let Some(text) = source.text_source() + { + self.vm + .inline_module_script_source_with_origin(client.script(), text.to_owned()) + } else { + source + }; let source_url = if client.source_is_external() { client.script().url.clone() } else { diff --git a/moli-renderer-v8/src/script_vm/native_module.rs b/moli-renderer-v8/src/script_vm/native_module.rs index 3579eb197..16071eeee 100644 --- a/moli-renderer-v8/src/script_vm/native_module.rs +++ b/moli-renderer-v8/src/script_vm/native_module.rs @@ -981,7 +981,29 @@ impl ScriptVm { let source = self .inline_script_element_source_for_execution(script.node_id, source, request) .unwrap_or_default(); - ModuleSource::text(source) + self.inline_module_script_source_with_origin(script, source) + } + + pub(crate) fn inline_module_script_source_with_origin( + &self, + script: &PreparedScript, + source: String, + ) -> ModuleSource { + let position = self + .document_runtime + .parser_script_start_position(script.node_id); + ModuleSource::text_with_origin( + source, + crate::document_module_graph::ModuleSourceOrigin { + url: script.url.clone(), + line_offset: position.map_or(0, |position| { + position.line.saturating_sub(1).min(i32::MAX as u64) as u32 + }), + column_offset: position.map_or(0, |position| { + position.column.saturating_sub(1).min(i32::MAX as u64) as u32 + }), + }, + ) } pub(crate) fn seal_main_parser_deferred_scripts( @@ -3283,6 +3305,7 @@ impl ScriptVm { ) -> std::result::Result<(ModuleRecordEntry, ModuleIdentityHash), ModuleLoadError> { match key.kind() { ModuleKind::JavaScript => { + let origin = source.origin(); let Some(source) = source.text_source() else { return Err(ModuleLoadError::new( ModuleLoadStage::Compile, @@ -3295,6 +3318,7 @@ impl ScriptVm { source, source_url, fetch_metadata, + origin, ) } ModuleKind::Json | ModuleKind::Css => { @@ -3334,6 +3358,7 @@ impl ScriptVm { source: &str, source_url: &Url, fetch_metadata: &crate::module_runtime::ModuleFetchMetadata, + source_origin: Option<&crate::document_module_graph::ModuleSourceOrigin>, ) -> std::result::Result<(ModuleRecordEntry, ModuleIdentityHash), ModuleLoadError> { let mut exception_id = None; self.renderer_document_isolate @@ -3351,6 +3376,7 @@ impl ScriptVm { &mut scope, source_url.as_str(), fetch_metadata, + source_origin, ); let mut compiler_source = v8::script_compiler::Source::new(source_string, Some(&origin)); @@ -4993,8 +5019,15 @@ fn create_module_script_origin<'s>( scope: &mut v8::PinScope<'s, '_>, resource_name: &str, fetch_metadata: &crate::module_runtime::ModuleFetchMetadata, + source_origin: Option<&crate::document_module_graph::ModuleSourceOrigin>, ) -> v8::ScriptOrigin<'s> { - let name = v8::String::new(scope, resource_name).expect("v8 string allocation"); + let name = v8::String::new( + scope, + source_origin.map_or(resource_name, |origin| origin.url.as_str()), + ) + .expect("v8 string allocation"); + // Inline diagnostics identify the source document, while imports continue + // to resolve against the module's preparation-time base URL. let base_url = Url::parse(resource_name).ok(); let host_defined_options = base_url.as_ref().and_then(|base_url| { crate::util::script_host_defined_options_with_fetch_metadata( @@ -5007,8 +5040,8 @@ fn create_module_script_origin<'s>( v8::ScriptOrigin::new( scope, name.into(), - 0, - 0, + source_origin.map_or(0, |origin| origin.line_offset.min(i32::MAX as u32) as i32), + source_origin.map_or(0, |origin| origin.column_offset.min(i32::MAX as u32) as i32), false, -1, None, diff --git a/moli-renderer-v8/src/script_vm/native_module/tests/parse_errors.rs b/moli-renderer-v8/src/script_vm/native_module/tests/parse_errors.rs index 99c738327..157cbac90 100644 --- a/moli-renderer-v8/src/script_vm/native_module/tests/parse_errors.rs +++ b/moli-renderer-v8/src/script_vm/native_module/tests/parse_errors.rs @@ -12,6 +12,102 @@ fn compile_parse_error(vm: &mut ScriptVm, path: &str) -> ModuleLoadError { .expect_err("the module must have a syntax error") } +#[test] +fn module_parse_error_reporting_uses_retained_location_without_author_getters() { + let mut vm = new_test_vm("https://module-errors.test/page.html"); + let url = Url::parse("https://module-errors.test/dependency.mjs").unwrap(); + let error = vm + .compile_native_module_record( + ModuleMapKey::java_script(url.clone()), + &ModuleSource::text("\n\nexport const value = ;".to_owned()), + &url, + &ModuleFetchMetadata::default(), + ) + .expect_err("the dependency must have a syntax error"); + vm.with_default_context_scope(|scope, _| { + let exception = module_load_error_value(scope, &error)?; + let global = scope.get_current_context().global(scope); + assert_eq!( + global.set(scope, v8str(scope, "__original").into(), exception), + Some(true) + ); + Ok(()) + }) + .unwrap(); + vm.eval( + r#" + globalThis.__locationReads = 0; + for (const name of ['fileName', 'lineNumber', 'columnNumber', 'stack']) { + Object.defineProperty(__original, name, { get() { + ++__locationReads; + throw new Error('must not read author location'); + }}); + } + Object.freeze(__original); + addEventListener('error', event => { + globalThis.__location = [event.filename, event.lineno, event.colno, + event.error === __original, __locationReads]; + event.preventDefault(); + }); + "#, + ) + .unwrap(); + vm.report_window_error_body( + error.message(), + Some("https://module-errors.test/root.mjs"), + error.error_value(), + ) + .unwrap(); + assert_eq!( + vm.eval("JSON.stringify(__location)").unwrap(), + r#"["https://module-errors.test/dependency.mjs",3,22,true,0]"# + ); +} + +#[test] +fn module_parse_error_compile_origin_preserves_line_and_first_line_column_offsets() { + for (source, line, column) in [ + ("export const value = ;", 21, 33), + ("\n\nexport const value = ;", 23, 22), + ] { + let mut vm = new_test_vm("https://module-errors.test/page.html"); + let base = Url::parse("https://module-errors.test/import-base/").unwrap(); + let source = ModuleSource::text_with_origin( + source.to_owned(), + crate::document_module_graph::ModuleSourceOrigin { + url: Url::parse("https://module-errors.test/source-document.html").unwrap(), + line_offset: 20, + column_offset: 11, + }, + ); + let error = vm + .compile_native_module_record( + ModuleMapKey::java_script(base.clone()), + &source, + &base, + &ModuleFetchMetadata::default(), + ) + .expect_err("inline source must have a syntax error"); + vm.with_default_context_scope(|scope, _| { + let exception = module_load_error_value(scope, &error)?; + let message = v8::Exception::create_message(scope, exception); + assert_eq!( + message + .get_script_resource_name(scope) + .unwrap() + .to_string(scope) + .unwrap() + .to_rust_string_lossy(scope), + "https://module-errors.test/source-document.html" + ); + assert_eq!(message.get_line_number(scope), Some(line)); + assert_eq!(message.get_start_column() + 1, column); + Ok(()) + }) + .unwrap(); + } +} + fn install_module(vm: &mut ScriptVm, path: &str, source: &str) { let url = Url::parse(path).unwrap(); let key = ModuleMapKey::java_script(url.clone()); diff --git a/moli-renderer-v8/src/script_vm/script_event_body.rs b/moli-renderer-v8/src/script_vm/script_event_body.rs index b7c917206..fc33b7303 100644 --- a/moli-renderer-v8/src/script_vm/script_event_body.rs +++ b/moli-renderer-v8/src/script_vm/script_event_body.rs @@ -118,6 +118,26 @@ fn dispatch_script_failure_error_body( } None => window_script_failure_error_value(scope, global, None, message_value), }; + // Retained V8 exceptions already carry engine-owned source information. + // Read that metadata, not author-visible stack or location properties, and + // do not replace a dependency's URL with the root script's fallback URL. + let location = retained.then(|| { + let exception_message = v8::Exception::create_message(scope, error_value); + let filename = exception_message + .get_script_resource_name(scope) + .and_then(|value| v8::Local::::try_from(value).ok()) + .map(|value| value.to_rust_string_lossy(scope)); + let line = exception_message + .get_line_number(scope) + .and_then(|line| u32::try_from(line).ok()) + .unwrap_or(0); + let column = exception_message + .get_start_column() + .checked_add(1) + .and_then(|column| u32::try_from(column).ok()) + .unwrap_or(0); + (filename, line, column) + }); // Location metadata belongs to the ErrorEvent. Never mutate // the original exception (or invoke an author's setter). if !retained @@ -137,9 +157,13 @@ fn dispatch_script_failure_error_body( scope, host_ptr, message, - filename.unwrap_or(""), - 0, - 0, + location + .as_ref() + .and_then(|(filename, _, _)| filename.as_deref()) + .or(filename) + .unwrap_or(""), + location.as_ref().map_or(0, |(_, line, _)| *line), + location.as_ref().map_or(0, |(_, _, column)| *column), Some(error_value), ) .map_err(anyhow::Error::msg) diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/trusted_types.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/trusted_types.rs index e37ade81b..def78bff7 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/trusted_types.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/trusted_types.rs @@ -1320,13 +1320,18 @@ fn inline_module_graph_roots_use_trusted_types_compliant_source() { }; let html_module = prepared(html_script, 1); let svg_module = prepared(svg_script, 2); + let source_origin = crate::document_module_graph::ModuleSourceOrigin { + url: document_url, + line_offset: 0, + column_offset: 0, + }; assert_eq!( vm.inline_module_script_source_for_graph_start( &html_module, "postMessage('blocked', '*');" ), - crate::module_runtime::ModuleSource::text(String::new()), + crate::module_runtime::ModuleSource::text_with_origin(String::new(), source_origin.clone(),), "a module blocked by Trusted Types should enter the graph as an inert root" ); @@ -1343,8 +1348,10 @@ trustedTypes.createPolicy("default", { ) .expect("inline-module default policy should install"); - let expected = - crate::module_runtime::ModuleSource::text("postMessage('transformed', '*');".to_owned()); + let expected = crate::module_runtime::ModuleSource::text_with_origin( + "postMessage('transformed', '*');".to_owned(), + source_origin, + ); assert_eq!( vm.inline_module_script_source_for_graph_start( &html_module,