Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[browser] Allow download retry+throttling in unified code with blazor #88910

Merged
merged 20 commits into from
Jul 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions eng/testing/scenarios/BuildWasmAppsJobsList.txt
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,4 @@ Wasm.Build.Tests.WasmRunOutOfAppBundleTests
Wasm.Build.Tests.WasmSIMDTests
Wasm.Build.Tests.WasmTemplateTests
Wasm.Build.Tests.WorkloadTests
Wasm.Build.Tests.TestAppScenarios.DownloadResourceProgressTests
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,12 @@ protected async Task<RunResult> RunSdkStyleApp(RunOptions options)
if (options.BrowserQueryString != null)
queryString += "&" + string.Join("&", options.BrowserQueryString.Select(kvp => $"{kvp.Key}={kvp.Value}"));

page = await runner.RunAsync(runCommand, runArgs, onConsoleMessage: OnConsoleMessage, modifyBrowserUrl: url => url + queryString);
page = await runner.RunAsync(runCommand, runArgs, onConsoleMessage: OnConsoleMessage, modifyBrowserUrl: url =>
{
url += queryString;
_testOutput.WriteLine($"Opening browser at {url}");
return url;
});

void OnConsoleMessage(IConsoleMessage msg)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using Xunit;
using Xunit.Abstractions;

#nullable enable

namespace Wasm.Build.Tests.TestAppScenarios;

public class DownloadResourceProgressTests : AppTestBase
{
public DownloadResourceProgressTests(ITestOutputHelper output, SharedBuildPerTestClassFixture buildContext)
: base(output, buildContext)
{
}

[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task DownloadProgressFinishes(bool failAssemblyDownload)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename to indicate that this is testing retries.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(intended) Retry happens only when failAssemblyDownload=true

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, and that's what we are testing here :)

Copy link
Member Author

@maraf maraf Jul 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are testing "download progress" counter, one case is with retry and one is without

{
CopyTestAsset("WasmBasicTestApp", $"DownloadResourceProgressTests_{failAssemblyDownload}");
PublishProject("Debug");

var result = await RunSdkStyleApp(new(
Configuration: "Debug",
ForPublish: true,
TestScenario: "DownloadResourceProgressTest",
BrowserQueryString: new Dictionary<string, string> { ["failAssemblyDownload"] = failAssemblyDownload.ToString().ToLowerInvariant() }
));
Assert.True(
result.TestOutput.Any(m => m.Contains("DownloadResourceProgress: Finished")),
"The download progress test didn't emit expected error message"
);
Assert.True(
result.ConsoleOutput.Any(m => m.Contains("Retrying download")) == failAssemblyDownload,
failAssemblyDownload
? "The download progress test didn't emit expected message about retrying download"
: "The download progress test did emit unexpected message about retrying download"
);
Assert.False(
result.ConsoleOutput.Any(m => m.Contains("Retrying download (2)")),
"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),
failAssemblyDownload
? "The download progress test didn't emit expected message about failing download"
: "The download progress test did emit unexpected message about failing download"
);
}
}
2 changes: 1 addition & 1 deletion src/mono/wasm/runtime/dotnet.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -436,4 +436,4 @@ declare global {
}
declare const createDotnetRuntime: CreateDotnetRuntimeType;

export { AssetEntry, CreateDotnetRuntimeType, DotnetModuleConfig, EmscriptenModule, GlobalizationMode, IMemoryView, ModuleAPI, MonoConfig, ResourceRequest, RuntimeAPI, createDotnetRuntime as default, dotnet, exit };
export { AssetEntry, CreateDotnetRuntimeType, DotnetHostBuilder, DotnetModuleConfig, EmscriptenModule, GlobalizationMode, IMemoryView, ModuleAPI, MonoConfig, ResourceRequest, RuntimeAPI, createDotnetRuntime as default, dotnet, exit };
radical marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions src/mono/wasm/runtime/loader/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,11 +230,14 @@ export async function start_asset_download(asset: AssetEntryInternal): Promise<A
// second attempt only after all first attempts are queued
await loaderHelpers.allDownloadsQueued.promise;
try {
mono_log_debug(`Retrying download '${asset.name}'`);
maraf marked this conversation as resolved.
Show resolved Hide resolved
return await start_asset_download_with_throttle(asset);
} catch (err) {
asset.pendingDownloadInternal = undefined;
// third attempt after small delay
await delay(100);

mono_log_debug(`Retrying download (2) '${asset.name}' after delay`);
return await start_asset_download_with_throttle(asset);
}
}
Expand Down
7 changes: 3 additions & 4 deletions src/mono/wasm/runtime/loader/blazor/_Integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export async function initializeBootConfig(bootConfigResult: BootConfigResult, m
}

