Replace explicit client route registration with SPA catch-all fallback#22136
Open
dannon wants to merge 3 commits intogalaxyproject:devfrom
Open
Replace explicit client route registration with SPA catch-all fallback#22136dannon wants to merge 3 commits intogalaxyproject:devfrom
dannon wants to merge 3 commits intogalaxyproject:devfrom
Conversation
dannon
force-pushed
the
fix/spa-catch-all
branch
from
a53a54b to
0228701
Compare
Member
|
Such a nice developer QOL improvement - awesome! |
Member
|
It looks like there's just no request handling getting triggered at all ? |
Galaxy maintained 133 explicit add_client_route() calls in buildapp.py that had to stay in sync with the Vue router -- forget one and the route 404s on page refresh. Since all client routes serve the same static js-app.mako template with no server-side data injection, there's no reason to enumerate them. Instead, when no server route matches a non-API path, the WSGI layer now falls back to serving the SPA directly and lets the Vue router handle routing (including showing its own 404 for truly invalid paths). API paths still 404 normally, static files are handled by URLMap before this code runs, and legacy controller routes still match via the mapper first. This removes the clientside_routes mapper, the add_client_route() method, and all 133 route registrations from buildapp.py.
Published pages and workflows with embed=true are meant for cross-origin iframe embedding, but the global X-Frame-Options: SAMEORIGIN header blocks that. Conditionally skip the header when the request path starts with /published/ and the embed=true query param is present. Applied to both the WSGI transaction init and the FastAPI middleware for consistency.
The health check was doing GET / and expecting 200, but with the SPA catch-all the root URL may not return 200 when the client build is skipped. Using /api/version instead is more reliable since it works regardless of client build state.
dannon
force-pushed
the
fix/spa-catch-all
branch
from
0228701 to
6162889
Compare
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
3 participants
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
Galaxy maintained 133 explicit
add_client_route()calls inbuildapp.pythat had to stay in sync with the Vue router — forget one and the route 404s on page refresh. Since all client routes serve the same staticjs-app.makotemplate with no server-side data injection, there's no reason to enumerate them individually.I replaced this with a simple catch-all: when no server route matches a non-API path, the WSGI layer falls back to serving the SPA directly and lets the Vue router handle routing. This includes showing its own 404 for truly invalid paths, which is better UX than a raw server 404 anyway.
What stays safe:
/api/*)/authnz/*,/datasets/*,/u/{username}/*, etc.) match via the mapper before the fallbackThe only behavioral change is that previously-unregistered paths like
/asdfghnow serve the SPA (where Vue shows its 404 page) instead of a raw server 404. New Vue routes no longer need a correspondingbuildapp.pyentry.Came out of #22107 where Marius asked about per-route X-Frame-Options — that's not feasible because
/published/*are client-side SPA routes, not FastAPI routes. But it surfaced this cleanup opportunity, and I also added conditional X-Frame-Options handling: published embed views (/published/*?embed=true) now skip theX-Frame-Options: SAMEORIGINheader so they can actually load in cross-origin iframes as intended (#21074). The exemption is applied in both the WSGI transaction init and the FastAPI middleware. Normal (non-embed) requests keep the header for clickjacking protection.Test plan
test_framework_base,test_routes)test_navigates_galaxy,test_histories_published,test_login,test_histories_list,test_published_pages)test_framework— includes new X-Frame-Options tests for embed/non-embed published routes)