JSModuleLoader: a top-level load stores its failure into the entry it loaded

Source/JavaScriptCore/runtime/JSMicrotask.cpp

@@ -1177,6 +1177,10 @@ static void moduleLoadTopSettled(JSGlobalObject* globalObject, VM& vm, ThrowScop
         }
 #if USE(BUN_JSC_ADDITIONS)
         }
+        // The entry the key holds now, registered by provideFetch() just above or
+        // by provideModule() before the load, is the one the loadModule below
+        // loads; moduleLoadTopRejected stores into it.
+        context->setEntry(vm, globalObject->moduleLoader()->ensureRegistered(globalObject, specifier, type));
 #endif
 
         JSPromise* statePromise = JSPromise::create(vm, globalObject->promiseStructure());
@@ -1259,11 +1263,19 @@ static void moduleLoadTopRejected(JSGlobalObject* globalObject, VM& vm, std::spa
     if (status == JSPromise::Status::Fulfilled)
         resultPromise->fulfill(vm, arguments[1]);
     else {
+#if USE(BUN_JSC_ADDITIONS)
+        // The entry this load loaded (ModuleLoadingContext::setEntry). Null when
+        // the load failed at its fetch: moduleLoadTopSettled registers nothing for
+        // that, or has stored the error itself, so there is nothing left to store.
+        ModuleRegistryEntry* entry = context->entry();
+#else
         const Identifier& specifier = context->moduleRequest().m_specifier;
         auto type = context->moduleRequest().type();
         // https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-single-module-script step 13.1
         // Only set an error if the entry already exists.
-        if (ModuleRegistryEntry* entry = globalObject->moduleLoader()->getRegisteredMayBeNull(specifier, type))
+        ModuleRegistryEntry* entry = globalObject->moduleLoader()->getRegisteredMayBeNull(specifier, type);
+#endif
+        if (entry)
             entry->setEvaluationError(globalObject, arguments[1]);
         resultPromise->reject(vm, arguments[1]);
     }
@@ -1416,9 +1428,15 @@ static void moduleLoadStoreError(JSGlobalObject* globalObject, std::span<const J
         JSValue errorValue = arguments[1];
         const Identifier& specifier = context->moduleRequest().m_specifier;
         auto type = context->moduleRequest().type();
+#if USE(BUN_JSC_ADDITIONS)
+        // The entry this load loaded (ModuleLoadingContext::setEntry), recorded by
+        // the loadModule overload that created this context.
+        ModuleRegistryEntry* entry = context->entry();
+#else
         // https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-single-module-script step 13.1
         // Only set an error if the entry already exists.
         ModuleRegistryEntry* entry = globalObject->moduleLoader()->getRegisteredMayBeNull(specifier, type);
+#endif
         if (!entry)
             return;
         if (auto* error = dynamicDowncast<ErrorInstance>(errorValue)) {

Source/JavaScriptCore/runtime/JSModuleLoader.cpp

@@ -844,10 +844,22 @@ JSPromise* JSModuleLoader::loadModule(JSGlobalObject* globalObject, const Module
     VM& vm = globalObject->vm();
     auto scope = DECLARE_THROW_SCOPE(vm);
 
+#if USE(BUN_JSC_ADDITIONS)
+    // Every caller has just registered the key (provideFetch()). That entry is the
+    // one this load links and evaluates, and the one moduleLoadStoreError stores
+    // its failure into. Read before hostLoadImportedModule(), whose resolve() hook
+    // runs host code, so that it is the entry moduleLoadTopSettled recorded too.
+    ModuleRegistryEntry* entry = getRegisteredMayBeNull(moduleRequest.m_specifier, moduleRequest.type());
+#endif
+
     JSPromise* promise = hostLoadImportedModule(globalObject, referrer, moduleRequest, payload, scriptFetcher, flags.contains(ModuleLoadFlag::UseImportMap));
     RETURN_IF_EXCEPTION(scope, nullptr);
 
     auto* context = ModuleLoadingContext::create(vm, moduleRequest, WTF::move(scriptFetcher), flags);
+#if USE(BUN_JSC_ADDITIONS)
+    if (entry)
+        context->setEntry(vm, entry);
+#endif
     JSPromise* resultPromise = JSPromise::create(vm, globalObject->promiseStructure());
     resultPromise->markAsHandled();
 

Source/JavaScriptCore/runtime/JSModuleLoader.h

@@ -199,8 +199,10 @@ class JSModuleLoader final : public JSCell {
         // delete every (specifier, type) variant — text/json/HostDefined etc.
         //
         // A load of the removed entry may still be in flight. Its steps keep
-        // the detached entry, and isCacheableLoadedModule() keeps the record
-        // they produce out of m_loadedModules. Callers hold cellLock().
+        // the detached entry (ModuleLoadingContext::setEntry for the top-level
+        // ones) and store its outcome there, and isCacheableLoadedModule()
+        // keeps the record they produce out of m_loadedModules. Callers hold
+        // cellLock().
         auto* impl = key.impl();
         m_loadedModules.removeIf([&](auto& entry) { return entry.key.first == impl; });
         m_resolutionFailures.removeIf([&](auto& entry) { return entry.key.first == impl || entry.key.second == impl; });

Source/JavaScriptCore/runtime/ModuleLoadingContext.cpp

@@ -84,6 +84,13 @@ ModuleLoadingContext* ModuleLoadingContext::create(VM& vm, const AbstractModuleR
     return context;
 }
 
+#if USE(BUN_JSC_ADDITIONS)
+void ModuleLoadingContext::setEntry(VM& vm, ModuleRegistryEntry* entry)
+{
+    m_entry.set(vm, this, entry);
+}
+#endif
+
 JSModuleLoader::ModuleReferrer ModuleLoadingContext::referrer() const
 {
     JSValue ref = m_referrer.get();

Source/JavaScriptCore/runtime/ModuleLoadingContext.h

@@ -70,6 +70,17 @@ class ModuleLoadingContext final : public JSCell {
     const AbstractModuleRecord::ModuleRequest& moduleRequest() const { return m_moduleRequest; }
     JSCell* payload() const { return m_payload.get(); }
     ModuleRegistryEntry* entry() const { return m_entry.get(); }
+#if USE(BUN_JSC_ADDITIONS)
+    // A top-level load (second create() overload) has no entry until its fetch
+    // has settled and moduleLoadTopSettled has registered the key. The loader
+    // then records the entry the load goes on to load, and moduleLoadTopRejected
+    // and moduleLoadStoreError store the load's failure into that entry instead
+    // of into whatever entry the key holds by then. The two differ once the host
+    // has removed the entry while the load was in flight (removeEntry()): the key
+    // may by then hold the entry of a replacement load, which must not inherit
+    // the removed load's error.
+    void setEntry(VM&, ModuleRegistryEntry*);
+#endif
     ScriptFetcher* scriptFetcher() const { return m_scriptFetcher.get(); }
     AbstractModuleRecord* module() const { return m_module.get(); }
     void module(VM& vm, AbstractModuleRecord* mod) { m_module.set(vm, this, mod); }