unistack-org/micro
4
0
Fork 3
Code Issues 20 Pull Requests 2 Actions Packages Projects Releases 288 Wiki Activity

Add recover in grpc service handler (#1727)

Browse Source 
* 🐛 bug(grpc): add missing recover in grpc unkonwn service handler

* Add grpc handler recover testcases

* improve test case

Co-authored-by: ben-toogood <bentoogood@gmail.com>
Co-authored-by: Asim Aslam <asim@aslam.me>
This commit is contained in:
Enix Yu 2020-07-07 03:13:51 +08:00 committed by GitHub
parent 90dca65f55
commit eaa46c2de7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 114 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
server/grpc/grpc.go
View File

@ -183,7 +183,17 @@ func (g *grpcServer) getListener() net.Listener {
return nil
}
func (g *grpcServer) handler(srv interface{}, stream grpc.ServerStream) error {
func (g *grpcServer) handler(srv interface{}, stream grpc.ServerStream) (err error) {
defer func() {
if r := recover(); r != nil {
if logger.V(logger.ErrorLevel, logger.DefaultLogger) {
logger.Error("panic recovered: ", r)
logger.Error(string(debug.Stack()))
}
err = errors.InternalServerError("go.micro.server", "panic recovered: %v", r)
}
}()
if g.wg != nil {
g.wg.Add(1)
defer g.wg.Done()
@ -367,15 +377,6 @@ func (g *grpcServer) processRequest(stream grpc.ServerStream, service *service,
// define the handler func
fn := func(ctx context.Context, req server.Request, rsp interface{}) (err error) {
defer func() {
if r := recover(); r != nil {
if logger.V(logger.ErrorLevel, logger.DefaultLogger) {
logger.Error("panic recovered: ", r)
logger.Error(string(debug.Stack()))
}
err = errors.InternalServerError("go.micro.server", "panic recovered: %v", r)
}
}()
returnValues = function.Call([]reflect.Value{service.rcvr, mtype.prepareContext(ctx), reflect.ValueOf(argv.Interface()), reflect.ValueOf(rsp)})
// The return value for the method is an error.

103
server/grpc/grpc_test.go
View File

@ -60,6 +60,11 @@ func (s *testServer) Call(ctx context.Context, req *pb.Request, rsp *pb.Response
return &errors.Error{Id: "1", Code: 99, Detail: "detail"}
}
if req.Name == "Panic" {
// make it panic
panic("handler panic")
}
rsp.Msg = "Hello " + req.Name
return nil
}
@ -205,3 +210,101 @@ func TestGRPCServer(t *testing.T) {
}
}
}
// TestGRPCServerWithPanicWrapper test grpc server with panic wrapper
// gRPC server should not crash when wrapper crashed
func TestGRPCServerWithPanicWrapper(t *testing.T) {
r := rmemory.NewRegistry()
b := bmemory.NewBroker()
tr := tgrpc.NewTransport()
s := gsrv.NewServer(
server.Broker(b),
server.Name("foo"),
server.Registry(r),
server.Transport(tr),
server.WrapHandler(func(hf server.HandlerFunc) server.HandlerFunc {
return func(ctx context.Context, req server.Request, rsp interface{}) error {
// make it panic
panic("wrapper panic")
}
}),
)
h := &testServer{}
pb.RegisterTestHandler(s, h)
if err := s.Start(); err != nil {
t.Fatalf("failed to start: %v", err)
}
// check registration
services, err := r.GetService("foo")
if err != nil || len(services) == 0 {
t.Fatalf("failed to get service: %v # %d", err, len(services))
}
defer func() {
if err := s.Stop(); err != nil {
t.Fatalf("failed to stop: %v", err)
}
}()
cc, err := grpc.Dial(s.Options().Address, grpc.WithInsecure())
if err != nil {
t.Fatalf("failed to dial server: %v", err)
}
rsp := pb.Response{}
if err := cc.Invoke(context.Background(), "/test.Test/Call", &pb.Request{Name: "John"}, &rsp); err == nil {
t.Fatal("this must return error, as wrapper should be panic")
}
// both wrapper and handler should panic
rsp = pb.Response{}
if err := cc.Invoke(context.Background(), "/test.Test/Call", &pb.Request{Name: "Panic"}, &rsp); err == nil {
t.Fatal("this must return error, as wrapper and handler should be panic")
}
}
// TestGRPCServerWithPanicWrapper test grpc server with panic handler
// gRPC server should not crash when handler crashed
func TestGRPCServerWithPanicHandler(t *testing.T) {
r := rmemory.NewRegistry()
b := bmemory.NewBroker()
tr := tgrpc.NewTransport()
s := gsrv.NewServer(
server.Broker(b),
server.Name("foo"),
server.Registry(r),
server.Transport(tr),
)
h := &testServer{}
pb.RegisterTestHandler(s, h)
if err := s.Start(); err != nil {
t.Fatalf("failed to start: %v", err)
}
// check registration
services, err := r.GetService("foo")
if err != nil || len(services) == 0 {
t.Fatalf("failed to get service: %v # %d", err, len(services))
}
defer func() {
if err := s.Stop(); err != nil {
t.Fatalf("failed to stop: %v", err)
}
}()
cc, err := grpc.Dial(s.Options().Address, grpc.WithInsecure())
if err != nil {
t.Fatalf("failed to dial server: %v", err)
}
rsp := pb.Response{}
if err := cc.Invoke(context.Background(), "/test.Test/Call", &pb.Request{Name: "Panic"}, &rsp); err == nil {
t.Fatal("this must return error, as handler should be panic")
}
}