Repository navigation
fix: serve SPA client routes with 200 and keep API 404s out of the fallback - #383
Open
eastagiletracker wants to merge 1 commit into
Open
eastagiletracker wants to merge 1 commit into
eastagiletracker wants to merge 1 commit into
Conversation
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.
This PR proposes serving the Dappi admin UI's client-side routes with HTTP 200 and keeping unknown
/apiroutes and missing static files out of the SPA fallback inUseDappi. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/1150. You can sign in with your GitHub ID to claim ownership of the project.What was wrong
The fallback middleware in
AppExtensions.UseDappirewrites the request to/index.htmlwhenever the first pass ends in an unstarted 404, but it never resets the status.StaticFileMiddlewarekeeps a non-200 status it finds, soindex.htmlgoes out with the original 404: every admin UI page, includingGET /, is served as404 Not Found. That is invisible in a browser but trips uptime checks, load balancer health probes and CDN caching. The same|| StatusCode == 404branch also catches/api/*and file requests, so an unknown API route or a missing JS chunk returns the SPA's HTML instead of a plain 404, which makes client-side errors hard to read.Reproduced at
65708a5(currentmain) with the integration test host plus awwwroot/index.html:The change
The fallback now only applies to
GET/HEADrequests for extensionless, non-/apipaths that matched no endpoint and ended in an unstarted 404, and it sets the status back to 200 before servingindex.html. API responses (success and error) and existing static files are untouched, and a 404 returned by a matched endpoint is left as it is rather than re-run against/index.html.Verification
Dappi.HeadlessCms.Tests/Middleware/SpaFallbackTests.cs(10 cases) runs the realUseDappimiddleware chain with a temporary web root. On the unchanged tree 7 fail (client routes andHEAD /return 404;/api/does-not-exist,/api/auditTrail/and/assets/missing-chunk.jsreturn the SPA HTML) and the 3 controls pass (/index.htmlserved directly,POSTto a client route does not get the SPA, API 401/200/404 responses keep their status and body). With the change all 10 pass. Removing only the status reset turns the 4 client-route cases red again.dotnet test Dappi.HeadlessCms.Tests: 241 passed before, 251 passed after (the 10 new tests), 0 failures.dotnet test Dappi.Core.Tests: 89 passed.dotnet build Dappi.HeadlessCms -c Releasesucceeds. Touched files formatted with the repo's CSharpier hook.How this was managed
This work was tracked as https://eastagiletracker.com/projects/1150/stories/1349837 on a board imported from your issues and pull requests (351 stories), https://eastagiletracker.com/projects/1150.
If you'd rather not receive contributions like this, reply
no-more-prson this pull request and we won't open any further ones on your repositories.Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com