diff --git a/moli-benchmark/wpt-cross-current/failed-cases.txt b/moli-benchmark/wpt-cross-current/failed-cases.txt index a1d963a1b3..cfa3f054e0 100644 --- a/moli-benchmark/wpt-cross-current/failed-cases.txt +++ b/moli-benchmark/wpt-cross-current/failed-cases.txt @@ -2978,7 +2978,6 @@ html/semantics/scripting-1/the-script-element/module/compilation-error-2.html 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/dynamic-import/dynamic-imports-script-error.html html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-nonce-classic.html html/semantics/scripting-1/the-script-element/module/dynamic-import/string-compilation-nonce-module.html html/semantics/scripting-1/the-script-element/module/error-type-3.html diff --git a/moli-benchmark/wpt-cross-current/passed-cases.txt b/moli-benchmark/wpt-cross-current/passed-cases.txt index e207ca92cb..d4597c2ca0 100644 --- a/moli-benchmark/wpt-cross-current/passed-cases.txt +++ b/moli-benchmark/wpt-cross-current/passed-cases.txt @@ -6498,6 +6498,7 @@ html/semantics/scripting-1/the-script-element/module/duplicated-imports-1.html html/semantics/scripting-1/the-script-element/module/duplicated-imports-2.html html/semantics/scripting-1/the-script-element/module/dynamic-import/code-cache-nonce.html html/semantics/scripting-1/the-script-element/module/dynamic-import/delay-load-event.html +html/semantics/scripting-1/the-script-element/module/dynamic-import/dynamic-imports-script-error.html html/semantics/scripting-1/the-script-element/module/dynamic-import/dynamic-imports.html html/semantics/scripting-1/the-script-element/module/dynamic-import/inline-event-handler.html html/semantics/scripting-1/the-script-element/module/dynamic-import/propagate-nonce-external-classic.html diff --git a/moli-module-script-tree/src/host.rs b/moli-module-script-tree/src/host.rs index 2b093a33cc..2fecd4ee98 100644 --- a/moli-module-script-tree/src/host.rs +++ b/moli-module-script-tree/src/host.rs @@ -41,4 +41,13 @@ pub trait ModuleScriptTreeHost { ) -> Result; fn mark_module_failed(&mut self, key: ModuleMapKey, error: ModuleLoadError) -> ModuleEntryId; + + /// Static requested-module validation errors belong to the module script, + /// unlike failures to resolve the specifier passed directly to import(). + /// Retain the actual exception and cache it for subsequent graph loads. + fn cache_module_request_error( + &mut self, + key: ModuleMapKey, + error: ModuleLoadError, + ) -> ModuleLoadError; } diff --git a/moli-module-script-tree/src/job.rs b/moli-module-script-tree/src/job.rs index d5c4f2a4df..a0ee5a0f70 100644 --- a/moli-module-script-tree/src/job.rs +++ b/moli-module-script-tree/src/job.rs @@ -303,7 +303,12 @@ impl ModuleScriptTreeJob { ) -> ModuleScriptTreePoll { let snapshot = match host.module_dependencies(entry) { Ok(snapshot) => snapshot, - Err(error) => return self.fail(error), + Err(error) => { + if let Some(key) = error.key.as_deref().cloned() { + return self.finish_module_load_error(host, key, error); + } + return self.fail(error); + } }; let snapshot = self.dependency_snapshot_with_tree_context(snapshot); if snapshot.requested_modules.is_empty() { @@ -318,7 +323,11 @@ impl ModuleScriptTreeJob { }; let candidates = match self.collect_dependency_candidates(host, &snapshot) { Ok(candidates) => candidates, - Err(error) => return self.fail(error), + Err(error) => { + let error = host.cache_module_request_error(snapshot.key.clone(), error); + self.record_parse_error(snapshot.key, error); + return self.finish_after_parse_error(host); + } }; for candidate in candidates { @@ -689,5 +698,6 @@ struct DependencyCandidate { } fn is_parse_error(error: &ModuleLoadError) -> bool { - error.error_constructor == Some(ModuleErrorConstructorKind::SyntaxError) + error.exception_id.is_some() + || error.error_constructor == Some(ModuleErrorConstructorKind::SyntaxError) } diff --git a/moli-module-script-tree/src/types.rs b/moli-module-script-tree/src/types.rs index f789938661..e4872f5756 100644 --- a/moli-module-script-tree/src/types.rs +++ b/moli-module-script-tree/src/types.rs @@ -520,12 +520,18 @@ pub enum ModuleSource { Binary(Vec), } +/// Identifies a JavaScript exception retained by the host in its owning realm. +/// The tree transports this token without owning or reconstructing the value. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct ModuleExceptionId(pub u64); + #[derive(Debug, Clone, PartialEq, Eq)] pub struct ModuleLoadError { pub stage: ModuleLoadStage, pub key: Option>, pub message: String, pub error_constructor: Option, + pub exception_id: Option, } impl ModuleLoadError { @@ -535,6 +541,7 @@ impl ModuleLoadError { key: None, message: message.into(), error_constructor: None, + exception_id: None, } } @@ -547,6 +554,11 @@ impl ModuleLoadError { self.error_constructor = Some(constructor); self } + + pub fn with_exception_id(mut self, exception_id: ModuleExceptionId) -> Self { + self.exception_id = Some(exception_id); + self + } } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] diff --git a/moli-module-script-tree/tests/module_tree.rs b/moli-module-script-tree/tests/module_tree.rs index ccc969d81b..acc1a0efea 100644 --- a/moli-module-script-tree/tests/module_tree.rs +++ b/moli-module-script-tree/tests/module_tree.rs @@ -85,9 +85,10 @@ impl ModuleScriptTreeHost for FakeHost { attributes: &ModuleAttributesKey, requested_phase: ModuleImportPhase, ) -> Result { - let source_url = base_url - .join(specifier) - .map_err(|error| ModuleLoadError::new(ModuleLoadStage::Resolve, error.to_string()))?; + let source_url = base_url.join(specifier).map_err(|error| { + ModuleLoadError::new(ModuleLoadStage::Resolve, error.to_string()) + .with_error_constructor(ModuleErrorConstructorKind::TypeError) + })?; let kind = if attributes .attributes .iter() @@ -246,6 +247,25 @@ impl ModuleScriptTreeHost for FakeHost { fn mark_module_failed(&mut self, _key: ModuleMapKey, _error: ModuleLoadError) -> ModuleEntryId { ModuleEntryId(0) } + + fn cache_module_request_error( + &mut self, + key: ModuleMapKey, + error: ModuleLoadError, + ) -> ModuleLoadError { + let error = error.with_key(key.clone()).with_exception_id( + moli_module_script_tree::ModuleExceptionId(self.next_entry_id as u64), + ); + self.next_entry_id += 1; + self.entries.insert( + key, + FakeEntry::Failed { + error: error.clone(), + phase: ModuleImportPhase::Evaluation, + }, + ); + error + } } fn url(input: &str) -> Url { @@ -967,6 +987,91 @@ fn parse_error_result_uses_module_discovery_order_not_completion_order() { assert_eq!(host.link_calls, 0); } +#[test] +fn static_request_error_is_cached_before_any_dependency_fetch_starts() { + let root_key = key("https://example.test/app/root.mjs"); + let mut host = FakeHost::new(); + host.ready( + root_key.clone(), + ModuleEntryId(1), + vec![ + request("./valid.mjs", ModuleImportPhase::Evaluation), + request("http://[", ModuleImportPhase::Evaluation), + ], + ); + let mut first = job(inline_root(root_key.clone(), ModuleEntryId(1))); + let ModuleScriptTreePoll::Failed(error) = first.poll(&mut host) else { + panic!("invalid static request must fail the graph"); + }; + assert!( + host.started.is_empty(), + "all requests must be validated before fetching any" + ); + assert_eq!(error.key.as_deref(), Some(&root_key)); + assert!(error.exception_id.is_some()); + assert_eq!( + error.error_constructor, + Some(ModuleErrorConstructorKind::TypeError) + ); + let mut second = job(external_root( + root_key.url.clone(), + ModuleImportPhase::Evaluation, + )); + assert_eq!( + second.drive(&mut host), + ModuleScriptTreeDrive::Failed(error) + ); + assert!(host.started.is_empty()); + assert_eq!(host.link_calls, 0); +} + +#[test] +fn static_request_type_error_waits_for_pending_siblings_and_preserves_network_failure() { + let root_key = key("https://example.test/app/root.mjs"); + let bad_key = key("https://example.test/app/bad.mjs"); + let mut host = FakeHost::new(); + host.ready( + root_key.clone(), + ModuleEntryId(1), + vec![ + request("./bad.mjs", ModuleImportPhase::Evaluation), + request("./network.mjs", ModuleImportPhase::Evaluation), + ], + ); + host.ready( + bad_key.clone(), + ModuleEntryId(2), + vec![request("http://[", ModuleImportPhase::Evaluation)], + ); + let mut tree = job(inline_root(root_key, ModuleEntryId(1))); + let ModuleScriptTreePoll::NeedFetches(mut fetches) = tree.poll(&mut host) else { + panic!("root must start its uncached sibling fetch"); + }; + assert_eq!(fetches.len(), 1); + let ModuleScriptTreeDrive::WaitingForSingleModuleClients(wait) = tree.drive(&mut host) else { + panic!("static request failure must wait for the outstanding network fetch"); + }; + assert_eq!(wait.client_count, 1); + assert!( + matches!(host.entries.get(&bad_key), Some(FakeEntry::Failed { error, .. }) + if error.exception_id.is_some()) + ); + let fetch = fetches.pop().unwrap(); + let error = ModuleLoadError::new(ModuleLoadStage::Fetch, "network failure"); + let result = tree.resume_single_module( + &mut host, + fetch.client, + ModuleFetchResult { + key: fetch.key, + client: fetch.client, + requested_phase: ModuleImportPhase::Evaluation, + outcome: ModuleFetchOutcome::Failed(error.clone()), + }, + ); + assert_eq!(result, ModuleScriptTreePoll::Failed(error)); + assert_eq!(host.link_calls, 0); +} + #[test] fn evaluation_phase_upgrades_source_phase_for_same_key() { let root_key = key("https://example.test/app/root.mjs"); diff --git a/moli-renderer-v8/src/document_module_graph/diagnostics.rs b/moli-renderer-v8/src/document_module_graph/diagnostics.rs index c5c9726c90..e7c3a8b3f8 100644 --- a/moli-renderer-v8/src/document_module_graph/diagnostics.rs +++ b/moli-renderer-v8/src/document_module_graph/diagnostics.rs @@ -1,4 +1,5 @@ use crate::types::ScriptErrorConstructorKind; +use moli_module_script_tree::ModuleExceptionId; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum ModuleLoadStage { @@ -14,6 +15,7 @@ pub(crate) struct ModuleLoadError { stage: ModuleLoadStage, message: String, error_constructor: Option, + exception_id: Option, top_level_module_load_failure: bool, } @@ -23,6 +25,7 @@ impl ModuleLoadError { stage, message: message.into(), error_constructor: None, + exception_id: None, top_level_module_load_failure: false, } } @@ -40,6 +43,15 @@ impl ModuleLoadError { self } + pub(crate) fn with_exception_id(mut self, exception_id: ModuleExceptionId) -> Self { + self.exception_id = Some(exception_id); + self + } + + pub(crate) fn exception_id(&self) -> Option { + self.exception_id + } + pub(crate) fn stage(&self) -> ModuleLoadStage { self.stage } diff --git a/moli-renderer-v8/src/module_runtime/graph.rs b/moli-renderer-v8/src/module_runtime/graph.rs index 556da443a4..85d381a522 100644 --- a/moli-renderer-v8/src/module_runtime/graph.rs +++ b/moli-renderer-v8/src/module_runtime/graph.rs @@ -1539,6 +1539,9 @@ impl module_tree::ModuleScriptTreeHost { let entry = local_entry_id(entry); let key = self.owner.module_entry_key(entry); + if let Some(error) = self.owner.module_failure(entry) { + return Err(chromium_error(error).with_key(chromium_module_key(&key))); + } let base_url = self.owner.module_entry_url(entry); let effective_fetch_metadata = self.owner.module_effective_fetch_metadata(entry); let requested_modules = self @@ -1598,6 +1601,23 @@ impl module_tree::ModuleScriptTreeHost let entry = self.owner.mark_module_failed(local_key, local_error(error)); chromium_entry_id(entry) } + + fn cache_module_request_error( + &mut self, + key: module_tree::ModuleMapKey, + error: module_tree::ModuleLoadError, + ) -> module_tree::ModuleLoadError { + let local_key = match local_module_key(&key) { + Ok(key) => key, + Err(error) => return chromium_error(error), + }; + let error = match self.owner.preserve_module_load_error(local_error(error)) { + Ok(error) => error, + Err(error) => return chromium_error(error), + }; + self.owner.mark_module_failed(local_key, error.clone()); + chromium_error(error).with_key(key) + } } fn module_map_fetch_outcome_for_key_with_owner( @@ -1899,6 +1919,9 @@ fn local_graph(graph: module_tree::ModuleGraphHandle) -> ModuleGraphHandle { fn chromium_error(error: ModuleLoadError) -> module_tree::ModuleLoadError { let mut converted = module_tree::ModuleLoadError::new(chromium_load_stage(error.stage()), error.message()); + if let Some(exception_id) = error.exception_id() { + converted = converted.with_exception_id(exception_id); + } if let Some(constructor) = error.error_constructor() { let constructor = match constructor { ScriptErrorConstructorKind::SyntaxError => { @@ -1918,6 +1941,9 @@ fn chromium_error(error: ModuleLoadError) -> module_tree::ModuleLoadError { fn local_error(error: module_tree::ModuleLoadError) -> ModuleLoadError { let mut converted = ModuleLoadError::new(local_load_stage(error.stage), error.message); + if let Some(exception_id) = error.exception_id { + converted = converted.with_exception_id(exception_id); + } if let Some(constructor) = error.error_constructor { converted = converted.with_error_constructor(local_error_constructor(constructor)); } diff --git a/moli-renderer-v8/src/module_runtime/tree_owner.rs b/moli-renderer-v8/src/module_runtime/tree_owner.rs index 742520861c..4f373c8d4e 100644 --- a/moli-renderer-v8/src/module_runtime/tree_owner.rs +++ b/moli-renderer-v8/src/module_runtime/tree_owner.rs @@ -105,6 +105,11 @@ pub(crate) trait NativeModuleTreeDocumentOwnerAdapter { fn mark_module_failed(&mut self, key: ModuleMapKey, error: ModuleLoadError) -> ModuleEntryId; + fn preserve_module_load_error( + &mut self, + error: ModuleLoadError, + ) -> std::result::Result; + fn record_runtime_warning(&mut self, message: fmt::Arguments<'_>); } @@ -239,6 +244,13 @@ impl NativeModuleTreeDocumentO (*self).mark_module_failed(key, error) } + fn preserve_module_load_error( + &mut self, + error: ModuleLoadError, + ) -> std::result::Result { + (*self).preserve_module_load_error(error) + } + fn record_runtime_warning(&mut self, message: fmt::Arguments<'_>) { (*self).record_runtime_warning(message); } @@ -279,6 +291,14 @@ impl<'a> NativeModuleTreeFrameDocumentOwner<'a> { } impl NativeModuleTreeDocumentOwnerAdapter for NativeModuleTreeDocumentOwner<'_> { + fn preserve_module_load_error( + &mut self, + error: ModuleLoadError, + ) -> std::result::Result { + self.vm + .preserve_native_module_load_error(self.compile_frame_realm, error) + } + fn compile_module_record( &mut self, key: ModuleMapKey, @@ -447,6 +467,14 @@ impl NativeModuleTreeDocumentOwnerAdapter for NativeModuleTreeDocumentOwner<'_> } impl NativeModuleTreeDocumentOwnerAdapter for NativeModuleTreeFrameDocumentOwner<'_> { + fn preserve_module_load_error( + &mut self, + error: ModuleLoadError, + ) -> std::result::Result { + self.vm + .preserve_native_module_load_error(Some(self.realm_id), error) + } + fn compile_module_record( &mut self, key: ModuleMapKey, diff --git a/moli-renderer-v8/src/script_vm/native_module.rs b/moli-renderer-v8/src/script_vm/native_module.rs index 89e49d0b4d..17000b3d7b 100644 --- a/moli-renderer-v8/src/script_vm/native_module.rs +++ b/moli-renderer-v8/src/script_vm/native_module.rs @@ -84,7 +84,9 @@ mod child_dynamic_import; mod child_parser_module; mod child_ready_document_script; mod dynamic_import_selected_task_body; +mod load_error; mod main_selected_task; +use load_error::{module_load_error_value, retain_module_exception}; pub(crate) use main_selected_task::{ MainDynamicImportGraphFetchBodySettlement, MainNativeModuleSelectedTaskApplication, MainNativeModuleSelectedTaskBodyActivity, @@ -3332,6 +3334,7 @@ impl ScriptVm { source_url: &Url, fetch_metadata: &crate::module_runtime::ModuleFetchMetadata, ) -> std::result::Result<(ModuleRecordEntry, ModuleIdentityHash), ModuleLoadError> { + let mut exception_id = None; self.renderer_document_isolate .with_entered_renderer_document_isolate(|isolate| { let scope = pin!(v8::HandleScope::new(isolate)); @@ -3352,6 +3355,12 @@ impl ScriptVm { v8::script_compiler::Source::new(source_string, Some(&origin)); let module = v8::script_compiler::compile_module(&scope, &mut compiler_source) .ok_or_else(|| { + if let Some(exception) = scope.exception() { + match retain_module_exception(&mut scope, exception) { + Ok(id) => exception_id = Some(id), + Err(error) => return error, + } + } let exception = scope .exception() .and_then(|exception| exception.to_detail_string(&scope)) @@ -3384,7 +3393,10 @@ impl ScriptVm { }) .map_err(|error| { let message = error.to_string(); - let load_error = ModuleLoadError::new(ModuleLoadStage::Compile, message.clone()); + let mut load_error = ModuleLoadError::new(ModuleLoadStage::Compile, message.clone()); + if let Some(exception_id) = exception_id { + load_error = load_error.with_exception_id(exception_id); + } if message.starts_with("v8 failed to compile WebAssembly module `") { load_error .with_error_constructor(ScriptErrorConstructorKind::WebAssemblyCompileError) @@ -4101,7 +4113,10 @@ impl ScriptVm { request: PendingDynamicModuleImport, message: &str, ) -> std::result::Result<(), ModuleLoadError> { - self.reject_native_dynamic_module_import_with_constructor(request, message, None) + self.reject_native_dynamic_module_import_and_checkpoint( + request, + &ModuleLoadError::new(ModuleLoadStage::Fetch, message), + ) } #[cfg(test)] @@ -4110,18 +4125,13 @@ impl ScriptVm { request: PendingDynamicModuleImport, error: &ModuleLoadError, ) -> std::result::Result<(), ModuleLoadError> { - self.reject_native_dynamic_module_import_with_constructor( - request, - error.message(), - error.error_constructor(), - ) + self.reject_native_dynamic_module_import_and_checkpoint(request, error) } - fn reject_native_dynamic_module_import_with_constructor( + fn reject_native_dynamic_module_import_and_checkpoint( &mut self, request: PendingDynamicModuleImport, - message: &str, - error_constructor: Option, + error: &ModuleLoadError, ) -> std::result::Result<(), ModuleLoadError> { self.renderer_document_isolate .with_entered_renderer_document_isolate(|isolate| { @@ -4130,14 +4140,7 @@ impl ScriptVm { let context = v8::Local::new(scope, request.context()); let scope = &mut v8::ContextScope::new(scope, context); let resolver = v8::Local::new(scope, request.resolver()); - let message = v8_string(scope, message); - let exception = message - .and_then(|message| { - error_constructor - .and_then(|kind| script_error_value(scope, kind, message)) - .or_else(|| Some(v8::Exception::type_error(scope, message))) - }) - .unwrap_or_else(|| v8::undefined(scope).into()); + let exception = module_load_error_value(scope, error)?; let _ = resolver.reject(scope, exception); Self::perform_microtask_checkpoints(scope, None)?; Ok(()) @@ -5066,6 +5069,7 @@ fn module_import_phase(phase: v8::ModuleImportPhase) -> ModuleImportPhase { #[cfg(test)] mod tests { + mod parse_errors; use std::pin::pin; use super::{ diff --git a/moli-renderer-v8/src/script_vm/native_module/dynamic_import_selected_task_body.rs b/moli-renderer-v8/src/script_vm/native_module/dynamic_import_selected_task_body.rs index 68ea3a8994..cd019a01d3 100644 --- a/moli-renderer-v8/src/script_vm/native_module/dynamic_import_selected_task_body.rs +++ b/moli-renderer-v8/src/script_vm/native_module/dynamic_import_selected_task_body.rs @@ -90,8 +90,6 @@ impl ScriptVm { request: PendingDynamicModuleImport, error: &ModuleLoadError, ) -> std::result::Result<(), ModuleLoadError> { - let message = error.message(); - let error_constructor = error.error_constructor(); self.renderer_document_isolate .with_entered_renderer_document_isolate(|isolate| { let scope = pin!(v8::HandleScope::new(isolate)); @@ -99,14 +97,7 @@ impl ScriptVm { let context = v8::Local::new(scope, request.context()); let scope = &mut v8::ContextScope::new(scope, context); let resolver = v8::Local::new(scope, request.resolver()); - let message = v8_string(scope, message); - let exception = message - .and_then(|message| { - error_constructor - .and_then(|kind| script_error_value(scope, kind, message)) - .or_else(|| Some(v8::Exception::type_error(scope, message))) - }) - .unwrap_or_else(|| v8::undefined(scope).into()); + let exception = module_load_error_value(scope, error)?; let _ = resolver.reject(scope, exception); Ok(()) }) diff --git a/moli-renderer-v8/src/script_vm/native_module/load_error.rs b/moli-renderer-v8/src/script_vm/native_module/load_error.rs new file mode 100644 index 0000000000..041d9636cd --- /dev/null +++ b/moli-renderer-v8/src/script_vm/native_module/load_error.rs @@ -0,0 +1,107 @@ +use std::sync::atomic::{AtomicU64, Ordering}; + +use moli_module_script_tree::ModuleExceptionId; + +use super::*; +use crate::util::private_key; + +const MODULE_EXCEPTIONS_SLOT: &str = "__moliModuleExceptions"; +static NEXT_EXCEPTION_ID: AtomicU64 = AtomicU64::new(1); + +/// Keep the original exception in the realm's GC-traced heap, not in a Rust +/// context slot containing a Global that would keep a retired realm alive. +/// The module map survives document.open(), as does this private registry. +pub(super) fn retain_module_exception( + scope: &mut v8::PinScope<'_, '_>, + exception: v8::Local<'_, v8::Value>, +) -> Result { + let global = scope.get_current_context().global(scope); + let map = match get_private_value(scope, global, MODULE_EXCEPTIONS_SLOT) + .and_then(|value| v8::Local::::try_from(value).ok()) + { + Some(map) => map, + None => { + let map = v8::Map::new(scope); + let key = private_key(scope, MODULE_EXCEPTIONS_SLOT).ok_or_else(|| { + anyhow::anyhow!("failed to allocate module exception registry key") + })?; + anyhow::ensure!( + global.set_private(scope, key, map.into()) == Some(true), + "failed to retain module exception registry" + ); + map + } + }; + let id = NEXT_EXCEPTION_ID + .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |id| id.checked_add(1)) + .map_err(|_| anyhow::anyhow!("module exception identifiers exhausted"))?; + let key = v8::BigInt::new_from_u64(scope, id); + map.set(scope, key.into(), exception) + .ok_or_else(|| anyhow::anyhow!("failed to retain module exception"))?; + Ok(ModuleExceptionId(id)) +} + +pub(super) fn module_load_error_value<'s>( + scope: &mut v8::PinScope<'s, '_>, + error: &ModuleLoadError, +) -> Result> { + if let Some(id) = error.exception_id() { + let global = scope.get_current_context().global(scope); + let map = get_private_value(scope, global, MODULE_EXCEPTIONS_SLOT) + .and_then(|value| v8::Local::::try_from(value).ok()) + .ok_or_else(|| { + anyhow::anyhow!("module exception registry missing from request realm") + })?; + let key = v8::BigInt::new_from_u64(scope, id.0); + anyhow::ensure!( + map.has(scope, key.into()) == Some(true), + "module exception {id:?} does not belong to request realm" + ); + return map + .get(scope, key.into()) + .ok_or_else(|| anyhow::anyhow!("failed to read retained module exception")); + } + let message = v8_string(scope, error.message()) + .ok_or_else(|| anyhow::anyhow!("failed to allocate module error message"))?; + script_error_value( + scope, + error + .error_constructor() + .unwrap_or(ScriptErrorConstructorKind::TypeError), + message, + ) + .ok_or_else(|| anyhow::anyhow!("failed to create module error")) +} + +impl ScriptVm { + pub(crate) fn preserve_native_module_load_error( + &mut self, + realm_id: Option, + error: ModuleLoadError, + ) -> std::result::Result { + if error.exception_id().is_some() { + return Ok(error); + } + let context_ptr = if let Some(realm_id) = realm_id { + self.frame_realm_context_ptr(realm_id).map_err(|error| { + ModuleLoadError::new(ModuleLoadStage::Resolve, error.to_string()) + })? + } else { + self.native_module_default_context_ptr() + }; + self.renderer_document_isolate + .with_entered_renderer_document_isolate(|isolate| { + let scope = pin!(v8::HandleScope::new(isolate)); + let scope = &mut scope.init(); + // SAFETY: the selected realm is owned by this VM during entry. + let context = unsafe { v8::Local::new(scope, &*context_ptr) }; + let scope = &mut v8::ContextScope::new(scope, context); + let exception = module_load_error_value(scope, &error)?; + let id = retain_module_exception(scope, exception)?; + Ok(error.with_exception_id(id)) + }) + .map_err(|error: anyhow::Error| { + ModuleLoadError::new(ModuleLoadStage::Resolve, error.to_string()) + }) + } +} 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 new file mode 100644 index 0000000000..99c738327c --- /dev/null +++ b/moli-renderer-v8/src/script_vm/native_module/tests/parse_errors.rs @@ -0,0 +1,403 @@ +use super::super::{module_load_error_value, retain_module_exception}; +use super::*; + +fn compile_parse_error(vm: &mut ScriptVm, path: &str) -> ModuleLoadError { + let url = Url::parse(path).unwrap(); + vm.compile_native_module_record( + ModuleMapKey::java_script(url.clone()), + &ModuleSource::text("export const = ;".to_owned()), + &url, + &ModuleFetchMetadata::default(), + ) + .expect_err("the module must have a syntax error") +} + +fn install_module(vm: &mut ScriptVm, path: &str, source: &str) { + let url = Url::parse(path).unwrap(); + let key = ModuleMapKey::java_script(url.clone()); + let source = ModuleSource::text(source.to_owned()); + let metadata = ModuleFetchMetadata::default(); + let (record, identity) = vm + .compile_native_module_record(key.clone(), &source, &url, &metadata) + .unwrap(); + vm.document_runtime + .insert_native_module_source(key.clone(), source); + vm.document_runtime + .insert_native_compiled_module_record_with_metadata(key, record, identity, metadata); +} + +fn graph_error(vm: &mut ScriptVm, specifier: &str) -> ModuleLoadError { + let mut job = dynamic_import_job_in_vm( + vm, + specifier, + Url::parse("https://module-errors.test/page.html").unwrap(), + ModuleImportPhase::Evaluation, + ); + job.advance_dynamic_import_owner_lane(vm) + .err() + .expect("module graph should fail") +} + +fn reject_with_error(vm: &mut ScriptVm, error: &ModuleLoadError) -> v8::Global { + let request = dynamic_import_request_in_vm( + vm, + "./bad.mjs", + Url::parse("https://module-errors.test/page.html").unwrap(), + ModuleImportPhase::Evaluation, + ); + let resolver = request.resolver().clone(); + vm.renderer_document_isolate + .with_entered_renderer_document_isolate(|isolate| { + let scope = pin!(v8::HandleScope::new(isolate)); + let scope = &mut scope.init(); + let context = v8::Local::new(scope, &vm.page_default_context); + let scope = &mut v8::ContextScope::new(scope, context); + v8::Local::new(scope, &resolver) + .get_promise(scope) + .mark_as_handled(); + Ok(()) + }) + .unwrap(); + vm.reject_native_dynamic_module_import_with_error_selected_task_body(request, error) + .unwrap(); + vm.renderer_document_isolate + .with_entered_renderer_document_isolate(|isolate| { + let scope = pin!(v8::HandleScope::new(isolate)); + let scope = &mut scope.init(); + let context = v8::Local::new(scope, &vm.page_default_context); + let scope = &mut v8::ContextScope::new(scope, context); + let promise = v8::Local::new(scope, &resolver).get_promise(scope); + assert_eq!(promise.state(), v8::PromiseState::Rejected); + Ok(v8::Global::new(scope, promise.result(scope))) + }) + .unwrap() +} + +#[test] +fn module_parse_error_preserves_exception_identity_without_merging_equal_messages() { + let mut vm = new_test_vm("https://module-errors.test/page.html"); + let error = compile_parse_error(&mut vm, "https://module-errors.test/first.mjs"); + let other_error = compile_parse_error(&mut vm, "https://module-errors.test/second.mjs"); + let first = reject_with_error(&mut vm, &error); + let again = reject_with_error(&mut vm, &error.clone()); + let other = reject_with_error(&mut vm, &other_error); + vm.with_default_context_scope_and_checkpoint_for_test(|scope, _| { + let first = v8::Local::new(scope, &first); + let again = v8::Local::new(scope, &again); + let other = v8::Local::new(scope, &other); + assert!(first.is_native_error()); + assert!( + first.strict_equals(again), + "cloned load errors must retain the JS exception" + ); + assert!( + !first.strict_equals(other), + "separate modules must retain separate exceptions" + ); + Ok(()) + }) + .unwrap(); +} + +#[test] +fn module_parse_error_identity_survives_document_open_in_the_same_realm() { + let mut vm = new_test_vm("https://module-errors.test/page.html"); + let error = compile_parse_error(&mut vm, "https://module-errors.test/bad.mjs"); + let first = reject_with_error(&mut vm, &error); + vm.eval("document.open(); document.write('

