Skip to content

Commit dc87da6

Browse files
author
Aliaksandr Mianzhynski
authored
recovery: change the default behavior (#439)
1 parent 49d9c0d commit dc87da6

File tree

2 files changed

+22
-9
lines changed

2 files changed

+22
-9
lines changed

interceptors/recovery/interceptors.go

+16-5
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ package recovery
88

99
import (
1010
"context"
11+
"fmt"
12+
"runtime"
1113

1214
"google.golang.org/grpc"
13-
"google.golang.org/grpc/codes"
14-
"google.golang.org/grpc/status"
1515
)
1616

1717
// RecoveryHandlerFunc is a function that recovers from the panic `p` by returning an `error`.
@@ -50,8 +50,19 @@ func StreamServerInterceptor(opts ...Option) grpc.StreamServerInterceptor {
5050
}
5151

5252
func recoverFrom(ctx context.Context, p interface{}, r RecoveryHandlerFuncContext) error {
53-
if r == nil {
54-
return status.Errorf(codes.Internal, "%s", p)
53+
if r != nil {
54+
return r(ctx, p)
5555
}
56-
return r(ctx, p)
56+
stack := make([]byte, 64<<10)
57+
stack = stack[:runtime.Stack(stack, false)]
58+
return &PanicError{Panic: p, Stack: stack}
59+
}
60+
61+
type PanicError struct {
62+
Panic interface{}
63+
Stack []byte
64+
}
65+
66+
func (e *PanicError) Error() string {
67+
return fmt.Sprintf("panic caught: %v\n\n%s", e.Panic, e.Stack)
5768
}

interceptors/recovery/interceptors_test.go

+6-4
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,9 @@ func (s *RecoverySuite) TestUnary_SuccessfulRequest() {
6666
func (s *RecoverySuite) TestUnary_PanickingRequest() {
6767
_, err := s.Client.Ping(s.SimpleCtx(), &testpb.PingRequest{Value: "panic"})
6868
require.Error(s.T(), err, "there must be an error")
69-
assert.Equal(s.T(), codes.Internal, status.Code(err), "must error with internal")
70-
assert.Equal(s.T(), "very bad thing happened", status.Convert(err).Message(), "must error with message")
69+
assert.Equal(s.T(), codes.Unknown, status.Code(err), "must error with unknown")
70+
assert.Contains(s.T(), status.Convert(err).Message(), "panic caught", "must error with message")
71+
assert.Contains(s.T(), status.Convert(err).Message(), "recovery.recoverFrom", "must include stack trace")
7172
}
7273

7374
func (s *RecoverySuite) TestStream_SuccessfulReceive() {
@@ -83,8 +84,9 @@ func (s *RecoverySuite) TestStream_PanickingReceive() {
8384
require.NoError(s.T(), err, "should not fail on establishing the stream")
8485
_, err = stream.Recv()
8586
require.Error(s.T(), err, "there must be an error")
86-
assert.Equal(s.T(), codes.Internal, status.Code(err), "must error with internal")
87-
assert.Equal(s.T(), "very bad thing happened", status.Convert(err).Message(), "must error with message")
87+
assert.Equal(s.T(), codes.Unknown, status.Code(err), "must error with unknown")
88+
assert.Contains(s.T(), status.Convert(err).Message(), "panic caught", "must error with message")
89+
assert.Contains(s.T(), status.Convert(err).Message(), "recovery.recoverFrom", "must include stack trace")
8890
}
8991

9092
func TestRecoveryOverrideSuite(t *testing.T) {

0 commit comments

Comments
 (0)