let resourcesLoaded = 0;
let totalResources = 0;
const totalResources = new Set<string>();

const behaviorByName = (name: string): AssetBehaviours | "other" => {
return name === "dotnet.native.wasm" ? "dotnetwasm"
Expand Down Expand Up @@ -61,13 +61,12 @@ export function setupModuleForBlazor(module: DotnetModuleInternal) {
const type = monoToBlazorAssetTypeMap[asset.behavior];
if (type !== undefined) {
const res = resourceLoader.loadResource(asset.name, asset.resolvedUrl!, asset.hash!, type);
asset.pendingDownload = res;

totalResources++;
totalResources.add(asset.name!);
res.response.then(() => {
resourcesLoaded++;
if (module.onDownloadResourceProgress)
module.onDownloadResourceProgress(resourcesLoaded, totalResources);
module.onDownloadResourceProgress(resourcesLoaded, totalResources.size);
});

return res;
Expand Down
4 changes: 2 additions & 2 deletions src/mono/wasm/runtime/types/export-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

import type { IMemoryView } from "../marshal";
import type { CreateDotnetRuntimeType, DotnetModuleConfig, RuntimeAPI, MonoConfig, ModuleAPI, AssetEntry, ResourceRequest, GlobalizationMode } from ".";
import type { CreateDotnetRuntimeType, DotnetHostBuilder, DotnetModuleConfig, RuntimeAPI, MonoConfig, ModuleAPI, AssetEntry, ResourceRequest, GlobalizationMode } from ".";
import type { EmscriptenModule } from "./emscripten";
import type { dotnet, exit } from "../loader/index";

Expand All @@ -21,6 +21,6 @@ export default createDotnetRuntime;

export {
EmscriptenModule,
RuntimeAPI, ModuleAPI, DotnetModuleConfig, CreateDotnetRuntimeType, MonoConfig, IMemoryView, AssetEntry, ResourceRequest, GlobalizationMode,
RuntimeAPI, ModuleAPI, DotnetHostBuilder, DotnetModuleConfig, CreateDotnetRuntimeType, MonoConfig, IMemoryView, AssetEntry, ResourceRequest, GlobalizationMode,
dotnet, exit
};
39 changes: 39 additions & 0 deletions src/mono/wasm/testassets/WasmBasicTestApp/wwwroot/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ if (testCase == null) {
exit(2, new Error("Missing test scenario. Supply query argument 'test'."));
}

function testOutput(msg) {
console.log(`TestOutput -> ${msg}`);
}

// Prepare base runtime parameters
dotnet
.withElementOnExit()
Expand All @@ -21,6 +25,37 @@ switch (testCase) {
case "AppSettingsTest":
dotnet.withApplicationEnvironment(params.get("applicationEnvironment"));
break;
case "DownloadResourceProgressTest":
if (params.get("failAssemblyDownload") === "true") {
let assemblyCounter = 0;
let failAtAssemblyNumbers = [
Math.floor(Math.random() * 5),
Math.floor(Math.random() * 5) + 5,
Math.floor(Math.random() * 5) + 10
maraf marked this conversation as resolved.
Show resolved Hide resolved
];
dotnet.withDiagnosticTracing(true).withResourceLoader((type, name, defaultUri, integrity) => {
if (type !== "assembly")
return defaultUri;

assemblyCounter++;
if (!failAtAssemblyNumbers.includes(assemblyCounter))
return defaultUri;

testOutput("Throw error instead of downloading resource");
maraf marked this conversation as resolved.
Show resolved Hide resolved
const error = new Error("Simulating a failed fetch");
error.silent = true;
throw error;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this get surfaced to the user at all? What behavior does blazor have when we fail to load an assembly at all?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this PR, the Blazor aborts on first download failure. Their original approach was to let the user override loadBootResource do retries by themself. After discussion with @pavelsavara we agreed that turn-on our built-in retry support.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant, is this surfaced when even a 3rd retry fails?

});
}
dotnet.withModuleConfig({
onDownloadResourceProgress: (loaded, total) => {
console.log(`DownloadResourceProgress: ${loaded} / ${total}`);
if (loaded === total && loaded !== 0) {
testOutput("DownloadResourceProgress: Finished");
}
}
});
break;
}

const { getAssemblyExports, getConfig, INTERNAL } = await dotnet.create();
Expand Down Expand Up @@ -48,9 +83,13 @@ try {
exports.AppSettingsTest.Run();
exit(0);
break;
case "DownloadResourceProgressTest":
exit(0);
break;
default:
console.error(`Unknown test case: ${testCase}`);
exit(3);
break;
}
} catch (e) {
exit(1, e);
Expand Down