fix(modules): settle failed dynamic import fetches

Mark a failed fetch in the shared module map before notifying graph clients, so concurrent imports and later cached imports all reject. Route document and child owners through the same completion path.

Cover concurrent imports of the same root and a shared dependency, fresh TypeError rejections, and subsequent imports after a cached fetch failure.

moli-renderer-v8/src/module_runtime/graph.rs

@@ -831,23 +831,8 @@ impl NativeModuleGraphJob {
         request: &NativeModuleGraphFetchRequest,
         source: std::result::Result<ModuleGraphFetchedSource, ModuleLoadError>,
     ) -> std::result::Result<NativeModuleGraphJobAdvance, ModuleLoadError> {
-        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<O>(
@@ -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<ModuleGraphFetchedSource, ModuleLoadError>,
-    ) -> std::result::Result<NativeModuleGraphJobAdvance, ModuleLoadError> {
-        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<O>(
         &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<NativeModuleGraphJobAdvance, ModuleLoadError> {
-        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<O>(
         &mut self,
         owner: &mut O,

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");