diff --git a/moli-renderer-v8/src/module_runtime/graph.rs b/moli-renderer-v8/src/module_runtime/graph.rs index 915d7f86e2..94ca19de8b 100644 --- a/moli-renderer-v8/src/module_runtime/graph.rs +++ b/moli-renderer-v8/src/module_runtime/graph.rs @@ -831,23 +831,8 @@ impl NativeModuleGraphJob { request: &NativeModuleGraphFetchRequest, source: std::result::Result, ) -> std::result::Result { - let client = request.tree_client.ok_or_else(|| { - ModuleLoadError::new( - ModuleLoadStage::Fetch, - "module graph fetch request was missing its module tree client token", - ) - })?; - if let Err(error) = &source { - let key = request.pending_fetch_key().cloned().ok_or_else(|| { - ModuleLoadError::new( - ModuleLoadStage::Fetch, - "failed module graph fetch request was missing its module map key", - ) - })?; - vm.document_runtime - .mark_native_module_failed(key, error.clone()); - } - self.finish_pending_chromium_tree_fetch_for_client(vm, client, request, source) + let mut owner = NativeModuleTreeDocumentOwner::new(vm); + self.finish_pending_chromium_tree_fetch_for_request_with_owner(&mut owner, request, source) } fn finish_pending_chromium_tree_fetch_for_request_with_owner( @@ -865,28 +850,22 @@ impl NativeModuleGraphJob { "module graph fetch request was missing its module tree client token", ) })?; + if let Err(error) = &source { + let key = request.pending_fetch_key().cloned().ok_or_else(|| { + ModuleLoadError::new( + ModuleLoadStage::Fetch, + "failed module graph fetch request was missing its module map key", + ) + })?; + // Settle the shared fetch before failing this graph, so joined + // clients and later imports observe the cached fetch failure. + owner.mark_module_failed(key, error.clone()); + } self.finish_pending_chromium_tree_fetch_for_client_with_owner( owner, client, request, source, ) } - fn finish_pending_chromium_tree_fetch_for_client( - &mut self, - vm: &mut ScriptVm, - client: module_tree::SingleModuleClientToken, - request: &NativeModuleGraphFetchRequest, - source: std::result::Result, - ) -> std::result::Result { - trace_module_tree_fetch_completed_to_job(client, source.is_ok()); - let outcome = match source { - Ok(source) => module_tree::ModuleFetchOutcome::Fetched(Box::new( - chromium_fetched_source_for_request(source, request)?, - )), - Err(error) => module_tree::ModuleFetchOutcome::Failed(chromium_error(error)), - }; - self.resume_chromium_tree_fetch_outcome(vm, client, outcome) - } - fn finish_pending_chromium_tree_fetch_for_client_with_owner( &mut self, owner: &mut O, @@ -907,16 +886,6 @@ impl NativeModuleGraphJob { self.resume_chromium_tree_fetch_outcome_with_owner(owner, client, outcome) } - fn resume_chromium_tree_fetch_outcome( - &mut self, - vm: &mut ScriptVm, - client: module_tree::SingleModuleClientToken, - outcome: module_tree::ModuleFetchOutcome, - ) -> std::result::Result { - let mut owner = NativeModuleTreeDocumentOwner::new(vm); - self.resume_chromium_tree_fetch_outcome_with_owner(&mut owner, client, outcome) - } - pub(crate) fn resume_chromium_tree_fetch_outcome_with_owner( &mut self, owner: &mut O, diff --git a/moli-renderer-v8/src/script_vm/native_module.rs b/moli-renderer-v8/src/script_vm/native_module.rs index 953d86bd53..f20dd3a2ae 100644 --- a/moli-renderer-v8/src/script_vm/native_module.rs +++ b/moli-renderer-v8/src/script_vm/native_module.rs @@ -5076,8 +5076,8 @@ mod tests { DynamicModuleFetchFailure, DynamicModuleFetchOwnerAdvance, DynamicModuleImportOwner, ModuleAttributesKey, ModuleEntryId, ModuleFetchMetadata, ModuleGraphFetchedSource, ModuleGraphHandle, ModuleImportPhase, ModuleKind, ModuleLoadError, ModuleLoadStage, - ModuleMapKey, ModuleSource, NativeModuleGraphFetchRequest, NativeModuleGraphJob, - NativeModuleGraphJobAdvance, PendingDynamicModuleImport, + ModuleMapEntryState, ModuleMapKey, ModuleSource, NativeModuleGraphFetchRequest, + NativeModuleGraphJob, NativeModuleGraphJobAdvance, PendingDynamicModuleImport, }; use crate::module_script_continuation::NativeDynamicModuleTerminalFanout; use crate::network::ResourceRequestClient; @@ -5470,6 +5470,97 @@ mod tests { ); } + #[test] + fn dynamic_import_fetch_failures_reject_joined_and_cached_imports() { + for shared_dependency in [false, true] { + let mut vm = new_test_vm("https://app.example.test/page.html"); + let second_specifier = if shared_dependency { + "./second.mjs" + } else { + "./first.mjs" + }; + vm.eval(&format!( + r#" +globalThis.__fetchErrors = []; +globalThis.__failedImport = specifier => import(specifier).then( + () => __fetchErrors.push(null), + error => __fetchErrors.push(error) +); +__failedImport('./first.mjs'); +__failedImport({second_specifier:?}); +"queued" +"# + )) + .expect("both imports should enqueue their graph jobs"); + for _ in 0..2 { + assert!(matches!( + vm.run_next_native_dynamic_module_owner_action_selected_task_body(), + MainNativeModuleSelectedTaskApplication::Applied(_) + )); + } + + let (failed_load_id, failed_url) = if shared_dependency { + for (load_id, root) in [(0, "first"), (1, "second")] { + let target = vm + .current_main_dynamic_import_graph_fetch_target(load_id) + .expect("each root should have its own fetch"); + let completion = dynamic_import_completion_with_source( + load_id, + &format!("https://app.example.test/{root}.mjs"), + "import './shared.mjs';", + ); + vm.complete_current_main_dynamic_import_graph_fetch_result( + target, + completion.result, + ) + .expect("the roots should start or join the shared dependency fetch"); + } + (2, "https://app.example.test/shared.mjs") + } else { + (0, "https://app.example.test/first.mjs") + }; + assert_eq!(vm.eval("String(__fetchErrors.length)").unwrap(), "0"); + let target = vm + .current_main_dynamic_import_graph_fetch_target(failed_load_id) + .expect("the shared request should retain its fetch owner"); + vm.complete_current_main_dynamic_import_graph_fetch_result( + target, + Err("HTTP 404".to_owned()), + ) + .expect("fetch failure should reject both graph clients"); + + let key = ModuleMapKey::java_script(url::Url::parse(failed_url).unwrap()); + let entry = vm.document_runtime.native_module_entry_id(&key).unwrap(); + assert_eq!( + vm.document_runtime.native_module_entry_state(entry), + ModuleMapEntryState::Failed, + "a failed request must settle the shared module map entry" + ); + assert_eq!( + vm.eval("JSON.stringify([__fetchErrors.length, __fetchErrors.every(e => e instanceof TypeError), new Set(__fetchErrors).size])") + .unwrap(), + "[2,true,2]", + "both concurrent imports must reject with distinct TypeErrors" + ); + + vm.eval("__failedImport('./first.mjs'); 'queued'").unwrap(); + assert!(matches!( + vm.run_next_native_dynamic_module_owner_action_selected_task_body(), + MainNativeModuleSelectedTaskApplication::Applied(_) + )); + // The selected body leaves Promise reactions to its task-end checkpoint. + vm.perform_script_task_checkpoint(None) + .expect("the cached import task should dispatch its rejection reaction"); + assert_eq!( + vm.eval("JSON.stringify([__fetchErrors.length, __fetchErrors.every(e => e instanceof TypeError), new Set(__fetchErrors).size])") + .unwrap(), + "[3,true,3]", + "a cached fetch failure must reject a later import with a fresh TypeError" + ); + assert!(!vm.has_inflight_dynamic_module_fetch()); + } + } + #[test] fn module_reaction_source_consumes_exactly_one_current_target_per_turn() { let mut vm = new_test_vm("https://module-reaction-one-turn.test/page.html");