fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233

Activity

1. dborup commented on Oct 5, 2026

   

   dborup

   on Oct 5, 2026

   OwnerAuthor

   More actions

   Plan — CS-MacBook #233

   Where the fallback goes: (s *Server) RegisterRoutes in cmd/server/routes.go, as the last statement — not a separate change in main.go. Every r.HandleFunc/r.Handle call in RegisterRoutes already registers an absolute /api/... path on the single top-level *mux.Router that main.go builds (router := mux.NewRouter(); srv.RegisterRoutes(router)); there is no existing /api subrouter to attach NotFoundHandler/MethodNotAllowedHandler to without rewriting every route's path string. So the fix adds one explicit fallback route instead:

   router.PathPrefix("/api/").HandlerFunc(apiFallbackHandler(router))

   registered as the final line of RegisterRoutes, i.e. strictly after every real /api/* route and strictly before main.go registers /ws and the SPA PathPrefix("/") catch-all (those run later, once RegisterRoutes has returned). gorilla/mux tries routes in registration order; a route whose path matches but whose method doesn't is a "keep trying", not a final 405 — that's exactly why a POST /api/packets request currently falls through, past the mismatched GET /api/packets route, all the way to the SPA catch-all. Putting our own PathPrefix("/api/") route right after the real ones intercepts that fall-through before it ever reaches the SPA handler, for every caller of RegisterRoutes (main.go and every test's setupTestServer), without touching main.go or any other route's registration.

   404 vs 405: apiFallbackHandler re-walks the router (router.Walk) and, for the incoming request's literal path, checks every other /api/* route's own declared method(s) via route.Match with the method swapped to the route's own — i.e. "would this route's path pattern (including {params}) match this URL at all, regardless of method". This reuses mux's own path/regex matching instead of reimplementing it (same trick buildOpenAPISpec already uses for routes). If that set is non-empty, respond 405 with Allow: <methods> and the existing writeError JSON body; otherwise 404 with writeError. No new map[string]interface{} — writeError already exists in routes.go.

   What stays untouched:

   • /ws is not under /api/, so the new PathPrefix("/api/") never sees it.
   • Health/metrics/mqtt-status/perf/etc. are all registered as /api/... routes inside RegisterRoutes already, so they match before the fallback is ever reached — unaffected.
   • The SPA fallback for non-/api paths (/, static assets, #/... deep links) is registered later in main.go and is never reached by /api/* requests once the new route exists — unaffected for everything else.
   • Existing TestPostPacketsRemovedFallsThroughToSPAInProductionRouter (refactor(server): remove the dead POST /api/packets endpoint (#223) #231) currently pins the bug (POST /api/packets → 200 index.html) as "tracked in fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233". That assertion will be updated to expect 405, since fixing fix(server): unknown /api/* paths and wrong methods return 200 index.html instead of a JSON 404/405 #233 is exactly what flips it.

   Docs: add a short 404/405 note to docs/api-spec.md's existing "Error Responses" section, and a one-sentence addendum to the OpenAPI info.description in openapi.go.

   Tests (red first): unknown GET /api/doesnotexist → JSON 404; POST /api/packets → 405 with Allow: GET; a full OpenAPI-route sweep (GET /api/spec, then replay every documented method+path) asserting none of them regressed to 404/405; /ws upgrade and SPA deep-link/static-file behavior unchanged. Mutants: fallback removed, fallback registered after the SPA catch-all instead of before, 405 replaced by 200.
2. dborup commented on Oct 6, 2026

   

   dborup

   on Oct 6, 2026

   OwnerAuthor

   More actions

   Fixed by #266 (merged as 81b13b20).
3. dborup

   closed this as completedon Oct 6, 2026