uber-go / fx

A dependency injection based application framework for Go.
https://uber-go.github.io/fx/
MIT License
5.83k stars 290 forks source link

panic logging middleware #236

Closed yutongp closed 7 years ago

yutongp commented 7 years ago

not sure if some pre defined lite weight rpc middlewares fall into fx bucket. But almost every service I create, I will need to have a middleware(wrapper) to log panic stack tracer to sentry and return 500 to client.

for example for http I have:

// WithRecovery returns a recovery middleware handle painc.
func WithRecovery() Middleware {
    return func(next http.Handler) http.HandlerFunc {
        return func(w http.ResponseWriter, r *http.Request) {
            defer func() {
                if r := recover(); r != nil {
                    log.WithField("Error", string(debug.Stack()).Errorf("http handler panic: %v", r)
                    http.Error(w, "server_error", http.StatusInternalServerError)
                }
            }()
            next.ServeHTTP(w, r)
        }
    }
}

for tchannel I have:

type recoverableTChanServer struct {
    thrift.TChanServer
}

func (tChanServ *recoverableTChanServer) Handle(ctx thrift.Context, method string, protocol athrift.TProtocol) (ok bool, resp athrift.TStruct, err error) {
    defer func() {
        if r := recover(); r != nil {
            log.WithField("Error", string(debug.Stack())).Errorf("tchannel handler panic: %v. ", r)
            ok = false
            resp = nil
            err = errors.New("server panic")
        }
    }()
    return tChanServ.TChanServer.Handle(ctx, method, protocol)
}

// NewRecoverableServer wraps an existing TChanServer to make it recoverable from panic.
func NewRecoverableServer(serv thrift.TChanServer) thrift.TChanServer {
    return &recoverableTChanServer{TChanServer: serv}
}

It could be nice if this can be included in service framework.

ghost commented 7 years ago

We have it for uhttp: https://github.com/uber-go/fx/blob/master/modules/uhttp/filters.go#L107

And can and definitely can add to yarpc.

ascandella commented 7 years ago

According to @breerly, YARPC already does this as well.

On Fri, Feb 10, 2017 at 10:31 AM Alex notifications@github.com wrote:

We have it for uhttp: https://github.com/uber-go/fx/blob/master/modules/uhttp/filters.go#L107

And can and definitely can add to yarpc.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/uber-go/fx/issues/236#issuecomment-279026188, or mute the thread https://github.com/notifications/unsubscribe-auth/AAF7P4zcSrhoc3pHR5vOIidb9V2BhzXGks5rbKz7gaJpZM4L9s4I .

yutongp commented 7 years ago

I am actually plan to do it for YARPC. Because when I play with our YARPC, it doesn't ask me to pass in a logger.

ascandella commented 7 years ago

Please sync with @breerly

On Fri, Feb 10, 2017 at 11:22 AM Yutong Pei notifications@github.com wrote:

I am actually plan to do it for YARPC. Because when I play with our YARPC, it doesn't ask me to pass in a logger.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/uber-go/fx/issues/236#issuecomment-279040341, or mute the thread https://github.com/notifications/unsubscribe-auth/AAF7P-E1GEwG1qrCkG96kEogkk84vIu3ks5rbLjNgaJpZM4L9s4I .

yutongp commented 7 years ago

240