Give the JS bridge the same fallback the framework script got #5

Merged

cheapnud merged 2 commits from b/asset-fallback-layers into master 2026-08-26 14:25:31 +00:00

Conversation 1 Commits 2 Files changed 12 +382 -39

cheapnud commented 2026-08-26 14:20:38 +00:00

Owner

Copy link

Follow-up to #4. That PR covered blazor.web.js and left cheap-blazor-interop.js on the single static-web-asset path the removed extractor used to back up.

Measured: a broken static web asset does not 404. MapStaticAssets keeps the route from the endpoints manifest and either throws FileNotFoundException (500) or answers 200 with a zero-length body. Both render as a blank white window, and the zero-length case looks healthy in a network trace and in the request log.

The bridge is embedded alongside blazor.web.js and served from the assembly when it is not on disk. A dead static web assets manifest no longer kills the process at startup. And the host now checks its own page at startup and names any asset that comes back failing or empty, including third-party ones it cannot serve itself.

Verified on six deployment shapes with an empty NUGET_PACKAGES.

Follow-up to #4. That PR covered blazor.web.js and left cheap-blazor-interop.js on the single static-web-asset path the removed extractor used to back up. Measured: a broken static web asset does not 404. MapStaticAssets keeps the route from the endpoints manifest and either throws FileNotFoundException (500) or answers 200 with a zero-length body. Both render as a blank white window, and the zero-length case looks healthy in a network trace and in the request log. The bridge is embedded alongside blazor.web.js and served from the assembly when it is not on disk. A dead static web assets manifest no longer kills the process at startup. And the host now checks its own page at startup and names any asset that comes back failing or empty, including third-party ones it cannot serve itself. Verified on six deployment shapes with an empty NUGET_PACKAGES.

cheapnud added 1 commit 2026-08-26 14:20:38 +00:00

Give the JS bridge the same fallback the framework script got, and name silent asset failures ... a9a7dd86e8

The blazor.web.js fix covered one of the two scripts this library is responsible
for. The other one, cheap-blazor-interop.js, lost its embedded copy and its
serving fallbacks when the extractor was removed, on the grounds that a static
web asset is enough. It is not.

Measured against a published app with its wwwroot assets removed and no NuGet
cache anywhere: a broken static web asset does not 404. MapStaticAssets keeps
the route from the endpoints manifest, so it either throws FileNotFoundException
and returns 500, or answers 200 with a zero-length body. Both render as a blank
white window with no controls, and the zero-length case reports as healthy in a
network trace, in the request log, and to the browser.

So:

- cheap-blazor-interop.js is embedded in the assembly alongside blazor.web.js
  and served from there when it is not on disk. The two share one fallback
  table and one middleware. A test asserts every entry in that table actually
  resolves, and that the embedded bridge matches the file on disk byte for byte.
- A .staticwebassets.runtime.json whose content roots no longer exist used to
  take the process down at startup with no window, no message and exit code 0.
  That manifest records absolute paths into the build machine's NuGet cache and
  source tree, so it is wrong on every machine but the one that produced it.
  Now caught, logged, and the app runs on what is reachable.
- The host fetches its own root document once at startup, follows every local
  _framework and _content reference in it, and logs an error naming any that
  come back failing or empty. External URLs are skipped so an offline machine
  does not look like a broken app.

Third-party _content assets are still disk-only; this library has no copy of
MudBlazor to serve. The startup check names them, and the deployment table in
architecture.md says which deployments leave them behind.

Verified on six deployment shapes with an empty NUGET_PACKAGES: publish via
PackageReference and ProjectReference, publish with our own assets deleted,
Debug publish, single-file self-contained, and a copy of raw build output with
its manifests pointed at a machine that does not exist.

forgejo-actions commented 2026-08-26 14:21:22 +00:00

Copy link

🤖 Claude AI Code Review

Code Review: JS Bridge Embedded Fallback

1. Summary

This PR extends the embedded-fallback pattern (previously applied only to blazor.web.js) to cheap-blazor-interop.js. It also:

• Introduces a shared EmbeddedAssets helper (deduplicating manifest-resource access logic)
• Generalizes the fallback middleware to serve any asset in a lookup table
• Adds resilience around UseStaticWebAssets() so a stale manifest cannot crash startup
• Adds StartupAssetCheck, a startup diagnostic that fetches the root page and reports assets returning failures or empty bodies
• Adds solid test coverage and thorough documentation

Overall this is high-quality, well-motivated work with excellent comments and tests. My concerns are minor.

2. Code Quality

Strengths:

