fix(server): serve SPA fallback index.html with 200 instead of 404 - #9
Open
itwormz wants to merge 1 commit into
Open
fix(server): serve SPA fallback index.html with 200 instead of 404#9itwormz wants to merge 1 commit into
itwormz wants to merge 1 commit into
Conversation
serve_static_ui() attached the SPA fallback router via ServeDir::not_found_service(), which tower-http implements as fallback(SetStatus(_, 404)) — the SetStatus layer unconditionally overrode the fallback's own 200, so every SPA route that did not match a physical file (e.g. /login, /stacks/<id>) came back as 404 + index.html. This broke client-side routing on full page load, refresh, deep and share links (moghtech/komodo#886). Use ServeDir::fallback() instead, which does not rewrite the response status. ETag (SHA-256 of index.html) and force_no_cache (Cache-Control: no-cache) behavior are preserved. Public return type changes from ServeDir<SetStatus<Router>> to ServeDir<Router>; consumers attaching the service as-is (Router::fallback_service, e.g. komodo) are unaffected. Add 5 regression tests in ui.rs (tower::ServiceExt::oneshot, axum 0.8) covering GET / (200 + ETag), GET /login and GET /stacks/abc (200 + index.html — failed with 404 before this fix), GET /asset.txt (200 + file contents) and force_no_cache=true (200 + Cache-Control: no-cache).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mogh_server::ui::serve_static_ui()returns HTTP 404 (with theindex.htmlbody) for any SPA route that does not match a physical file — e.g.
/login,/stacks/<id>. This PR makes the SPA fallback respond with 200 as intended.Root cause
The fallback router is built correctly and returns 200:
but it is attached via
ServeDir::not_found_service():tower-http's
ServeDir::not_found_serviceis implemented as:i.e. it unconditionally wraps the fallback in a
SetStatus(_, 404)layer, whichoverrides the fallback's own 200 status. Every request that misses a
physical file therefore comes back as
404 + index.html.This breaks single-page apps: client-side routing never works for any
non-root URL (full page load on
/login, refresh on a detail page, deep links,share links). Observed downstream in
moghtech/komodo#886:
Safari shows "The server responded with 404" on
/loginwhile the HTML body iscorrect.
Fix
Use
ServeDir::fallback()instead, which does not rewrite the response status:Applied in both branches of
serve_static_ui(force_no_cacheand ETagpaths). The
SetStatusimport and theServeDir<SetStatus<Router>>returntype become
ServeDir<Router>.Behavior
GET /GET /login,GET /stacks/<id>Notes:
SHA-256ofindex.html) andforce_no_cache(
Cache-Control: no-cache) behavior is preserved.request for a non-existent static asset now returns
200 + index.htmlinstead of a bare
404. The SPA shell handles this via its 404 route.Public API change
serve_static_ui's return type changes fromServeDir<SetStatus<Router>>toServeDir<Router>. Consumers that use the service as-is (e.g.Router::fallback_service(serve_static_ui(...))— how komodo consumes it) areunaffected; only code that names the concrete type explicitly would need a
one-line update.
Tests
Added
#[cfg(test)]unit tests inui.rsusingtower::ServiceExt::oneshot(
Router::oneshotwas removed in axum 0.8). The service is wrapped in an axumRouterfallback — exactly the way downstream consumers (komodo) attach it:GET /→ 200, body equalsindex.html,ETagpresentGET /login→ 200 +index.htmlbody (fails with 404 before this PR)GET /stacks/abc→ 200 +index.htmlbody (fails with 404 before this PR)GET /asset.txt(existing file) → 200 + file contentsforce_no_cache = true:GET /login→ 200 +Cache-Control: no-cache(
cargo fmt --all -- --checkandcargo buildare clean. This repo has no CIworkflow, so results are attached here.)
Release request
Could the version be bumped to
1.5.1(or1.6.0, if the public return-typechange should be treated as minor) and published to crates.io after merge?
Fixes moghtech/komodo#886 (komodo; cross-repo link, will close on that side).