Give every JSON answer @meta, errors included (closes #201)
check / check (push) Canceled after 0s
check / check (push) Canceled after 0s
A panic in a JSON route's handler answered 500 with an empty body, because the router's recoverer sits outside the middleware that adds `@meta`. `JSONValidationMiddleware` now wraps the handler in the same recoverer, and answers the panic with the JSON error body. A method that `/status.json` or the health check does not take got chi's empty 405. Each such method now goes to the JSON 405 that the routes under `/api/v1` and `/ip` give. The home page's IP lookup card showed only the answer's `data`. It now shows the whole answer, `@meta` included. A table test checks `@meta` on every JSON route, in its successes and its errors. Model: opus-5-5
This commit is contained in:
@@ -233,11 +233,16 @@ originates no prefix and has no record in the database or in the AS list built
|
||||
into RouteWatch. AS 0 always answers 404, because it means no known origin. The
|
||||
AS name search never lists it. A prefix with no live route answers 404 too. The
|
||||
HTML pages give these errors as plain text. Under `/api/v1` and `/ip/`, a path
|
||||
no route matches answers 404 with the message `Not found`, and `POST`, `PUT`,
|
||||
`PATCH`, `DELETE`, `OPTIONS`, `TRACE` or `CONNECT` answers 405 with
|
||||
`Method not allowed`. A 500 answer says only `Internal server error`, and a host
|
||||
name that does not resolve only `could not resolve host name`; the detail goes
|
||||
to the log.
|
||||
no route matches answers 404 with the message `Not found`. A request with
|
||||
`POST`, `PUT`, `PATCH`, `DELETE`, `OPTIONS`, `TRACE` or `CONNECT` answers 405
|
||||
with `Method not allowed` on these routes:
|
||||
|
||||
- the routes under `/api/v1` and `/ip/`
|
||||
- `/status.json`
|
||||
- `/.well-known/healthcheck.json`
|
||||
|
||||
A 500 answer says only `Internal server error`, and a host name that does not
|
||||
resolve only `could not resolve host name`; the detail goes to the log.
|
||||
|
||||
Every answer carries `Strict-Transport-Security`, `Content-Security-Policy`,
|
||||
`X-Frame-Options`, `X-Content-Type-Options`, `Referrer-Policy` and
|
||||
|
||||
@@ -35,6 +35,13 @@ tested.
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-10-27: every JSON answer carries `@meta`. A panic on a JSON route answers
|
||||
500 with the JSON error body. A method that `/status.json` or the health check
|
||||
does not take answers 405 with it, as under `/api/v1`. Both used to answer
|
||||
with an empty body. The home page's IP lookup shows the whole JSON answer,
|
||||
`@meta` included. It used to show only the answer's `data`, or only the
|
||||
message of an error answer. A test checks `@meta` on every JSON route, in its
|
||||
successes and its errors (closes #201)
|
||||
- 2026-10-27: every value in a JSON answer that has a unit is a number, with the
|
||||
unit at the end of its field name. `@meta.execution_time` is now
|
||||
`execution_time_ms`. The stats' `uptime`, `connection_duration` and
|
||||
|
||||
@@ -75,7 +75,6 @@ const (
|
||||
asPagePath = "/as/64500"
|
||||
asJSONPath = "/api/v1/as/64500"
|
||||
prefixLengthPagePath = "/prefixlength/24"
|
||||
statusJSONPath = "/status.json"
|
||||
statsPath = "/api/v1/stats"
|
||||
)
|
||||
|
||||
@@ -1109,7 +1108,7 @@ func TestJSONRoutesDoNotSendErrorDetails(t *testing.T) {
|
||||
path string
|
||||
wantCode int
|
||||
}{
|
||||
{"/.well-known/healthcheck.json", http.StatusServiceUnavailable},
|
||||
{healthCheckPath, http.StatusServiceUnavailable},
|
||||
{statusJSONPath, http.StatusInternalServerError},
|
||||
{statsPath, http.StatusInternalServerError},
|
||||
{asJSONPath, http.StatusInternalServerError},
|
||||
|
||||
@@ -16,6 +16,7 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/go-chi/chi/v5/middleware"
|
||||
)
|
||||
|
||||
// responseWriter wraps http.ResponseWriter to capture the response
|
||||
@@ -154,7 +155,8 @@ func deadlinePassed(ctx context.Context) bool {
|
||||
// has passed, it answers 504 with the error message "Request timeout". A 4xx
|
||||
// keeps its status, with the status's standard text as the message. For other
|
||||
// errors, it returns "Internal server error", with status 500 if the original
|
||||
// status was 200.
|
||||
// status was 200. A panic in the handler answers 500 with "Internal server
|
||||
// error" too.
|
||||
//
|
||||
// Non-JSON responses are passed through unchanged.
|
||||
func JSONValidationMiddleware(next http.Handler) http.Handler {
|
||||
@@ -166,8 +168,9 @@ func JSONValidationMiddleware(next http.Handler) http.Handler {
|
||||
statusCode: http.StatusOK,
|
||||
}
|
||||
|
||||
// Serve the request
|
||||
next.ServeHTTP(rw, r)
|
||||
// Serve the request. Recoverer logs a panic in the handler and turns it
|
||||
// into a 500 without a body, which gets the JSON error body below.
|
||||
middleware.Recoverer(next).ServeHTTP(rw, r)
|
||||
|
||||
// Check if it's meant to be a JSON response
|
||||
contentType := rw.Header().Get("Content-Type")
|
||||
|
||||
@@ -2,6 +2,7 @@ package server
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"io"
|
||||
"log/slog"
|
||||
@@ -119,7 +120,7 @@ func TestJSONAnswersPutNoUnitInAString(t *testing.T) {
|
||||
paths := []string{
|
||||
statusJSONPath,
|
||||
statsPath,
|
||||
"/.well-known/healthcheck.json",
|
||||
healthCheckPath,
|
||||
"/api/v1/ip/" + untrustedPeerAddr,
|
||||
"/ip/" + untrustedPeerAddr,
|
||||
asJSONPath,
|
||||
@@ -156,6 +157,144 @@ func TestJSONAnswersPutNoUnitInAString(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestJSONAnswersCarryMeta sends requests to every JSON route through the
|
||||
// server's router, and checks that each answer carries @meta with its three
|
||||
// fields. The answers include successes, and errors that the handlers, the
|
||||
// router and the middleware write. Before the fix a panic in a handler, and a
|
||||
// method /status.json or the health check does not take, answered with an
|
||||
// empty body (https://git.eeqj.de/sneak/routewatch/issues/201).
|
||||
func TestJSONAnswersCarryMeta(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// JSON routes whose handlers write nothing or panic, so that the JSON
|
||||
// check on the route writes the error answer itself.
|
||||
const (
|
||||
writesNothingPath = "/writes-nothing.json"
|
||||
panicsPath = "/panics.json"
|
||||
)
|
||||
|
||||
s := newServerWithTestRoutes(t)
|
||||
s.resolver = &fakeResolver{}
|
||||
s.streamer.GetMetricsTracker().SetConnected(true)
|
||||
s.router.Get(writesNothingPath, JSONValidationMiddleware(http.HandlerFunc(
|
||||
func(http.ResponseWriter, *http.Request) {})).ServeHTTP)
|
||||
s.router.Get(panicsPath, JSONValidationMiddleware(http.HandlerFunc(
|
||||
func(http.ResponseWriter, *http.Request) { panic(errDiskIO) })).ServeHTTP)
|
||||
|
||||
tests := []struct {
|
||||
method string
|
||||
path string
|
||||
wantCode int
|
||||
}{
|
||||
// Answers that succeed
|
||||
{http.MethodGet, statusJSONPath, http.StatusOK},
|
||||
{http.MethodGet, healthCheckPath, http.StatusOK},
|
||||
{http.MethodGet, "/ip", http.StatusOK},
|
||||
{http.MethodGet, ownAddressPath, http.StatusOK},
|
||||
{http.MethodGet, "/ip/" + addrInTestPrefixV4, http.StatusOK},
|
||||
{http.MethodGet, statsPath, http.StatusOK},
|
||||
{http.MethodGet, "/api/v1/ip/" + addrInTestPrefixV4, http.StatusOK},
|
||||
{http.MethodGet, asJSONPath, http.StatusOK},
|
||||
{http.MethodGet, "/api/v1/prefix/" + testPrefixV4, http.StatusOK},
|
||||
{http.MethodGet, "/api/v1/search/as?q=google", http.StatusOK},
|
||||
// Errors the handlers write
|
||||
{http.MethodGet, "/ip/no-such-host.example", http.StatusBadRequest},
|
||||
{http.MethodGet, "/api/v1/ip/no-such-host.example", http.StatusBadRequest},
|
||||
{http.MethodGet, "/api/v1/as/foo", http.StatusBadRequest},
|
||||
{http.MethodGet, "/api/v1/as/0", http.StatusNotFound},
|
||||
{http.MethodGet, "/api/v1/prefix/192.0.2.0/33", http.StatusBadRequest},
|
||||
{http.MethodGet, "/api/v1/prefix/198.51.100.0/24", http.StatusNotFound},
|
||||
{http.MethodGet, "/api/v1/search/as", http.StatusBadRequest},
|
||||
// Errors the router and the middleware write
|
||||
{http.MethodGet, "/api/v1/no-such-route", http.StatusNotFound},
|
||||
{http.MethodGet, "/ip/192.0.2.1/nothing", http.StatusNotFound},
|
||||
{http.MethodPost, statsPath, http.StatusMethodNotAllowed},
|
||||
{http.MethodPost, ownAddressPath, http.StatusMethodNotAllowed},
|
||||
{http.MethodPost, statusJSONPath, http.StatusMethodNotAllowed},
|
||||
{http.MethodPut, healthCheckPath, http.StatusMethodNotAllowed},
|
||||
{http.MethodGet, writesNothingPath, http.StatusInternalServerError},
|
||||
{http.MethodGet, panicsPath, http.StatusInternalServerError},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
checkAnswerCarriesMeta(t, s, httptest.NewRequestWithContext(
|
||||
t.Context(), tt.method, tt.path, nil), tt.wantCode)
|
||||
}
|
||||
|
||||
// A request whose time limit has already passed answers 504. The handlers
|
||||
// write the answers of the first five routes, and the JSON check on the
|
||||
// route writes the last.
|
||||
pastTimeLimit, cancel := context.WithDeadline(t.Context(), time.Now())
|
||||
defer cancel()
|
||||
|
||||
for _, path := range []string{
|
||||
statusJSONPath, statsPath, "/ip/" + addrInTestPrefixV4,
|
||||
"/api/v1/ip/" + addrInTestPrefixV4, asJSONPath, writesNothingPath,
|
||||
} {
|
||||
checkAnswerCarriesMeta(t, s, httptest.NewRequestWithContext(
|
||||
pastTimeLimit, http.MethodGet, path, nil), http.StatusGatewayTimeout)
|
||||
}
|
||||
|
||||
// With a store whose reads fail, the health check answers 503, and the
|
||||
// handlers of the other routes 500.
|
||||
failing := New(failingDB{}, newTestTable(),
|
||||
streamer.New(logger.New(), metrics.New()), logger.New(), &config.Config{})
|
||||
failing.resolver = &fakeResolver{}
|
||||
|
||||
checkAnswerCarriesMeta(t, failing, httptest.NewRequestWithContext(
|
||||
t.Context(), http.MethodGet, healthCheckPath, nil),
|
||||
http.StatusServiceUnavailable)
|
||||
|
||||
for _, path := range []string{
|
||||
statusJSONPath, statsPath, asJSONPath,
|
||||
"/api/v1/ip/" + addrInTestPrefixV4, "/ip/" + addrInTestPrefixV4,
|
||||
} {
|
||||
checkAnswerCarriesMeta(t, failing, httptest.NewRequestWithContext(
|
||||
t.Context(), http.MethodGet, path, nil), http.StatusInternalServerError)
|
||||
}
|
||||
}
|
||||
|
||||
// checkAnswerCarriesMeta serves req through the server's router, and checks
|
||||
// that it answers wantCode with @meta and its three fields.
|
||||
func checkAnswerCarriesMeta(
|
||||
t *testing.T, s *Server, req *http.Request, wantCode int,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
// The values README "API v1" gives the two fields that never change.
|
||||
const (
|
||||
wantTimeZone = "UTC"
|
||||
wantAPIVersion = 1
|
||||
)
|
||||
|
||||
name := req.Method + " " + req.URL.String()
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
s.router.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != wantCode {
|
||||
t.Errorf("%s: status %d, want %d; body %s", name, rec.Code, wantCode, rec.Body)
|
||||
}
|
||||
|
||||
var answer map[string]any
|
||||
|
||||
err := json.Unmarshal(rec.Body.Bytes(), &answer)
|
||||
if err != nil {
|
||||
t.Errorf("%s: decoding the answer %q: %v", name, rec.Body, err)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
meta, _ := answer[jsonKeyMeta].(map[string]any)
|
||||
_, timeIsNumber := meta[metaKeyExecutionTimeMs].(float64)
|
||||
|
||||
if meta[metaKeyTimeZone] != wantTimeZone ||
|
||||
meta[metaKeyAPIVersion] != float64(wantAPIVersion) || !timeIsNumber {
|
||||
t.Errorf("%s: @meta %v, want time_zone %q, api_version %d and a "+
|
||||
"number in execution_time_ms", name, meta, wantTimeZone, wantAPIVersion)
|
||||
}
|
||||
}
|
||||
|
||||
// stringsIn returns every string in a decoded JSON value, at any depth.
|
||||
func stringsIn(value any) []string {
|
||||
var found []string
|
||||
|
||||
@@ -8,8 +8,8 @@ import (
|
||||
"github.com/go-chi/chi/v5/middleware"
|
||||
)
|
||||
|
||||
// The error messages the JSON routes under /api/v1 and /ip answer with when no
|
||||
// route matches the path, and when none takes the method.
|
||||
// The error messages the JSON routes answer with when no route matches the
|
||||
// path, and when the route does not take the method.
|
||||
const (
|
||||
notFoundMessage = "Not found"
|
||||
methodNotAllowedMessage = "Method not allowed"
|
||||
@@ -19,6 +19,13 @@ const (
|
||||
jsonRoutesAllow = "GET, HEAD"
|
||||
)
|
||||
|
||||
// The paths of the status figures and the health check, the two JSON routes
|
||||
// outside /api/v1 and /ip.
|
||||
const (
|
||||
statusJSONPath = "/status.json"
|
||||
healthCheckPath = "/.well-known/healthcheck.json"
|
||||
)
|
||||
|
||||
// setupRoutes configures the HTTP routes
|
||||
func (s *Server) setupRoutes() {
|
||||
r := chi.NewRouter()
|
||||
@@ -32,6 +39,8 @@ func (s *Server) setupRoutes() {
|
||||
r.Use(middleware.RequestID)
|
||||
r.Use(RealIPMiddleware(s.trustedProxies))
|
||||
r.Use(RequestLoggerMiddleware(s.logger.Logger)) // Structured request logging
|
||||
// Recoverer answers a panic on a page. JSONValidationMiddleware recovers a
|
||||
// panic on a JSON route itself, so that the answer is JSON with @meta.
|
||||
r.Use(middleware.Recoverer)
|
||||
r.Use(JSONResponseMiddleware)
|
||||
// Timeout must come after JSONResponseMiddleware, which sends its answer
|
||||
@@ -43,11 +52,6 @@ func (s *Server) setupRoutes() {
|
||||
// Routes
|
||||
r.Get("/", s.handleIndex())
|
||||
r.Get("/status", s.handleStatusHTML())
|
||||
r.Get("/status.json", JSONValidationMiddleware(s.handleStatusJSON()).ServeHTTP)
|
||||
r.Get(
|
||||
"/.well-known/healthcheck.json",
|
||||
JSONValidationMiddleware(s.handleHealthCheck()).ServeHTTP,
|
||||
)
|
||||
|
||||
// AS and prefix detail pages
|
||||
r.Get("/as/{asn}", s.handleASDetail())
|
||||
@@ -70,6 +74,21 @@ func (s *Server) setupRoutes() {
|
||||
s.writeJSONError(w, http.StatusMethodNotAllowed, methodNotAllowedMessage)
|
||||
}
|
||||
|
||||
// The status figures and the health check. chi sets one 405 answer for a
|
||||
// whole router. These two routes are on the pages' router, whose 405 stays
|
||||
// chi's own. So each method they do not take is routed to the JSON 405
|
||||
// here.
|
||||
r.Get(statusJSONPath, JSONValidationMiddleware(s.handleStatusJSON()).ServeHTTP)
|
||||
r.Get(healthCheckPath, JSONValidationMiddleware(s.handleHealthCheck()).ServeHTTP)
|
||||
|
||||
for _, method := range []string{
|
||||
http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodDelete,
|
||||
http.MethodOptions, http.MethodTrace, http.MethodConnect,
|
||||
} {
|
||||
r.MethodFunc(method, statusJSONPath, jsonMethodNotAllowed)
|
||||
r.MethodFunc(method, healthCheckPath, jsonMethodNotAllowed)
|
||||
}
|
||||
|
||||
// IP info JSON endpoints (replaces old /ip redirect)
|
||||
r.Route("/ip", func(r chi.Router) {
|
||||
r.Use(JSONValidationMiddleware)
|
||||
|
||||
@@ -17,7 +17,7 @@ func TestRoutesAnswerHEAD(t *testing.T) {
|
||||
|
||||
s := newServerWithTestRoutes(t)
|
||||
|
||||
for _, path := range []string{"/", statsPath, "/ip", "/ip/"} {
|
||||
for _, path := range []string{"/", statusJSONPath, statsPath, "/ip", "/ip/"} {
|
||||
rec := httptest.NewRecorder()
|
||||
s.router.ServeHTTP(rec, httptest.NewRequestWithContext(
|
||||
t.Context(), http.MethodHead, path, nil))
|
||||
@@ -31,10 +31,13 @@ func TestRoutesAnswerHEAD(t *testing.T) {
|
||||
|
||||
// TestJSONRoutesAnswerUnmatchedRequestsWithJSONErrors sends a method the JSON
|
||||
// routes do not take, and a path none of them matches, to each group of JSON
|
||||
// routes through the server's router. Each must answer with the JSON error
|
||||
// routes through the server's router. It also sends a method they do not take
|
||||
// to /status.json and the health check. Each must answer with the JSON error
|
||||
// body, and a 405 must name the methods the routes take in its Allow header.
|
||||
// Before the fix a 405 said "Internal server error" and a 404 was chi's plain
|
||||
// text answer (https://git.eeqj.de/sneak/routewatch/issues/111).
|
||||
// /status.json and the health check gave chi's empty 405
|
||||
// (https://git.eeqj.de/sneak/routewatch/issues/201).
|
||||
func TestJSONRoutesAnswerUnmatchedRequestsWithJSONErrors(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -48,6 +51,14 @@ func TestJSONRoutesAnswerUnmatchedRequestsWithJSONErrors(t *testing.T) {
|
||||
}{
|
||||
{http.MethodPost, statsPath, http.StatusMethodNotAllowed, methodNotAllowedMessage},
|
||||
{http.MethodPost, "/ip/", http.StatusMethodNotAllowed, methodNotAllowedMessage},
|
||||
{
|
||||
http.MethodDelete, statusJSONPath,
|
||||
http.StatusMethodNotAllowed, methodNotAllowedMessage,
|
||||
},
|
||||
{
|
||||
http.MethodOptions, healthCheckPath,
|
||||
http.StatusMethodNotAllowed, methodNotAllowedMessage,
|
||||
},
|
||||
{http.MethodGet, "/api/v1/nothing", http.StatusNotFound, notFoundMessage},
|
||||
{http.MethodGet, "/ip/192.0.2.1/nothing", http.StatusNotFound, notFoundMessage},
|
||||
}
|
||||
|
||||
@@ -714,7 +714,8 @@
|
||||
});
|
||||
});
|
||||
|
||||
// IP lookup
|
||||
// IP lookup: show the whole JSON answer, its @meta and an error answer
|
||||
// included, as the IP information API card does.
|
||||
document.getElementById('ip-form').addEventListener('submit', function(e) {
|
||||
e.preventDefault();
|
||||
const ip = document.getElementById('ip-input').value.trim();
|
||||
@@ -730,18 +731,11 @@
|
||||
|
||||
fetch('/ip/' + encodeURIComponent(ip))
|
||||
.then(response => response.json())
|
||||
.then(response => {
|
||||
contentEl.classList.remove('loading');
|
||||
if (response.status === 'error') {
|
||||
contentEl.className = 'error';
|
||||
contentEl.textContent = 'Error: ' + response.error.msg;
|
||||
} else {
|
||||
contentEl.className = '';
|
||||
contentEl.textContent = JSON.stringify(response.data, null, 2);
|
||||
}
|
||||
.then(answer => {
|
||||
contentEl.className = '';
|
||||
contentEl.textContent = JSON.stringify(answer, null, 2);
|
||||
})
|
||||
.catch(error => {
|
||||
contentEl.classList.remove('loading');
|
||||
contentEl.className = 'error';
|
||||
contentEl.textContent = 'Error: ' + error.message;
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user