Answer 500, not 404, when the IP lookup cannot read the database (closes #109)
check / check (push) Canceled after 0s
check / check (push) Canceled after 0s
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
This commit was merged in pull request #133.
This commit is contained in:
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user