Skip to content

Commit 2fe624f

Browse files
committed
Fix panic when servers return a wrapped error with status OK
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.
1 parent 642dd63 commit 2fe624f

File tree

2 files changed

+37
-0
lines changed

2 files changed

+37
-0
lines changed

status/status.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,20 @@ func FromError(err error) (s *Status, ok bool) {
9292
}
9393
type grpcstatus interface{ GRPCStatus() *Status }
9494
if gs, ok := err.(grpcstatus); ok {
95+
if gs.GRPCStatus() == nil {
96+
// Error has a status of OK. There is no sensible behavior for this, so map it to Unknown and discard the
97+
// status.
98+
return New(codes.Unknown, err.Error()), false
99+
}
95100
return gs.GRPCStatus(), true
96101
}
97102
var gs grpcstatus
98103
if errors.As(err, &gs) {
104+
if gs.GRPCStatus() == nil {
105+
// Error wraps an error that has an OK status. There is no sensible behavior for this, so map it to Unknown
106+
// and discard the status.
107+
return New(codes.Unknown, err.Error()), false
108+
}
99109
p := gs.GRPCStatus().Proto()
100110
p.Message = err.Error()
101111
return status.FromProto(p), true

status/status_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,33 @@ func (s) TestFromErrorWrapped(t *testing.T) {
202202
}
203203
}
204204

205+
type customErrorNilStatus struct {
206+
}
207+
208+
func (c customErrorNilStatus) Error() string {
209+
return fmt.Sprintf("test")
210+
}
211+
212+
func (c customErrorNilStatus) GRPCStatus() *Status {
213+
return nil
214+
}
215+
216+
func (s) TestFromErrorImplementsInterfaceReturnsOKStatus(t *testing.T) {
217+
err := customErrorNilStatus{}
218+
s, ok := FromError(err)
219+
if ok || s.Code() != codes.Unknown || s.Message() != err.Error() {
220+
t.Fatalf("FromError(%v) = %v, %v; want <Code()=%s, Message()=%q, Err()!=nil>, true", err, s, ok, codes.Unknown, err.Error())
221+
}
222+
}
223+
224+
func (s) TestFromErrorImplementsInterfaceReturnsOKStatusWrapped(t *testing.T) {
225+
err := fmt.Errorf("wrapping: %w", customErrorNilStatus{})
226+
s, ok := FromError(err)
227+
if ok || s.Code() != codes.Unknown || s.Message() != err.Error() {
228+
t.Fatalf("FromError(%v) = %v, %v; want <Code()=%s, Message()=%q, Err()!=nil>, true", err, s, ok, codes.Unknown, err.Error())
229+
}
230+
}
231+
205232
func (s) TestFromErrorImplementsInterfaceWrapped(t *testing.T) {
206233
const code, message = codes.Internal, "test description"
207234
err := fmt.Errorf("wrapped error: %w", customError{Code: code, Message: message})

0 commit comments

Comments
 (0)