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. `/status.json` and the health check answered chi's empty 405 to any method they do not take. They now answer with the same JSON 405 as the routes under `/api/v1` and `/ip`. 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 was merged in pull request #208.
This commit is contained in:
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user