replacement

'); document.close(); 'done'") + .unwrap(); + let again = reject_with_error(&mut vm, &error); + vm.with_default_context_scope_and_checkpoint_for_test(|scope, _| { + let first = v8::Local::new(scope, &first); + let again = v8::Local::new(scope, &again); + assert!( + first.strict_equals(again), + "document.open must retain the realm's module errors" + ); + Ok(()) + }) + .unwrap(); +} + +#[test] +fn module_parse_error_static_resolution_is_shared_across_roots_but_not_modules() { + let mut vm = new_test_vm("https://module-errors.test/page.html"); + for (name, source) in [ + ("bad.mjs", "import 'unmapped';"), + ("other-bad.mjs", "import 'unmapped';"), + ("first.mjs", "import './bad.mjs';"), + ("second.mjs", "import './bad.mjs';"), + ] { + install_module( + &mut vm, + &format!("https://module-errors.test/{name}"), + source, + ); + } + let first = graph_error(&mut vm, "./first.mjs"); + let again = graph_error(&mut vm, "./first.mjs"); + let second = graph_error(&mut vm, "./second.mjs"); + let dependency = graph_error(&mut vm, "./bad.mjs"); + let other = graph_error(&mut vm, "./other-bad.mjs"); + assert!(first.exception_id().is_some()); + assert_eq!( + first.error_constructor(), + Some(ScriptErrorConstructorKind::TypeError) + ); + assert_eq!(first.exception_id(), again.exception_id()); + assert_eq!(first.exception_id(), second.exception_id()); + assert_eq!(first.exception_id(), dependency.exception_id()); + assert_ne!(first.exception_id(), other.exception_id()); + let first = reject_with_error(&mut vm, &first); + let second = reject_with_error(&mut vm, &second); + vm.with_default_context_scope_and_checkpoint_for_test(|scope, _| { + assert!(v8::Local::new(scope, &first).strict_equals(v8::Local::new(scope, &second))); + Ok(()) + }) + .unwrap(); +} + +#[test] +fn module_parse_error_cache_does_not_capture_direct_resolution_fetch_or_link_errors() { + let mut vm = new_test_vm("https://module-errors.test/page.html"); + let direct = graph_error(&mut vm, "unmapped"); + for error in [ + direct, + ModuleLoadError::new(ModuleLoadStage::Fetch, "network failure"), + ModuleLoadError::new(ModuleLoadStage::Instantiate, "missing export") + .with_error_constructor(ScriptErrorConstructorKind::SyntaxError), + ] { + assert!(error.exception_id().is_none()); + let first = reject_with_error(&mut vm, &error); + let second = reject_with_error(&mut vm, &error); + vm.with_default_context_scope_and_checkpoint_for_test(|scope, _| { + assert!(!v8::Local::new(scope, &first).strict_equals(v8::Local::new(scope, &second))); + Ok(()) + }) + .unwrap(); + } +} + +#[test] +fn module_parse_error_registry_bypasses_public_constructors_and_collection_methods() { + let mut vm = new_test_vm("https://module-errors.test/page.html"); + vm.eval( + r#" + globalThis.__intrinsicSyntaxError = SyntaxError; + const forbidden = () => { throw new Error('public hook must not run'); }; + Map.prototype.set = Map.prototype.get = Map.prototype.has = forbidden; + globalThis.Map = globalThis.SyntaxError = globalThis.TypeError = forbidden; + 'installed' + "#, + ) + .unwrap(); + let error = compile_parse_error(&mut vm, "https://module-errors.test/bad.mjs"); + let first = reject_with_error(&mut vm, &error); + let second = reject_with_error(&mut vm, &error); + vm.with_default_context_scope_and_checkpoint_for_test(|scope, _| { + let first = v8::Local::new(scope, &first); + assert!(first.strict_equals(v8::Local::new(scope, &second))); + let global = scope.get_current_context().global(scope); + assert_eq!( + global.set(scope, v8str(scope, "__originalException").into(), first), + Some(true) + ); + Ok(()) + }) + .unwrap(); + assert_eq!( + vm.eval("__originalException instanceof __intrinsicSyntaxError") + .unwrap(), + "true" + ); +} + +#[test] +fn module_parse_error_registry_is_realm_local_and_does_not_root_retired_realms() { + ensure_v8(); + let mut isolate = v8::Isolate::new(Default::default()); + let (weak_context, weak_exception, error) = { + let scope = pin!(v8::HandleScope::new(&mut isolate)); + let scope = &mut scope.init(); + let context = v8::Context::new(scope, Default::default()); + let scope = &mut v8::ContextScope::new(scope, context); + let message = v8str(scope, "bad module"); + let exception = v8::Exception::syntax_error(scope, message); + let id = retain_module_exception(scope, exception).unwrap(); + let error = + ModuleLoadError::new(ModuleLoadStage::Compile, "bad module").with_exception_id(id); + assert!( + module_load_error_value(scope, &error) + .unwrap() + .strict_equals(exception) + ); + let weak_context = v8::Weak::new(scope, context); + let weak_exception = v8::Weak::new(scope, exception); + let other_context = v8::Context::new(scope, Default::default()); + let scope = &mut v8::ContextScope::new(scope, other_context); + assert!(module_load_error_value(scope, &error).is_err()); + let other = v8::Exception::syntax_error(scope, message); + let other_id = retain_module_exception(scope, other).unwrap(); + assert_ne!(id, other_id); + assert!(module_load_error_value(scope, &error).is_err()); + (weak_context, weak_exception, error) + }; + isolate.low_memory_notification(); + assert!( + weak_context.is_empty(), + "retained module errors must not root the realm" + ); + assert!( + weak_exception.is_empty(), + "retiring the realm must release its exceptions" + ); + assert!( + error.exception_id().is_some(), + "the portable token may outlive its realm" + ); +} + +#[tokio::test] +async fn module_parse_error_compilation_and_resolution_use_the_child_realm() { + let mut vm = new_test_vm("https://module-errors.test/page.html"); + vm.eval( + r#" + const root = document.appendChild(document.createElement('html')); + const body = root.appendChild(document.createElement('body')); + globalThis.frame = document.createElement('iframe'); + frame.srcdoc = '