[browser][coreCLR] Enable download retry by default and improve retry sequencing (#127559)

Author: pavelsavara

src/mono/wasm/Wasm.Build.Tests/ModuleConfigTests.cs

@@ -23,7 +23,7 @@ public ModuleConfigTests(ITestOutputHelper output, SharedBuildPerTestClassFixtur
 
     [Theory]
     [InlineData(false)]
-    // [InlineData(true)] // ActiveIssue: https://github.com/dotnet/runtime/issues/124946
+    [InlineData(true)]
     public async Task DownloadProgressFinishes(bool failAssemblyDownload)
     {
         Configuration config = Configuration.Debug;
@@ -50,13 +50,39 @@ public async Task DownloadProgressFinishes(bool failAssemblyDownload)
             "The download progress test did emit unexpected message about second download retry"
         );
         Assert.True(
-            result.TestOutput.Any(m => m.Contains("Throw error instead of downloading resource") == failAssemblyDownload),
+            result.TestOutput.Any(m => m.Contains("Throw error instead of downloading resource")) == failAssemblyDownload,
             failAssemblyDownload
                 ? "The download progress test didn't emit expected message about failing download"
                 : "The download progress test did emit unexpected message about failing download"
         );
     }
 
+    [Fact]
+    public async Task DownloadRetryRecoversFromFailure()
+    {
+        Configuration config = Configuration.Release;
+        ProjectInfo info = CopyTestAsset(config, false, TestAsset.WasmBasicTestApp, "ModuleConfigTests_DownloadRetryRecoversFromFailure");
+        PublishProject(info, config);
+
+        var result = await RunForPublishWithWebServer(new BrowserRunOptions(
+            Configuration: config,
+            TestScenario: "DownloadResourceProgressTest",
+            BrowserQueryString: new NameValueCollection { {"failAssemblyDownload", "true" } }
+        ));
+        Assert.True(
+            result.TestOutput.Any(m => m.Contains("DownloadResourceProgress: Finished")),
+            "Download progress didn't finish after retries"
+        );
+        Assert.True(
+            result.ConsoleOutput.Any(m => m.Contains("Retrying download")),
+            "Expected retry log message was not emitted"
+        );
+        Assert.False(
+            result.ConsoleOutput.Any(m => m.Contains("Retrying download (2)")),
+            "Second retry should not be needed since first retry succeeds"
+        );
+    }
+
     [Fact, TestCategory("bundler-friendly")]
     public async Task OutErrOverrideWorks()
     {

src/native/libs/Common/JavaScript/loader/assets.ts

@@ -17,6 +17,11 @@ let totalAssetsToDownload = 0;
 let loadBootResourceCallback: LoadBootResourceCallback | undefined = undefined;
 const loadedLazyAssemblies = new Set<string>();
 let mainWasmAsset: WasmAsset | null = null;
+const allDownloadsQueuedPCS = createPromiseCompletionSource<void>();
+
+export function resolveAllDownloadsQueued(): void {
+    allDownloadsQueuedPCS.resolve();
+}
 
 export function setLoadBootResourceCallback(callback: LoadBootResourceCallback | undefined): void {
     loadBootResourceCallback = callback;
@@ -332,7 +337,14 @@ export async function fetchNativeSymbols(asset: SymbolsAsset): Promise<void> {
 
 async function fetchBytes(asset: AssetEntryInternal): Promise<Uint8Array | null> {
     dotnetAssert.check(asset && asset.resolvedUrl, "Bad asset.resolvedUrl");
-    const response = await loadResource(asset);
+    let response: Response;
+    try {
+        response = await loadResource(asset);
+    } catch (err: any) {
+        // Strip .silent flag from download errors so they are properly reported via exit listeners
+        const message = err instanceof Error ? err.message : String(err);
+        throw new Error(`Failed to load resource '${asset.name}' from '${asset.resolvedUrl}': ${message}`, { cause: err });
+    }
     if (!response.ok) {
         if (asset.isOptional) {
             dotnetLogger.warn(`Optional resource '${asset.name}' failed to load from '${asset.resolvedUrl}'. HTTP status: ${response.status} ${response.statusText}`);
@@ -346,7 +358,14 @@ async function fetchBytes(asset: AssetEntryInternal): Promise<Uint8Array | null>
 
 async function fetchText(asset: AssetEntryInternal): Promise<string | null> {
     dotnetAssert.check(asset && asset.resolvedUrl, "Bad asset.resolvedUrl");
-    const response = await loadResource(asset);
+    let response: Response;
+    try {
+        response = await loadResource(asset);
+    } catch (err: any) {
+        // Strip .silent flag from download errors so they are properly reported via exit listeners
+        const message = err instanceof Error ? err.message : String(err);
+        throw new Error(`Failed to load resource '${asset.name}' from '${asset.resolvedUrl}': ${message}`, { cause: err });
+    }
     if (!response.ok) {
         if (asset.isOptional) {
             dotnetLogger.warn(`Optional resource '${asset.name}' failed to load from '${asset.resolvedUrl}'. HTTP status: ${response.status} ${response.statusText}`);
@@ -377,15 +396,19 @@ async function loadResourceRetry(asset: AssetEntryInternal): Promise<Response> {
     if (response.ok || asset.isOptional || noRetryStatusCodes.has(response.status)) {
         return response;
     }
+    // second attempt only after all first attempts are queued
+    await allDownloadsQueuedPCS.promise;
     if (response.status === 429) {
         // Too Many Requests
         await delay(100);
     }
+    dotnetLogger.debug(`Retrying download '${asset.name}'`);
     response = await loadResourceAttempt();
     if (response.ok || noRetryStatusCodes.has(response.status)) {
         return response;
     }
     await delay(100); // wait 100ms before the last retry
+    dotnetLogger.debug(`Retrying download (2) '${asset.name}' after delay`);
     response = await loadResourceAttempt();
     if (response.ok) {
         return response;

src/native/libs/Common/JavaScript/loader/config.ts

@@ -92,6 +92,7 @@ function defaultConfig(target: LoaderConfigInternal) {
     if (target.diagnosticTracing === undefined) target.diagnosticTracing = false;
     if (target.virtualWorkingDirectory === undefined) target.virtualWorkingDirectory = browserVirtualAppBase;
     if (target.maxParallelDownloads === undefined) target.maxParallelDownloads = 16;
+    if (target.enableDownloadRetry === undefined) target.enableDownloadRetry = true;
     normalizeConfig(target);
 }
 

src/native/libs/Common/JavaScript/loader/lib-initializers.ts

@@ -33,10 +33,9 @@ async function invokeWithErrorHandling(
     } catch (err) {
         const name = asset.name || asset.resolvedUrl || "unknown";
         const message = err instanceof Error ? err.message : String(err);
-        dotnetLogger.warn(
-            `Failed to invoke '${functionName}' on library initializer '${name}': ${message}`
-        );
-        exit(1, err);
-        throw err;
+        const wrappedError = new Error(`Failed to invoke '${functionName}' on library initializer '${name}': ${message}`, { cause: err });
+        dotnetLogger.warn(wrappedError.message);
+        exit(1, wrappedError);
+        throw wrappedError;
     }
 }

src/native/libs/Common/JavaScript/loader/run.ts

@@ -8,7 +8,7 @@ import { exit, runtimeState } from "./exit";
 import { createPromiseCompletionSource } from "./promise-completion-source";
 import { getIcuResourceName } from "./icu";
 import { loaderConfig, validateLoaderConfig } from "./config";
-import { fetchAssembly, fetchIcu, fetchNativeSymbols, fetchPdb, fetchSatelliteAssemblies, fetchVfs, fetchMainWasm, loadDotnetModule, loadJSModule, nativeModulePromiseController, verifyAllAssetsDownloaded, callLibraryInitializerOnRuntimeReady, callLibraryInitializerOnRuntimeConfigLoaded, prefetchAllResources, prefetchJSModuleLinks } from "./assets";
+import { fetchAssembly, fetchIcu, fetchNativeSymbols, fetchPdb, fetchSatelliteAssemblies, fetchVfs, fetchMainWasm, loadDotnetModule, loadJSModule, nativeModulePromiseController, verifyAllAssetsDownloaded, callLibraryInitializerOnRuntimeReady, callLibraryInitializerOnRuntimeConfigLoaded, prefetchAllResources, prefetchJSModuleLinks, resolveAllDownloadsQueued } from "./assets";
 import { initPolyfills } from "./polyfills";
 import { validateEngineFeatures } from "./bootstrap";
 
@@ -131,6 +131,11 @@ export async function createRuntime(downloadOnly: boolean, httpCacheOnly: boolea
         const isDebuggingSupported = loaderConfig.debugLevel != 0;
         const corePDBsPromise = forEachResource(resources.corePdb, fetchPdb, () => isDebuggingSupported);
         const pdbsPromise = forEachResource(resources.pdb, fetchPdb, () => isDebuggingSupported);
+
+        // Signal that all first-attempt asset fetches queued above via forEachResource/fetch* have been queued.
+        // Retry logic waits on this before attempting second downloads for those asset fetches.
+        resolveAllDownloadsQueued();
+
         // In download-only mode, just add prefetch hints for runtime-ready modules so create() loads them from cache.
         // In create mode, load them now so onRuntimeReady can be called later.
         let modulesAfterRuntimeReadyPromises: [JsAsset, Promise<any>][] = [];