From 144a89cfc234aa010fa3be875ac506188223a070 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 19 Oct 2026 23:43:40 +0000 Subject: [PATCH] Answer 500, not 404, when the IP lookup cannot read the database (closes #109) A failed database read for an address was listed in the answer's errors as if the address had no route, so the lookup answered 404, or 200 next to a routed address. Only a missing route or an invalid address is listed now; any other read error is logged and answers 500 with the JSON error body, as the AS and prefix JSON routes do. Model: opus-5-5 --- internal/server/handlers.go | 29 +++------ internal/server/handlers_internal_test.go | 77 ++++++++++++++++++++++- 2 files changed, 83 insertions(+), 23 deletions(-) diff --git a/internal/server/handlers.go b/internal/server/handlers.go index 7bc4545..c07527c 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -675,10 +675,7 @@ func (s *Server) handleIPInfo() http.HandlerFunc { } ctx := r.Context() - response := IPLookupResponse{ - Query: target, - Results: make([]*database.IPInfo, 0), - } + response := IPLookupResponse{Query: target, Results: make([]*database.IPInfo, 0)} // Collect all IPs to look up ipsToLookup, resolveErrors := s.resolveLookupTargets(ctx, target) @@ -708,11 +705,16 @@ func (s *Server) handleIPInfo() http.HandlerFunc { return } - if err != nil { - response.Errors = append(response.Errors, - ip+": "+s.lookupErrorMessage(ip, err)) + switch { + case errors.Is(err, database.ErrNoRoute), errors.Is(err, database.ErrInvalidIP): + response.Errors = append(response.Errors, ip+": "+err.Error()) continue + case err != nil: + s.logger.Error("Failed to look up IP", "ip", ip, "error", err) + s.writeJSONError(w, http.StatusInternalServerError, internalErrorMessage) + + return } // Do PTR lookup for this IP @@ -751,19 +753,6 @@ func (s *Server) handleIPInfo() http.HandlerFunc { } } -// lookupErrorMessage returns what an IP lookup answer says about err, the -// failed lookup of ip: err's own text when ip is not a valid address or has no -// route, otherwise a fixed message, with err in the log. -func (s *Server) lookupErrorMessage(ip string, err error) string { - if errors.Is(err, database.ErrNoRoute) || errors.Is(err, database.ErrInvalidIP) { - return err.Error() - } - - s.logger.Error("Failed to look up IP", "ip", ip, "error", err) - - return internalErrorMessage -} - // lookupTarget returns what an IP lookup request asks for: the ip or addr URL // parameter, or else the client's own address. func lookupTarget(r *http.Request) string { diff --git a/internal/server/handlers_internal_test.go b/internal/server/handlers_internal_test.go index ed8ee85..de5972f 100644 --- a/internal/server/handlers_internal_test.go +++ b/internal/server/handlers_internal_test.go @@ -806,8 +806,7 @@ func (failingDB) GetPrefixDetailsContext( // the health check with a store whose reads fail with the driver's error text, // and a prefix that does not parse, and checks that no answer carries the // driver's or the prefix parser's text. Before the fix the answers sent it to -// the client (https://git.eeqj.de/sneak/routewatch/issues/108). The IP lookup -// answers 404, as it does whenever no address could be looked up. +// the client (https://git.eeqj.de/sneak/routewatch/issues/108). func TestJSONRoutesDoNotSendErrorDetails(t *testing.T) { t.Parallel() @@ -823,7 +822,7 @@ func TestJSONRoutesDoNotSendErrorDetails(t *testing.T) { {statsPath, http.StatusInternalServerError}, {"/api/v1/as/64500", http.StatusInternalServerError}, {"/api/v1/prefix/192.0.2.0/24", http.StatusInternalServerError}, - {"/api/v1/ip/192.0.2.1", http.StatusNotFound}, + {"/api/v1/ip/192.0.2.1", http.StatusInternalServerError}, {"/api/v1/prefix/foo/24", http.StatusBadRequest}, } @@ -846,6 +845,78 @@ func TestJSONRoutesDoNotSendErrorDetails(t *testing.T) { } } +// The addresses failingIPLookupDB fails the read of and finds a route for. +const ( + failingLookupIP = "192.0.2.1" + routedLookupIP = "192.0.2.2" +) + +// failingIPLookupDB embeds database.Store (left nil) and answers the IP lookup +// with errDiskIO for failingLookupIP, a route for routedLookupIP, and no route +// for any other address. Every other Store method is unused and would panic if +// called. +type failingIPLookupDB struct { + database.Store +} + +func (failingIPLookupDB) GetIPInfoContext( + _ context.Context, ip string, +) (*database.IPInfo, error) { + switch ip { + case failingLookupIP: + return nil, errDiskIO + case routedLookupIP: + return &database.IPInfo{IP: ip}, nil + default: + return nil, database.ErrNoRoute + } +} + +// TestIPLookupAnswers500WhenDatabaseReadFails sends IP lookups through the +// server's router with a store whose read of one address fails with the +// driver's error. A lookup that asks for that address answers 500 with the +// JSON error body, as the AS and prefix JSON routes do; an address with no +// route still answers 404. Before the fix the failed read was reported as if +// the address had no route: 404 for that address alone, and 200 beside a +// routed address (https://git.eeqj.de/sneak/routewatch/issues/109). +func TestIPLookupAnswers500WhenDatabaseReadFails(t *testing.T) { + t.Parallel() + + s := New(failingIPLookupDB{}, streamer.New(logger.New(), metrics.New()), + logger.New(), &config.Config{}) + + tests := []struct { + path string + wantCode int + }{ + { + path: "/api/v1/ip/" + failingLookupIP, + wantCode: http.StatusInternalServerError, + }, + { + path: "/api/v1/ip/" + failingLookupIP + "," + routedLookupIP, + wantCode: http.StatusInternalServerError, + }, + { + path: "/api/v1/ip/203.0.113.1", + wantCode: http.StatusNotFound, + }, + } + + for _, tt := range tests { + rec := httptest.NewRecorder() + s.router.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), http.MethodGet, tt.path, nil)) + + if rec.Code != tt.wantCode { + t.Errorf("%s: status %d, want %d; body %s", + tt.path, rec.Code, tt.wantCode, rec.Body) + } + + checkJSONErrorCode(t, tt.path, rec, tt.wantCode) + } +} + // settledGoroutineCount lets transient goroutines finish, then reports the // current count. func settledGoroutineCount() int { -- 2.54.0