Skip to content

Commit

Permalink
Rename mimirpb.INVALID error cause into mimirpb.UNKNOWN_CAUSE (#6493)
Browse files Browse the repository at this point in the history
* Rename mimirpb.INVALID error cause into mimirpb.UNKNOWN_CAUSE

Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>

* Fixing review findings

Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>

---------

Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
  • Loading branch information
duricanikolic authored Oct 26, 2023
1 parent d1a9165 commit 4f9a9b0
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 151 deletions.
20 changes: 10 additions & 10 deletions pkg/distributor/distributor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4831,25 +4831,25 @@ func TestHandleIngesterPushError(t *testing.T) {
expectedIngesterPushError ingesterPushError
}{
"a gRPC error with details gives an ingesterPushError with the same details": {
ingesterPushError: createStatusWithDetails(t, codes.Unavailable, testErrorMsg, mimirpb.INVALID).Err(),
expectedIngesterPushError: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, testErrorMsg, mimirpb.INVALID)),
ingesterPushError: createStatusWithDetails(t, codes.Unavailable, testErrorMsg, mimirpb.UNKNOWN_CAUSE).Err(),
expectedIngesterPushError: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, testErrorMsg, mimirpb.UNKNOWN_CAUSE)),
},
"a DeadlineExceeded gRPC ingester error an ingesterPushError with INVALID cause": {
"a DeadlineExceeded gRPC ingester error gives an ingesterPushError with UNKNOWN_CAUSE cause": {
// This is how context.DeadlineExceeded error is translated into a gRPC error.
ingesterPushError: status.Error(codes.DeadlineExceeded, context.DeadlineExceeded.Error()),
expectedIngesterPushError: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, context.DeadlineExceeded.Error(), mimirpb.INVALID)),
expectedIngesterPushError: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, context.DeadlineExceeded.Error(), mimirpb.UNKNOWN_CAUSE)),
},
"an Unavailable gRPC error without details gives an ingesterPushError with INVALID cause": {
"an Unavailable gRPC error without details gives an ingesterPushError with UNKNOWN_CAUSE cause": {
ingesterPushError: status.Error(codes.Unavailable, testErrorMsg),
expectedIngesterPushError: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, testErrorMsg, mimirpb.INVALID)),
expectedIngesterPushError: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, testErrorMsg, mimirpb.UNKNOWN_CAUSE)),
},
"an Internal gRPC ingester error without details gives an ingesterPushError with INVALID cause": {
"an Internal gRPC ingester error without details gives an ingesterPushError with UNKNOWN_CAUSE cause": {
ingesterPushError: status.Error(codes.Internal, testErrorMsg),
expectedIngesterPushError: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, testErrorMsg, mimirpb.INVALID)),
expectedIngesterPushError: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, testErrorMsg, mimirpb.UNKNOWN_CAUSE)),
},
"an Unknown gRPC ingester error without details gives an ingesterPushError with INVALID cause": {
"an Unknown gRPC ingester error without details gives an ingesterPushError with UNKNOWN_CAUSE cause": {
ingesterPushError: status.Error(codes.Unknown, testErrorMsg),
expectedIngesterPushError: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, testErrorMsg, mimirpb.INVALID)),
expectedIngesterPushError: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, testErrorMsg, mimirpb.UNKNOWN_CAUSE)),
},
}
for testName, testData := range statusTests {
Expand Down
2 changes: 1 addition & 1 deletion pkg/distributor/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ type ingesterPushError struct {

// newIngesterPushError creates a ingesterPushError error representing the given status object.
func newIngesterPushError(stat *status.Status) ingesterPushError {
errorCause := mimirpb.INVALID
errorCause := mimirpb.UNKNOWN_CAUSE
details := stat.Details()
if len(details) == 1 {
if errorDetails, ok := details[0].(*mimirpb.WriteErrorDetails); ok {
Expand Down
18 changes: 9 additions & 9 deletions pkg/distributor/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,9 @@ func TestNewIngesterPushError(t *testing.T) {
originalStatus: createStatusWithDetails(t, codes.Internal, testMsg, mimirpb.SERVICE_UNAVAILABLE),
expectedCause: mimirpb.SERVICE_UNAVAILABLE,
},
"a gRPC error without details give an ingesterPushError with INVALID cause": {
"a gRPC error without details give an ingesterPushError with UNKNOWN_CAUSE cause": {
originalStatus: status.New(codes.Internal, testMsg),
expectedCause: mimirpb.INVALID,
expectedCause: mimirpb.UNKNOWN_CAUSE,
},
}

Expand Down Expand Up @@ -306,17 +306,17 @@ func TestToGRPCError(t *testing.T) {
expectedErrorMsg: fmt.Sprintf("%s: %s", failedPushingToIngesterMessage, originalMsg),
expectedErrorDetails: &mimirpb.WriteErrorDetails{Cause: mimirpb.TSDB_UNAVAILABLE},
},
"an ingesterPushError with INVALID cause gets translated into a Internal error with INVALID cause": {
err: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, originalMsg, mimirpb.INVALID)),
"an ingesterPushError with UNKNOWN_CAUSE cause gets translated into a Internal error with UNKNOWN_CAUSE cause": {
err: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, originalMsg, mimirpb.UNKNOWN_CAUSE)),
expectedGRPCCode: codes.Internal,
expectedErrorMsg: fmt.Sprintf("%s: %s", failedPushingToIngesterMessage, originalMsg),
expectedErrorDetails: &mimirpb.WriteErrorDetails{Cause: mimirpb.INVALID},
expectedErrorDetails: &mimirpb.WriteErrorDetails{Cause: mimirpb.UNKNOWN_CAUSE},
},
"a DoNotLogError of an ingesterPushError with INVALID cause gets translated into a Internal error with INVALID cause": {
err: log.DoNotLogError{Err: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, originalMsg, mimirpb.INVALID))},
"a DoNotLogError of an ingesterPushError with UNKNOWN_CAUSE cause gets translated into a Internal error with UNKNOWN_CAUSE cause": {
err: log.DoNotLogError{Err: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, originalMsg, mimirpb.UNKNOWN_CAUSE))},
expectedGRPCCode: codes.Internal,
expectedErrorMsg: fmt.Sprintf("%s: %s", failedPushingToIngesterMessage, originalMsg),
expectedErrorDetails: &mimirpb.WriteErrorDetails{Cause: mimirpb.INVALID},
expectedErrorDetails: &mimirpb.WriteErrorDetails{Cause: mimirpb.UNKNOWN_CAUSE},
},
}
for name, tc := range testCases {
Expand Down Expand Up @@ -353,5 +353,5 @@ func (e mockDistributorErr) Error() string {
}

func (e mockDistributorErr) errorCause() mimirpb.ErrorCause {
return mimirpb.INVALID
return mimirpb.UNKNOWN_CAUSE
}
10 changes: 5 additions & 5 deletions pkg/distributor/push_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -934,18 +934,18 @@ func TestHandler_ToHTTPStatus(t *testing.T) {
expectedHTTPStatus: http.StatusInternalServerError,
expectedErrorMsg: fmt.Sprintf("%s: %s", failedPushingToIngesterMessage, originalMsg),
},
"an ingesterPushError with INVALID cause gets translated into an HTTP 500": {
err: newIngesterPushError(createStatusWithDetails(t, codes.Internal, originalMsg, mimirpb.INVALID)),
"an ingesterPushError with UNKNOWN_CAUSE cause gets translated into an HTTP 500": {
err: newIngesterPushError(createStatusWithDetails(t, codes.Internal, originalMsg, mimirpb.UNKNOWN_CAUSE)),
expectedHTTPStatus: http.StatusInternalServerError,
expectedErrorMsg: fmt.Sprintf("%s: %s", failedPushingToIngesterMessage, originalMsg),
},
"a DoNotLogError of an ingesterPushError with INVALID cause gets translated into an HTTP 500": {
err: log.DoNotLogError{Err: newIngesterPushError(createStatusWithDetails(t, codes.Internal, originalMsg, mimirpb.INVALID))},
"a DoNotLogError of an ingesterPushError with UNKNOWN_CAUSE cause gets translated into an HTTP 500": {
err: log.DoNotLogError{Err: newIngesterPushError(createStatusWithDetails(t, codes.Internal, originalMsg, mimirpb.UNKNOWN_CAUSE))},
expectedHTTPStatus: http.StatusInternalServerError,
expectedErrorMsg: fmt.Sprintf("%s: %s", failedPushingToIngesterMessage, originalMsg),
},
"an ingesterPushError obtained from a DeadlineExceeded coming from the ingester gets translated into an HTTP 500": {
err: newIngesterPushError(createStatusWithDetails(t, codes.Internal, context.DeadlineExceeded.Error(), mimirpb.INVALID)),
err: newIngesterPushError(createStatusWithDetails(t, codes.Internal, context.DeadlineExceeded.Error(), mimirpb.UNKNOWN_CAUSE)),
expectedHTTPStatus: http.StatusInternalServerError,
expectedErrorMsg: fmt.Sprintf("%s: %s", failedPushingToIngesterMessage, context.DeadlineExceeded),
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/ingester/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ func (e mockIngesterErr) Error() string {
}

func (e mockIngesterErr) errorCause() mimirpb.ErrorCause {
return mimirpb.INVALID
return mimirpb.UNKNOWN_CAUSE
}

func checkIngesterError(t *testing.T, err error, expectedCause mimirpb.ErrorCause, isSoft bool) {
Expand Down
Loading

0 comments on commit 4f9a9b0

Please sign in to comment.