Skip to content

Commit

Permalink
Fix panic when servers return a wrapped error with status OK
Browse files Browse the repository at this point in the history
When wrapping an error that has a gRPC status, we reuse the underlying status message field and put
the error message in there. If the returned gRPC status is nil, then there is a nil dereference of
the status Message field.

This change fixes this behavior by returning Unknown status and the error message when the wrapped
error has status OK.
  • Loading branch information
atollena committed Jun 15, 2023
1 parent 642dd63 commit 667903b
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 0 deletions.
10 changes: 10 additions & 0 deletions status/status.go
Expand Up @@ -92,10 +92,20 @@ func FromError(err error) (s *Status, ok bool) {
}
type grpcstatus interface{ GRPCStatus() *Status }
if gs, ok := err.(grpcstatus); ok {
if gs.GRPCStatus() == nil {
// Error has a status of OK. There is no sensible behavior for this, so map it to Unknown and discard the
// status.
return New(codes.Unknown, err.Error()), false
}
return gs.GRPCStatus(), true
}
var gs grpcstatus
if errors.As(err, &gs) {
if gs.GRPCStatus() == nil {
// Error wraps an error that has an OK status. There is no sensible behavior for this, so map it to Unknown
// and discard the status.
return New(codes.Unknown, err.Error()), false
}
p := gs.GRPCStatus().Proto()
p.Message = err.Error()
return status.FromProto(p), true
Expand Down
27 changes: 27 additions & 0 deletions status/status_test.go
Expand Up @@ -202,6 +202,33 @@ func (s) TestFromErrorWrapped(t *testing.T) {
}
}

type customErrorNilStatus struct {
}

func (c customErrorNilStatus) Error() string {
return "test"
}

func (c customErrorNilStatus) GRPCStatus() *Status {
return nil
}

func (s) TestFromErrorImplementsInterfaceReturnsOKStatus(t *testing.T) {
err := customErrorNilStatus{}
s, ok := FromError(err)
if ok || s.Code() != codes.Unknown || s.Message() != err.Error() {
t.Fatalf("FromError(%v) = %v, %v; want <Code()=%s, Message()=%q, Err()!=nil>, true", err, s, ok, codes.Unknown, err.Error())
}
}

func (s) TestFromErrorImplementsInterfaceReturnsOKStatusWrapped(t *testing.T) {
err := fmt.Errorf("wrapping: %w", customErrorNilStatus{})
s, ok := FromError(err)
if ok || s.Code() != codes.Unknown || s.Message() != err.Error() {
t.Fatalf("FromError(%v) = %v, %v; want <Code()=%s, Message()=%q, Err()!=nil>, true", err, s, ok, codes.Unknown, err.Error())
}
}

func (s) TestFromErrorImplementsInterfaceWrapped(t *testing.T) {
const code, message = codes.Internal, "test description"
err := fmt.Errorf("wrapped error: %w", customError{Code: code, Message: message})
Expand Down

0 comments on commit 667903b

Please sign in to comment.