fix: serve SPA fallback with 200 instead of 404 - #218
Closed
matheuslealpa wants to merge 2 commits into
Closed
matheuslealpa wants to merge 2 commits into
matheuslealpa wants to merge 2 commits into
Conversation
`ServeDir::not_found_service` wraps the fallback in `SetStatus`, which forces every response to `404 Not Found`. Client side routes such as `/note/<id>` and `/about` were therefore served the correct `index.html` but with a 404 status. Use `ServeDir::fallback` instead, which leaves the status untouched. A note that genuinely does not exist is still reported as 404 by `/api/notes/<id>`. Behind a reverse proxy this made effectively every document request show up as a 4xx, skewing error rate dashboards and triggering false alerts. Fixes cupcakearmy#217
Owner
|
Thanks for this, superseeded by #221 |
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
Fixes #217.
The SPA fallback served
index.htmlwith a404 Not Foundstatus. Any clientside route —
/note/<id>,/about— returned the correct document, but withthe wrong status code.
Cause
ServeDir::not_found_service()is not just "serve this file when nothingmatches". It wraps the fallback in
SetStatusand forces every response to 404:The sibling method
ServeDir::fallback()does the same routing but leaves thestatus alone — as documented upstream: "The status code returned by the
fallback will not be altered."
Change
One method swap in
packages/backend/src/main.rs:A note that genuinely does not exist is still reported as
404by/api/notes/<id>, which is where that signal belongs.Why it matters
Behind a reverse proxy this made effectively 100% of document requests show up
as 4xx, which skews error rate dashboards and can trigger false alerts. It also
logged a
404 (Not Found)in the browser console on every page load.Tests
Added
test/web/spa-fallback.spec.ts, asserting/,/aboutand/note/<unknown>return 200 while/api/notes/<unknown>still returns 404.Verified the test actually catches the bug rather than passing vacuously — with
the fix reverted and the image rebuilt, the
/aboutand/note/<unknown>casesfail with
Expected: 200 / Received: 404; with the fix they pass.//about/note/<unknown>/_app/version.json/api/status/api/notes/<unknown>Reproduced the original behaviour against the published
cupcakearmy/cryptgeon:2.9.3image before changing anything.Trade-off worth flagging
With
fallback(), a genuinely missing static asset (e.g./_app/immutable/does-not-exist.js) now returns 200 withindex.htmlinsteadof 404. This is the standard SPA fallback behaviour — the same as nginx's
try_files $uri /index.html— and is the direct consequence of serving clientside routes with 200, since the server cannot tell the two apart by path alone.
If you would rather keep 404 for missing assets, the fallback can be gated on
the request's
Acceptheader (text/html→ index.html, otherwise 404). Happyto push that instead — it is a bit more machinery, so I went with the simpler
change first.