Go-micro: why grpc server exec recover(). not return error ?

[aimpsmile]

code in "github.com/micro/[email protected]/server/grpc/grpc.go:340"

    fn := func(ctx context.Context, req server.Request, rsp interface{}) error {
        defer func() {
            if r := recover(); r != nil {
                log.Logf("handler %s panic recovered, err: %s", mtype.method.Name, r)
            }
        }()
        .....

        return nil
    }

I think should return error.
Thanks for answering my question.

[asim]

How would we do that?

[aimpsmile]

Let development handle returns errors.Otherwise, it's hard to tell the difference between no data and errors.
like
fn := func(ctx context.Context, req server.Request, rsp interface{}) (e error) {

        defer func() {
            if r := recover(); r != nil {
                log.Logf("handler %s panic recovered, err: %s", mtype.method.Name, r)
                e = errors.InternalServerError("go.micro.server", "panic recovered: %v", r)
            }
        }()
        ....

        return
    }

[asim]

Feel free to submit a PR

[kid7st]

I think the motivation to recover from the panic is fine (maybe a good idea). Something similar is implemented here: https://github.com/micro/go-micro/blob/master/server/rpc_server.go#L90

I have submitted a PR to improve this: https://github.com/micro/go-micro/pull/906

[kid7st]

I saw the PR has been merged and released. Can we close this issue?

[aimpsmile]

Thanks for your help

[kid7st]

@aimpsmile can you close this issue?

[jaxsongqd]

hi all , i am still confused why just recover() after panic, but not return error?
could you explain more, plz?

[vtolstov]

yes, i think that we need fix that