• EmbeddedAssets is a clean refactor — resolving the manifest name set once into a HashSet and routing both BlazorFrameworkExtractor and the new middleware through it removes duplication nicely.
• The middleware generalization (table-driven available dictionary filtered by what's actually embedded) reads well and preserves the "middleware not endpoint" invariant.
• Comments are genuinely informative — they explain why (the 500/empty-200 failure modes), not just what.
• Test names are descriptive and the The_interop_bridge_fallback_matches_the_file_on_disk test guarding against a stale embedded copy is exactly the right safeguard for shipping the file twice.
• The UseStaticWebAssets() try/catch degradation is a good defensive move with a clear log message.

Minor nits:

• The stray blank-line-only diffs in EmbeddedBlazorHostService.cs (line 210 area and around _logger.LogInformation("Configuring pipeline...) add noise. Worth reverting the whitespace-only changes.
• PackagingWiringTests.cs gained a BOM (\ufeff) on line 1 — inconsistent with the rest and likely unintentional.

3. Potential Issues

Regex-based HTML parsing (StartupAssetCheck): The (?:src|href) regex will match attributes on any element, not just <script>/<link>. In practice the _framework/_content prefix filter contains the blast radius, so this is acceptable for a best-effort diagnostic — but the XML doc comment says "every script and stylesheet," which slightly overstates what the regex does. Consider softening the comment or noting the prefix filter is what scopes it.

Consistency between the fallback table and the startup check: EmbeddedFallbackAssets keys off Endpoints.JavaScriptBridgeEndpoint, while StartupAssetCheck filters on the /_content/ and /_framework/ prefixes. Please double-check that JavaScriptBridgeEndpoint actually resolves to /_content/CheapAvaloniaBlazor/cheap-blazor-interop.js (the value isn't in the diff). If it were, say, /js/bridge, the fallback path and the served path would diverge and the startup check would miss it. A small test asserting EmbeddedFallbackAssets keys all start with a recognized prefix would lock this down.

GetStringAsync(baseUrl, ...) for the root document: If the root page itself returns 500/empty (the very failure mode this targets), GetStringAsync throws and you land in the catch, which logs only at LogVerbose. That means a completely broken root document is quieter than a single broken asset (LogError). Consider treating a failed/empty root fetch as a LogError rather than swallowing it verbosely.

Fire-and-forget task: _ = Task.Run(() => StartupAssetCheck.RunAsync(...), _hostCts.Token). RunAsync is documented to never throw, so this is safe, but passing the token to both Task.Run and RunAsync means a pre-start cancellation surfaces as a TaskCanceledException on an unobserved task. Low risk given the guarantees, but worth a mental note.

Potential double-read of content: CheckOneAsync uses GetAsync + ReadAsByteArrayAsync (buffers the whole body). Fine for small assets; just be aware a misconfigured large _content asset gets fully buffered per check. Bounded by the 5s timeout, so acceptable.

4. Recommendations

• Revert the whitespace-only / BOM changes to keep the diff focused.
• Add a test asserting every key in EmbeddedFallbackAssets starts with a prefix StartupAssetCheck recognizes (ties the two subsystems together).
• Elevate a failed/empty root document fetch to LogError — it's the worst case and currently the quietest.
• Optionally tighten the doc comment on ExtractAssetPaths/StartupAssetCheck to acknowledge it's attribute-scanning bounded by prefix, not true script/link parsing.
• Confirm HttpClientFactory.CreateForServerCheck() sets the 5s timeout referenced in the comments (not visible in diff).

5. Verdict

APPROVE (with minor suggestions)

The core change is well-reasoned, the "ship it twice on purpose" decision is justified and guarded by tests, and the startup diagnostic addresses a genuinely painful silent-failure class. None of my concerns are blocking — the root-document-failure logging level and the fallback-table/prefix consistency check are the two most worth addressing before merge, but both are small.

---

📊 Tokens: 14733 input + 1954 output | 💰 Cost: ~$0.1225
Generated by Claude Opus 4.8 via Gitea Actions

## 🤖 Claude AI Code Review # Code Review: JS Bridge Embedded Fallback ## 1. Summary This PR extends the embedded-fallback pattern (previously applied only to `blazor.web.js`) to `cheap-blazor-interop.js`. It also: - Introduces a shared `EmbeddedAssets` helper (deduplicating manifest-resource access logic) - Generalizes the fallback middleware to serve any asset in a lookup table - Adds resilience around `UseStaticWebAssets()` so a stale manifest cannot crash startup - Adds `StartupAssetCheck`, a startup diagnostic that fetches the root page and reports assets returning failures or empty bodies - Adds solid test coverage and thorough documentation Overall this is high-quality, well-motivated work with excellent comments and tests. My concerns are minor. ## 2. Code Quality **Strengths:** - `EmbeddedAssets` is a clean refactor — resolving the manifest name set once into a `HashSet` and routing both `BlazorFrameworkExtractor` and the new middleware through it removes duplication nicely. - The middleware generalization (table-driven `available` dictionary filtered by what's actually embedded) reads well and preserves the "middleware not endpoint" invariant. - Comments are genuinely informative — they explain *why* (the 500/empty-200 failure modes), not just *what*. - Test names are descriptive and the `The_interop_bridge_fallback_matches_the_file_on_disk` test guarding against a stale embedded copy is exactly the right safeguard for shipping the file twice. - The `UseStaticWebAssets()` try/catch degradation is a good defensive move with a clear log message. **Minor nits:** - The stray blank-line-only diffs in `EmbeddedBlazorHostService.cs` (line 210 area and around `_logger.LogInformation("Configuring pipeline...`) add noise. Worth reverting the whitespace-only changes. - `PackagingWiringTests.cs` gained a BOM (`\ufeff`) on line 1 — inconsistent with the rest and likely unintentional. ## 3. Potential Issues **Regex-based HTML parsing (StartupAssetCheck):** The `(?:src|href)` regex will match attributes on *any* element, not just `<script>`/`<link>`. In practice the `_framework`/`_content` prefix filter contains the blast radius, so this is acceptable for a best-effort diagnostic — but the XML doc comment says "every script and stylesheet," which slightly overstates what the regex does. Consider softening the comment or noting the prefix filter is what scopes it. **Consistency between the fallback table and the startup check:** `EmbeddedFallbackAssets` keys off `Endpoints.JavaScriptBridgeEndpoint`, while `StartupAssetCheck` filters on the `/_content/` and `/_framework/` prefixes. Please double-check that `JavaScriptBridgeEndpoint` actually resolves to `/_content/CheapAvaloniaBlazor/cheap-blazor-interop.js` (the value isn't in the diff). If it were, say, `/js/bridge`, the fallback path and the served path would diverge and the startup check would miss it. A small test asserting `EmbeddedFallbackAssets` keys all start with a recognized prefix would lock this down. **`GetStringAsync(baseUrl, ...)` for the root document:** If the root page itself returns 500/empty (the very failure mode this targets), `GetStringAsync` throws and you land in the catch, which logs only at `LogVerbose`. That means a completely broken root document is *quieter* than a single broken asset (`LogError`). Consider treating a failed/empty root fetch as a `LogError` rather than swallowing it verbosely. **Fire-and-forget task:** `_ = Task.Run(() => StartupAssetCheck.RunAsync(...), _hostCts.Token)`. `RunAsync` is documented to never throw, so this is safe, but passing the token to both `Task.Run` and `RunAsync` means a pre-start cancellation surfaces as a `TaskCanceledException` on an unobserved task. Low risk given the guarantees, but worth a mental note. **Potential double-read of content:** `CheckOneAsync` uses `GetAsync` + `ReadAsByteArrayAsync` (buffers the whole body). Fine for small assets; just be aware a misconfigured large `_content` asset gets fully buffered per check. Bounded by the 5s timeout, so acceptable. ## 4. Recommendations - Revert the whitespace-only / BOM changes to keep the diff focused. - Add a test asserting every key in `EmbeddedFallbackAssets` starts with a prefix `StartupAssetCheck` recognizes (ties the two subsystems together). - Elevate a failed/empty **root document** fetch to `LogError` — it's the worst case and currently the quietest. - Optionally tighten the doc comment on `ExtractAssetPaths`/`StartupAssetCheck` to acknowledge it's attribute-scanning bounded by prefix, not true script/link parsing. - Confirm `HttpClientFactory.CreateForServerCheck()` sets the 5s timeout referenced in the comments (not visible in diff). ## 5. Verdict **APPROVE** (with minor suggestions) The core change is well-reasoned, the "ship it twice on purpose" decision is justified and guarded by tests, and the startup diagnostic addresses a genuinely painful silent-failure class. None of my concerns are blocking — the root-document-failure logging level and the fallback-table/prefix consistency check are the two most worth addressing before merge, but both are small. --- 📊 **Tokens**: 14733 input + 1954 output | 💰 **Cost**: ~$0.1225 *Generated by Claude Opus 4.8 via Gitea Actions*

cheapnud added 1 commit 2026-08-26 14:23:18 +00:00

Say plainly what the startup check does and does not promise e4a5486014

cheapnud merged commit be62a36a68 into master 2026-08-26 14:25:31 +00:00

cheapnud deleted branch b/asset-fallback-layers 2026-08-26 14:25:31 +00:00

cheapnud referenced this pull request from a commit 2026-08-26 14:25:33 +00:00

Merge pull request 'Give the JS bridge the same fallback the framework script got' (#5) from b/asset-fallback-layers into master

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

cheapnud/CheapAvaloniaBlazor!5

No description provided.