diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bc8f8e8..18d61f7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,6 +39,12 @@ jobs: - name: Test run: dotnet test SignsOfAI.slnx -c Release --no-build --nologo + # The web app's startup path is JavaScript, so no .NET test can execute it. These run the real + # boot.js against a fake network: a 503 that recovers, one that does not, a connection that + # hangs, and the integrity hash it must keep handing to fetch. Node's own runner, no packages. + - name: Test boot script + run: node --test tests/boot/boot.test.mjs + # The desktop app is WPF, so it needs a Windows runner and it is not in SignsOfAI.slnx — see the # comment at the top of that file. Its own job keeps the build above on Linux, where it stays fast # for the translation PRs that are the reason this workflow exists. diff --git a/src/SignsOfAI.Web/wwwroot/index.html b/src/SignsOfAI.Web/wwwroot/index.html index f86eaac..164284a 100644 --- a/src/SignsOfAI.Web/wwwroot/index.html +++ b/src/SignsOfAI.Web/wwwroot/index.html @@ -37,8 +37,140 @@ - + + + + + diff --git a/src/SignsOfAI.Web/wwwroot/js/boot.js b/src/SignsOfAI.Web/wwwroot/js/boot.js index 5f3209c..0dec91e 100644 --- a/src/SignsOfAI.Web/wwwroot/js/boot.js +++ b/src/SignsOfAI.Web/wwwroot/js/boot.js @@ -1,129 +1,97 @@ // Starting the app, out loud when it fails. // -// The runtime arrives as ~50 separate files, each one pinned by a SHA-256 in the boot manifest. +// The runtime arrives as ~54 separate files, each one pinned by a SHA-256 in the boot manifest. // GitHub Pages answers one of them with a 503 now and then. The browser hashes whatever body did // arrive — an error page, not the assembly — the integrity check fails, and Blazor gives up for -// good. What the visitor sees is the loading circle, forever, with no message: the failure happens +// good. What the visitor saw was the loading circle, forever, with no message: the failure happens // before Blazor has an error UI to show. That is how this reached us, as "the app is down", when // every file on the server was intact. // -// So: retry the download before believing it, and if it still will not come, say so. +// So: retry the download before believing it. The panel that speaks when the retry does not help +// lives inline in index.html, because it also has to survive this file never arriving. (function () { const ATTEMPTS = 3; const BACKOFF_MS = [400, 1200]; - // The .NET runtime's own scripts are ES modules — they have to be handed back as a URL for the - // import to work, so a Response would break them. Those keep the default loader. + // A fetch has no timeout of its own. A server that accepts the connection and then never + // answers would otherwise consume no attempt, reach no panel, and hand back the same eternal + // spinner this file exists to kill — the failure mode measured and missed on the first pass. + const ATTEMPT_TIMEOUT_MS = 25000; + + // The .NET runtime's own scripts are ES modules: the loader asserts they come back as a URL, so + // a Response would break the import outright. Those keep the default loader — and therefore get + // no retry, which is one of the holes the inline watchdog covers. const NOT_OURS = 'dotnetjs'; - let lastFailure = null; + const boot = window.signsofaiBoot; async function fetchWithRetry(url, integrity) { + // Local to this download, never shared. A module-level "last failure" would let a file that + // failed once and then recovered be named as the cause of somebody else's error later. + let failure = null; + for (let attempt = 0; attempt < ATTEMPTS; attempt++) { if (attempt > 0) { await new Promise(done => setTimeout(done, BACKOFF_MS[attempt - 1] ?? 1200)); } + + const abort = new AbortController(); + const timer = setTimeout(() => abort.abort(), ATTEMPT_TIMEOUT_MS); try { const response = await fetch(url, { // Handing fetch the manifest's own hash keeps the guarantee we would otherwise - // be dropping: returning a Response from loadBootResource takes the integrity - // check away from Blazor, so the browser has to do it here instead. + // be dropping. Returning a Response takes the check away from Blazor entirely: + // the runtime short-circuits on a returned promise before it sets integrity of + // its own, so this line is not hygiene, it is the whole verification. integrity: integrity || undefined, // A first try may legitimately come from the browser cache. A retry may not — // if what we hold is a truncated or stale body, asking for it again is useless. cache: attempt === 0 ? 'default' : 'reload', + signal: abort.signal, }); - if (response.ok) return response; - lastFailure = { url, reason: `HTTP ${response.status}` }; + if (response.ok) { + if (attempt > 0) { + // The only trace a rescued startup leaves. Without it, a recovered blip is + // indistinguishable from a clean load, and the next person to ask "was the + // site flaky?" has nothing to look at. + console.info(`[signsofai] ${url} recovered after ${attempt + 1} attempts`); + } + boot.progress(); + return response; + } + failure = { url, reason: `HTTP ${response.status}` }; } catch (error) { // An integrity mismatch rejects here too, which is the case we are actually chasing: - // a 503 body hashes to something else, and retrying is exactly the right answer. - lastFailure = { url, reason: String(error && error.message || error) }; + // a 503 body hashes to something else. So does an abort. Both are worth retrying. + failure = { url, reason: String(error && error.message || error) }; + } finally { + clearTimeout(timer); } } + // Blazor does not reject Blazor.start() when a boot file will not come — the failure - // surfaces as an unhandled rejection deep in mono_download_assets, which is why the - // spinner used to spin for good. We are the ones who know the download is out of tries, - // so the explanation is written from here rather than from a .catch that never runs. - reportFailure(); - throw new Error(`${lastFailure.reason} for ${lastFailure.url}`); + // surfaces as an unhandled rejection inside mono_download_assets, which is why the spinner + // used to spin for good. We are the ones who know this download is out of tries, so the + // explanation is asked for from here rather than from a .catch that never runs. + boot.fail(`${failure.reason}\n${failure.url}`); + throw new Error(`${failure.reason} for ${failure.url}`); } - // The app has not started, so Blazor cannot render this and its own error UI is not wired yet. - // It also styles itself: app.css is a separate request, and a page that is explaining a failed - // download should not depend on one more download having worked. The custom properties are used - // where they exist and fall back to their light-theme values where they do not. - let reported = false; - - function reportFailure(error) { - // Several assets can fail in the same run; the first explanation is the one that stands. - if (reported) return; - reported = true; - - const spanish = (navigator.language || '').toLowerCase().startsWith('es'); - const copy = spanish ? { - title: 'No se pudo terminar de cargar', - body: 'Un archivo de la aplicación no llegó. Casi siempre es un fallo pasajero del ' + - 'servidor, no un problema de tu texto ni de tu navegador. Nada de lo que ' + - 'escribiste salió de tu equipo — la aplicación ni siquiera llegó a arrancar.', - retry: 'Reintentar', - details: 'Detalle técnico', - } : { - title: "Couldn't finish loading", - body: 'One of the application files did not arrive. This is almost always a passing ' + - 'server fault, not a problem with your text or your browser. Nothing you typed ' + - 'left your machine — the application never got as far as starting.', - retry: 'Try again', - details: 'Technical detail', - }; - - const app = document.getElementById('app'); - if (!app) return; - - app.innerHTML = ''; - const panel = document.createElement('div'); - panel.setAttribute('role', 'alert'); - panel.style.cssText = - 'max-width:34rem;margin:12vh auto;padding:1.5rem;border-radius:14px;' + - 'border:1px solid var(--border,#e2e6ea);background:var(--surface,#fff);' + - 'color:var(--text,#1a1d21);font:1rem/1.5 system-ui,-apple-system,Segoe UI,sans-serif;'; - - const title = document.createElement('h1'); - title.textContent = copy.title; - title.style.cssText = 'margin:0 0 .6rem;font-size:1.25rem;'; - - const body = document.createElement('p'); - body.textContent = copy.body; - body.style.cssText = 'margin:0 0 1.2rem;color:var(--text-muted,#5b6470);'; - - const retry = document.createElement('button'); - retry.type = 'button'; - retry.textContent = copy.retry; - retry.style.cssText = - 'padding:.55rem 1.1rem;border:0;border-radius:8px;cursor:pointer;font:inherit;' + - 'background:var(--brand,#2563eb);color:var(--brand-ink,#fff);'; - retry.addEventListener('click', () => window.location.reload()); - - const details = document.createElement('details'); - details.style.cssText = 'margin-top:1.2rem;font-size:.82rem;color:var(--text-muted,#5b6470);'; - const summary = document.createElement('summary'); - summary.textContent = copy.details; - summary.style.cssText = 'cursor:pointer;'; - const pre = document.createElement('pre'); - // textContent, not innerHTML: the URL comes back from the network and is never markup here. - pre.textContent = lastFailure - ? `${lastFailure.reason}\n${lastFailure.url}` - : String(error && error.message || error); - pre.style.cssText = 'white-space:pre-wrap;word-break:break-all;margin:.5rem 0 0;'; - details.append(summary, pre); - - panel.append(title, body, retry, details); - app.append(panel); + // A 503 on the Blazor script leaves this file running with nothing to call. Saying so now beats + // waiting for the watchdog: we already know it is never going to start. + if (typeof Blazor === 'undefined') { + boot.fail('Blazor.start is unavailable — _framework/blazor.webassembly.js did not load.'); + return; } Blazor.start({ loadBootResource(type, name, defaultUri, integrity) { return type === NOT_OURS ? undefined : fetchWithRetry(defaultUri, integrity); }, - }).catch(reportFailure); + }).then( + () => boot.ok(), + // Reached by startup failures that are not downloads at all. Those carry their own error, + // and it is the one to show — the retry's own failures have already spoken for themselves. + error => boot.fail(String(error && error.message || error)) + ); })(); diff --git a/tests/SignsOfAI.Core.Tests/WebBootTests.cs b/tests/SignsOfAI.Core.Tests/WebBootTests.cs index ab5f1eb..054ac12 100644 --- a/tests/SignsOfAI.Core.Tests/WebBootTests.cs +++ b/tests/SignsOfAI.Core.Tests/WebBootTests.cs @@ -1,3 +1,4 @@ +using System.Text.RegularExpressions; using Xunit; namespace SignsOfAI.Core.Tests; @@ -11,18 +12,21 @@ namespace SignsOfAI.Core.Tests; /// the visitor gets the loading circle and nothing else. That is a healthy deployment reading as an /// outage, and it is how it was first reported to us. /// -/// boot.js retries, and says so when the retry does not help. Three things in it fail -/// silently if a later edit gets them wrong, which is why they are asserted here rather than left -/// to a reviewer: the script has to run *after* Blazor is defined, it has to keep handing the -/// manifest hash to fetch, and it has to leave the runtime's own ES modules alone. +/// The retry itself is tested where it can actually be executed: tests/boot/boot.test.mjs +/// runs boot.js against a fake network. What is left here is the wiring those tests cannot +/// see — which file loads in which order, and whether the explanation can still be shown when the +/// file holding it is the one that never arrived. +/// +/// Everything is asserted against source with comments stripped. An earlier version of these tests +/// searched the raw text, and a reviewer showed every one of them could be satisfied by a comment +/// while the real markup said the opposite. /// public class WebBootTests { private static readonly string WebRoot = FindWebRoot(); + private static readonly string IndexHtml = - File.ReadAllText(Path.Combine(WebRoot, "index.html")); - private static readonly string BootJs = - File.ReadAllText(Path.Combine(WebRoot, "js", "boot.js")); + StripComments(File.ReadAllText(Path.Combine(WebRoot, "index.html"))); [Fact] public void Blazor_does_not_autostart_so_boot_js_can_supply_the_retry() @@ -44,38 +48,62 @@ public void Boot_script_is_loaded_after_blazor_defines_itself() } [Fact] - public void Boot_script_starts_blazor_through_a_boot_resource_loader() + public void The_failure_panel_stays_inline_so_it_survives_its_own_file_not_arriving() { - Assert.Contains("Blazor.start(", BootJs); - Assert.Contains("loadBootResource", BootJs); + // The point of the whole exercise. Move this into a .js file and the one failure it could + // never explain becomes its own: a 503 on that request leaves the eternal spinner back. + var panel = IndexHtml.IndexOf("window.signsofaiBoot", StringComparison.Ordinal); + var firstScriptFile = IndexHtml.IndexOf("js/boot.js", StringComparison.Ordinal); + + Assert.True(panel >= 0, "The inline boot panel is gone from index.html."); + Assert.True(panel < firstScriptFile, + "The panel must be defined before any script it has to survive the loss of."); } [Fact] - public void Retry_still_hands_the_manifest_hash_to_fetch() + public void A_watchdog_covers_the_failures_the_retry_cannot_reach() { - // Returning a Response from loadBootResource takes the integrity check away from Blazor. - // Dropping `integrity` here would therefore not fail anything — it would quietly stop - // verifying every assembly the app loads, which is the opposite of what this file is for. - Assert.Contains("integrity: integrity", BootJs); + // The runtime's ES modules must be left to the default loader, so they get no retry; and a + // connection that hangs never errors. Only a watchdog catches those. + Assert.Contains("setInterval", IndexHtml); + Assert.Contains("lastProgress", IndexHtml); } [Fact] - public void Runtime_modules_are_left_to_the_default_loader() + public void The_panel_offers_the_desktop_app_by_absolute_url_not_an_in_app_route() { - // The dotnet.js family is imported as ES modules, which needs a URL. Answer those with a - // Response and the import fails — turning a fix for rare 503s into a permanent breakage. - Assert.Contains("dotnetjs", BootJs); - Assert.Contains("undefined", BootJs); + // Every route on this site is served by this same page, so a link to /download would try to + // start the app it just failed to start. The escape hatch has to leave the site. + var match = Regex.Match(IndexHtml, @"var DESKTOP = '([^']+)'"); + + Assert.True(match.Success, "The panel no longer names a desktop download URL."); + Assert.StartsWith("https://github.com/", match.Groups[1].Value); + } + + [Fact] + public void The_panel_prefers_the_language_the_reader_actually_chose() + { + // The app persists the switch here. Reading only navigator.language would show an English + // panel to a teacher using the app in Spanish — issue #36 in the one place Loc cannot reach. + Assert.Contains("signsofai.ui.lang", IndexHtml); } [Theory] - // The app picks its language from the browser, and this panel renders before the app exists, - // so it carries its own copy. English-only here would be the #36 bug in a new place. - [InlineData("No se pudo terminar de cargar")] - [InlineData("Couldn't finish loading")] - public void Failure_panel_speaks_both_languages(string phrase) + [InlineData("No se pudo completar la carga")] + [InlineData("Couldn’t load")] + public void The_panel_speaks_both_languages(string phrase) + { + Assert.Contains(phrase, IndexHtml); + } + + /// + /// Removes HTML comments and JavaScript line comments, so an assertion cannot be satisfied by + /// prose describing what the markup ought to do. // inside a URL is left alone. + /// + private static string StripComments(string source) { - Assert.Contains(phrase, BootJs); + var withoutHtml = Regex.Replace(source, "", string.Empty, RegexOptions.Singleline); + return Regex.Replace(withoutHtml, @"(? calls.push(args.join(' ')) }, + setTimeout: (fn, delay) => setTimeout(fn, delay === ABORT_DELAY ? abortAfterMs : 1), + clearTimeout, + fetch: (url, init) => fetch(url, init), + }; + if (withBlazor) { + sandbox.Blazor = { + start(options) { + loadBootResource = options.loadBootResource; + return onStart ? onStart() : new Promise(() => {}); + }, + }; + } + + vm.createContext(sandbox); + vm.runInContext(SOURCE, sandbox); + return { boot, logged: calls, load: (...args) => loadBootResource(...args) }; +} + +const ok = () => ({ ok: true, status: 200 }); +const unavailable = () => ({ ok: false, status: 503 }); + +test('a transient 503 is retried and the asset still loads', async () => { + let attempts = 0; + const app = load({ fetch: async () => (++attempts < 3 ? unavailable() : ok()) }); + + const response = await app.load('assembly', 'A.wasm', '/A.wasm', 'sha256-x'); + + assert.equal(response.ok, true); + assert.equal(attempts, 3, 'should have used all three attempts'); + assert.equal(app.boot.shown, false, 'a recovered download must not raise the panel'); + assert.equal(app.boot.progressed, 1, 'a completed download must report progress'); +}); + +test('a recovered download says so, so a rescued startup leaves a trace', async () => { + let attempts = 0; + const app = load({ fetch: async () => (++attempts < 2 ? unavailable() : ok()) }); + + await app.load('assembly', 'A.wasm', '/A.wasm', 'sha256-x'); + + assert.match(app.logged.join('\n'), /recovered after 2 attempts/); +}); + +test('a download that never succeeds gives up and names the file it could not get', async () => { + let attempts = 0; + const app = load({ fetch: async () => { attempts++; return unavailable(); } }); + + await assert.rejects(() => app.load('assembly', 'A.wasm', '/A.wasm', 'sha256-x')); + + assert.equal(attempts, 3, 'should stop after three attempts, not retry for ever'); + assert.equal(app.boot.shown, true, 'the panel must be raised from the retry loop'); + assert.match(app.boot.detail, /HTTP 503/); + assert.match(app.boot.detail, /\/A\.wasm/); +}); + +test('the manifest hash is handed to fetch on every attempt', async () => { + // Load-bearing, and silent if it breaks. Returning a Response short-circuits the runtime before + // it sets integrity of its own, so dropping this would fail no test and quietly stop verifying + // every assembly the app loads. + const seen = []; + const app = load({ fetch: async (url, init) => { seen.push(init.integrity); return unavailable(); } }); + + await assert.rejects(() => app.load('assembly', 'A.wasm', '/A.wasm', 'sha256-abc')); + + assert.deepEqual(seen, ['sha256-abc', 'sha256-abc', 'sha256-abc']); +}); + +test('an asset with no hash in the manifest is not sent an empty integrity', async () => { + // The runtime normalises a missing hash to "", and fetch would treat "" as "verify nothing" + // rather than "no opinion". Passing undefined keeps parity with the default loader. + let seen; + const app = load({ fetch: async (url, init) => { seen = init; return ok(); } }); + + await app.load('assembly', 'A.wasm', '/A.wasm', ''); + + assert.equal(seen.integrity, undefined); +}); + +test('the first attempt may use the cache; the retries may not', async () => { + const seen = []; + const app = load({ fetch: async (url, init) => { seen.push(init.cache); return unavailable(); } }); + + await assert.rejects(() => app.load('assembly', 'A.wasm', '/A.wasm', 'sha256-x')); + + assert.deepEqual(seen, ['default', 'reload', 'reload']); +}); + +test('the runtime ES modules are left to the default loader and never fetched here', async () => { + // Answering these with a Response breaks the import outright — a fix for rare 503s turned into + // a permanent failure. The old test for this passed against `void undefined;`. + let fetched = 0; + const app = load({ fetch: async () => { fetched++; return ok(); } }); + + const result = await app.load('dotnetjs', 'dotnet.js', '/dotnet.js', 'sha256-x'); + + assert.equal(result, undefined, 'must hand dotnetjs back to the default loader'); + assert.equal(fetched, 0, 'must not fetch a dotnetjs module itself'); +}); + +test('a connection that hangs is abandoned rather than waited on for ever', async () => { + // The failure the first version missed: fetch has no timeout, so a server that accepts and then + // never answers consumed no attempt, reached no panel, and left the eternal spinner in place. + let attempts = 0; + const app = load({ + abortAfterMs: 5, + fetch: (url, init) => new Promise((resolve, reject) => { + attempts++; + init.signal.addEventListener('abort', () => reject(new Error('aborted'))); + }), + }); + + await assert.rejects(() => app.load('assembly', 'A.wasm', '/A.wasm', 'sha256-x')); + + assert.equal(attempts, 3); + assert.equal(app.boot.shown, true, 'a hung connection must still reach the panel'); +}); + +test('a file that failed once and recovered is not blamed for a later startup error', async () => { + // The misattribution a reviewer found by executing it: failure state kept per module rather + // than per download let a recovered file be named as the cause of somebody else's error. + let attempts = 0; + let capture; + const app = load({ + fetch: async () => (++attempts === 1 ? unavailable() : ok()), + onStart: () => new Promise((resolve, reject) => { capture = reject; }), + }); + + await app.load('assembly', 'recovered.wasm', '/recovered.wasm', 'sha256-x'); + capture(new Error('WASM instantiation failed')); + await new Promise(done => setTimeout(done, 5)); + + assert.equal(app.boot.shown, true); + assert.match(app.boot.detail, /WASM instantiation failed/); + assert.doesNotMatch(app.boot.detail, /recovered\.wasm/, + 'the panel must report the error that raised it, not a download that succeeded'); +}); + +test('a successful startup marks the app as started', async () => { + const app = load({ fetch: async () => ok(), onStart: () => Promise.resolve() }); + + await new Promise(done => setTimeout(done, 5)); + + assert.equal(app.boot.started, true); + assert.equal(app.boot.shown, false); +}); + +test('a missing Blazor script is reported at once rather than waited out', async () => { + // A 503 on blazor.webassembly.js leaves this file running with nothing to call. Before the + // guard it threw a synchronous ReferenceError that no catch saw. + const app = load({ fetch: async () => ok(), withBlazor: false }); + + assert.equal(app.boot.shown, true); + assert.match(app.boot.detail, /blazor\.webassembly\.js/); +});