diff --git a/pkg/query/storeset.go b/pkg/query/storeset.go index 654cee494b..22abbda239 100644 --- a/pkg/query/storeset.go +++ b/pkg/query/storeset.go @@ -527,8 +527,6 @@ func (s *StoreSet) updateStoreStatus(store *storeRef, err error) { status = *prev } - status.LastError = &stringError{originalErr: err} - if err == nil { status.LastCheck = time.Now() mint, maxt := store.TimeRange() @@ -536,6 +534,9 @@ func (s *StoreSet) updateStoreStatus(store *storeRef, err error) { status.StoreType = store.StoreType() status.MinTime = mint status.MaxTime = maxt + status.LastError = nil + } else { + status.LastError = &stringError{originalErr: err} } s.storeStatuses[store.addr] = &status diff --git a/pkg/query/storeset_test.go b/pkg/query/storeset_test.go index 58813b2dce..0ccc61f9db 100644 --- a/pkg/query/storeset_test.go +++ b/pkg/query/storeset_test.go @@ -781,27 +781,80 @@ func TestStoreSet_Update_Rules(t *testing.T) { } } -type weirdError struct { - originalErr error +type errThatMarshalsToEmptyDict struct { + msg string } -// MarshalJSON marshals the error and returns weird results. -func (e *weirdError) MarshalJSON() ([]byte, error) { +// MarshalJSON marshals the error and returns and empty dict, not the error string. +func (e *errThatMarshalsToEmptyDict) MarshalJSON() ([]byte, error) { return json.Marshal(map[string]string{}) } // Error returns the original, underlying string. -func (e *weirdError) Error() string { - return e.originalErr.Error() +func (e *errThatMarshalsToEmptyDict) Error() string { + return e.msg } +// Test highlights that without wrapping the error, it is marshalled to empty dict {}, not its message. func TestStringError(t *testing.T) { - weirdError := &weirdError{originalErr: errors.New("test")} - properErr := &stringError{originalErr: weirdError} + dictErr := &errThatMarshalsToEmptyDict{msg: "Error message"} + stringErr := &stringError{originalErr: dictErr} + storestatusMock := map[string]error{} - storestatusMock["weird"] = weirdError - storestatusMock["proper"] = properErr + storestatusMock["dictErr"] = dictErr + storestatusMock["stringErr"] = stringErr + b, err := json.Marshal(storestatusMock) + + testutil.Ok(t, err) + testutil.Equals(t, []byte(`{"dictErr":{},"stringErr":"Error message"}`), b, "expected to get proper results") +} + +// Errors that usually marshal to empty dict should return the original error string. +func TestUpdateStoreStateLastError(t *testing.T) { + tcs := []struct { + InputError error + ExpectedLastErr string + }{ + {errors.New("normal_err"), `"normal_err"`}, + {nil, `null`}, + {&errThatMarshalsToEmptyDict{"the error message"}, `"the error message"`}, + } + + for _, tc := range tcs { + mockStoreSet := &StoreSet{ + storeStatuses: map[string]*StoreStatus{}, + } + mockStoreRef := &storeRef{ + addr: "testStore", + } + + mockStoreSet.updateStoreStatus(mockStoreRef, tc.InputError) + + b, err := json.Marshal(mockStoreSet.storeStatuses["testStore"].LastError) + testutil.Ok(t, err) + testutil.Equals(t, tc.ExpectedLastErr, string(b)) + } +} + +func TestUpdateStoreStateForgetsPreviousErrors(t *testing.T) { + mockStoreSet := &StoreSet{ + storeStatuses: map[string]*StoreStatus{}, + } + mockStoreRef := &storeRef{ + addr: "testStore", + } + + mockStoreSet.updateStoreStatus(mockStoreRef, errors.New("test err")) + + b, err := json.Marshal(mockStoreSet.storeStatuses["testStore"].LastError) + testutil.Ok(t, err) + testutil.Equals(t, `"test err"`, string(b)) + + // updating status without and error should clear the previous one. + mockStoreSet.updateStoreStatus(mockStoreRef, nil) + + b, err = json.Marshal(mockStoreSet.storeStatuses["testStore"].LastError) testutil.Ok(t, err) - testutil.Equals(t, []byte(`{"proper":"test","weird":{}}`), b, "expected to get proper results") + testutil.Equals(t, `null`, string(b)) }