🐛 Add MSW catch-all for unmocked API routes#4366
Conversation
- Add MSW handler for POST /auth/refresh — eliminates JSON parse errors from AuthCallback and silent token refresh getting HTML instead of JSON - Add www.googletagmanager.com to CSP connect-src — fixes GTM td endpoint being blocked by Content Security Policy - Add MSW passthrough for dicebear.com, fonts.gstatic.com, and fonts.googleapis.com — eliminates unhandled request warnings - Suppress MSW warnings for cross-origin requests and font files (.woff2/.woff/.ttf) in onUnhandledRequest handler Signed-off-by: Andrew Anderson <andy@clubanderson.com>
The unified-controls baseline (1 → 34) and non-localized strings baseline (167 → 187) drifted from reality as cards were added. Both counts are identical on main — this just syncs the baselines. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
On Netlify, unhandled API requests fall through to the SPA catch-all which returns index.html (200 OK). Code calling .json() then throws "Unexpected token '<'". This catch-all returns 503 JSON so callers hit their error paths gracefully instead of parsing HTML as JSON. Fixes: /api/mcp/gpu-nodes/stream, /api/kagent/status, and any future unmocked API routes. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
There was a problem hiding this comment.
Pull request overview
Adds a defensive MSW fallback response for otherwise-unhandled /api/* requests in Netlify/demo deployments, preventing HTML SPA fallback responses from being parsed as JSON and throwing Unexpected token '<'.
Changes:
- Add additional MSW passthrough + mock handlers (external resources and
/auth/refresh) and introduce an/api/*catch-all that returns JSON503. - Update MSW
onUnhandledRequestfiltering to ignore more static asset types and suppress warnings for cross-origin requests. - Expand Netlify CSP
connect-srcto includehttps://www.googletagmanager.com.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/mocks/handlers.ts | Adds passthrough/mock handlers and a new /api/* catch-all JSON 503 response. |
| web/src/mocks/browser.ts | Tweaks MSW unhandled-request warning suppression for assets and cross-origin requests. |
| netlify.toml | Updates CSP to allow googletagmanager.com in connect-src. |
| .github/ai-non-localized-baseline.txt | Updates AI baseline count. |
| .github/ai-missing-unified-baseline.txt | Updates AI baseline count. |
| // ── Catch-all for unmocked API routes ──────────────────────────── | ||
| // On Netlify, unhandled /api/* and /health requests fall through to the SPA | ||
| // catch-all which returns index.html (200 OK, text/html). Code calling | ||
| // .json() then throws "Unexpected token '<'". This catch-all returns a | ||
| // proper JSON 503 so callers hit their error paths gracefully. | ||
| http.all('/api/*', () => { | ||
| return HttpResponse.json( | ||
| { error: 'not available in demo mode' }, | ||
| { status: 503 }, | ||
| ) | ||
| }), |
There was a problem hiding this comment.
The new http.all('/api/*') catch-all will intercept every same-origin /api/... request that isn't already mocked earlier in this file. That likely blocks Netlify Function-backed endpoints that should stay live in demo mode (e.g. /api/missions/*, /api/nightly-e2e/* + /api/public/nightly-e2e/*, /api/gtag, /api/m, /api/analytics-dashboard), turning them into 503s.
Suggested fix: add explicit allow-list passthrough handlers for the Netlify Function routes (placed before this catch-all), or narrow the catch-all scope to only the known non-existent backend prefixes (e.g. /api/mcp/*, /api/kagent/*, etc.).
| }), | ||
|
|
||
| // ── Catch-all for unmocked API routes ──────────────────────────── | ||
| // On Netlify, unhandled /api/* and /health requests fall through to the SPA |
There was a problem hiding this comment.
The comment says unhandled "/api/* and /health" requests fall through, but this handler only matches /api/* (and /health is already explicitly mocked above). Please update the comment to match the actual behavior, or add a separate handler if /health is intended to be covered here.
| // On Netlify, unhandled /api/* and /health requests fall through to the SPA | |
| // On Netlify, unhandled /api/* requests fall through to the SPA |
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 1 code suggestion(s) and 1 general comment(s). @copilot Please apply all of the following code review suggestions:
Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
|
Post-merge build verification passed ✅ Both Go and frontend builds compiled successfully against merge commit |
✅ Post-Merge Verification: passedCommit: |
Both review items addressed in commit
Note: PR #4366 was already merged while this session was running. The changes in |
// On Netlify, unhandled /api/* requests fall through to the SPA)/api/*catch-all/api/nightly-e2e/*,/api/public/nightly-e2e/*/api/missions/*/api/analytics-dashboard/api/gtag,/api/m,/api/send,/api/ksc