diff --git a/examples/addsvc/pkg/addtransport/grpc.go b/examples/addsvc/pkg/addtransport/grpc.go index fc3de9d7c..afa3fdabf 100644 --- a/examples/addsvc/pkg/addtransport/grpc.go +++ b/examples/addsvc/pkg/addtransport/grpc.go @@ -18,6 +18,7 @@ import ( "github.com/go-kit/kit/ratelimit" "github.com/go-kit/kit/tracing/opentracing" "github.com/go-kit/kit/tracing/zipkin" + "github.com/go-kit/kit/transport" grpctransport "github.com/go-kit/kit/transport/grpc" "github.com/go-kit/kit/examples/addsvc/pb" @@ -44,7 +45,7 @@ func NewGRPCServer(endpoints addendpoint.Set, otTracer stdopentracing.Tracer, zi zipkinServer := zipkin.GRPCServerTrace(zipkinTracer) options := []grpctransport.ServerOption{ - grpctransport.ServerErrorLogger(logger), + grpctransport.ServerErrorHandler(transport.NewLogErrorHandler(logger)), zipkinServer, } diff --git a/examples/addsvc/pkg/addtransport/http.go b/examples/addsvc/pkg/addtransport/http.go index c1e2b2968..ad7061fd8 100644 --- a/examples/addsvc/pkg/addtransport/http.go +++ b/examples/addsvc/pkg/addtransport/http.go @@ -23,6 +23,7 @@ import ( "github.com/go-kit/kit/ratelimit" "github.com/go-kit/kit/tracing/opentracing" "github.com/go-kit/kit/tracing/zipkin" + "github.com/go-kit/kit/transport" httptransport "github.com/go-kit/kit/transport/http" "github.com/go-kit/kit/examples/addsvc/pkg/addendpoint" @@ -41,7 +42,7 @@ func NewHTTPHandler(endpoints addendpoint.Set, otTracer stdopentracing.Tracer, z options := []httptransport.ServerOption{ httptransport.ServerErrorEncoder(errorEncoder), - httptransport.ServerErrorLogger(logger), + httptransport.ServerErrorHandler(transport.NewLogErrorHandler(logger)), zipkinServer, } diff --git a/examples/profilesvc/transport.go b/examples/profilesvc/transport.go index 6d34126d1..9700dc8cd 100644 --- a/examples/profilesvc/transport.go +++ b/examples/profilesvc/transport.go @@ -14,6 +14,7 @@ import ( "github.com/gorilla/mux" "github.com/go-kit/kit/log" + "github.com/go-kit/kit/transport" httptransport "github.com/go-kit/kit/transport/http" ) @@ -29,7 +30,7 @@ func MakeHTTPHandler(s Service, logger log.Logger) http.Handler { r := mux.NewRouter() e := MakeServerEndpoints(s) options := []httptransport.ServerOption{ - httptransport.ServerErrorLogger(logger), + httptransport.ServerErrorHandler(transport.NewLogErrorHandler(logger)), httptransport.ServerErrorEncoder(encodeError), } diff --git a/examples/shipping/booking/transport.go b/examples/shipping/booking/transport.go index 8cf11a26c..a1e9a10e4 100644 --- a/examples/shipping/booking/transport.go +++ b/examples/shipping/booking/transport.go @@ -10,6 +10,7 @@ import ( "github.com/gorilla/mux" kitlog "github.com/go-kit/kit/log" + "github.com/go-kit/kit/transport" kithttp "github.com/go-kit/kit/transport/http" "github.com/go-kit/kit/examples/shipping/cargo" @@ -19,7 +20,7 @@ import ( // MakeHandler returns a handler for the booking service. func MakeHandler(bs Service, logger kitlog.Logger) http.Handler { opts := []kithttp.ServerOption{ - kithttp.ServerErrorLogger(logger), + kithttp.ServerErrorHandler(transport.NewLogErrorHandler(logger)), kithttp.ServerErrorEncoder(encodeError), } diff --git a/examples/shipping/handling/transport.go b/examples/shipping/handling/transport.go index 0e21365ba..e8a961aee 100644 --- a/examples/shipping/handling/transport.go +++ b/examples/shipping/handling/transport.go @@ -9,6 +9,7 @@ import ( "github.com/gorilla/mux" kitlog "github.com/go-kit/kit/log" + "github.com/go-kit/kit/transport" kithttp "github.com/go-kit/kit/transport/http" "github.com/go-kit/kit/examples/shipping/cargo" @@ -21,7 +22,7 @@ func MakeHandler(hs Service, logger kitlog.Logger) http.Handler { r := mux.NewRouter() opts := []kithttp.ServerOption{ - kithttp.ServerErrorLogger(logger), + kithttp.ServerErrorHandler(transport.NewLogErrorHandler(logger)), kithttp.ServerErrorEncoder(encodeError), } diff --git a/examples/shipping/tracking/transport.go b/examples/shipping/tracking/transport.go index 32db97167..2ea319cf4 100644 --- a/examples/shipping/tracking/transport.go +++ b/examples/shipping/tracking/transport.go @@ -9,6 +9,7 @@ import ( "github.com/gorilla/mux" kitlog "github.com/go-kit/kit/log" + kittransport "github.com/go-kit/kit/transport" kithttp "github.com/go-kit/kit/transport/http" "github.com/go-kit/kit/examples/shipping/cargo" @@ -19,7 +20,7 @@ func MakeHandler(ts Service, logger kitlog.Logger) http.Handler { r := mux.NewRouter() opts := []kithttp.ServerOption{ - kithttp.ServerErrorLogger(logger), + kithttp.ServerErrorHandler(kittransport.NewLogErrorHandler(logger)), kithttp.ServerErrorEncoder(encodeError), } diff --git a/transport/amqp/subscriber.go b/transport/amqp/subscriber.go index 346042a77..2e76c5e4c 100644 --- a/transport/amqp/subscriber.go +++ b/transport/amqp/subscriber.go @@ -7,6 +7,7 @@ import ( "github.com/go-kit/kit/endpoint" "github.com/go-kit/kit/log" + "github.com/go-kit/kit/transport" "github.com/streadway/amqp" ) @@ -19,7 +20,7 @@ type Subscriber struct { after []SubscriberResponseFunc responsePublisher ResponsePublisher errorEncoder ErrorEncoder - logger log.Logger + errorHandler transport.ErrorHandler } // NewSubscriber constructs a new subscriber, which provides a handler @@ -36,7 +37,7 @@ func NewSubscriber( enc: enc, responsePublisher: DefaultResponsePublisher, errorEncoder: DefaultErrorEncoder, - logger: log.NewNopLogger(), + errorHandler: transport.NewLogErrorHandler(log.NewNopLogger()), } for _, option := range options { option(s) @@ -78,8 +79,17 @@ func SubscriberErrorEncoder(ee ErrorEncoder) SubscriberOption { // are logged. This is intended as a diagnostic measure. Finer-grained control // of error handling, including logging in more detail, should be performed in a // custom SubscriberErrorEncoder which has access to the context. +// Deprecated: Use SubscriberErrorHandler instead. func SubscriberErrorLogger(logger log.Logger) SubscriberOption { - return func(s *Subscriber) { s.logger = logger } + return func(s *Subscriber) { s.errorHandler = transport.NewLogErrorHandler(logger) } +} + +// SubscriberErrorHandler is used to handle non-terminal errors. By default, non-terminal errors +// are ignored. This is intended as a diagnostic measure. Finer-grained control +// of error handling, including logging in more detail, should be performed in a +// custom SubscriberErrorEncoder which has access to the context. +func SubscriberErrorHandler(errorHandler transport.ErrorHandler) SubscriberOption { + return func(s *Subscriber) { s.errorHandler = errorHandler } } // ServeDelivery handles AMQP Delivery messages @@ -98,14 +108,14 @@ func (s Subscriber) ServeDelivery(ch Channel) func(deliv *amqp.Delivery) { request, err := s.dec(ctx, deliv) if err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(ctx, err) s.errorEncoder(ctx, err, deliv, ch, &pub) return } response, err := s.e(ctx, request) if err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(ctx, err) s.errorEncoder(ctx, err, deliv, ch, &pub) return } @@ -115,13 +125,13 @@ func (s Subscriber) ServeDelivery(ch Channel) func(deliv *amqp.Delivery) { } if err := s.enc(ctx, &pub, response); err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(ctx, err) s.errorEncoder(ctx, err, deliv, ch, &pub) return } if err := s.responsePublisher(ctx, deliv, ch, &pub); err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(ctx, err) s.errorEncoder(ctx, err, deliv, ch, &pub) return } diff --git a/transport/awslambda/handler.go b/transport/awslambda/handler.go index 1aedb286c..e03e60069 100644 --- a/transport/awslambda/handler.go +++ b/transport/awslambda/handler.go @@ -5,6 +5,7 @@ import ( "github.com/go-kit/kit/endpoint" "github.com/go-kit/kit/log" + "github.com/go-kit/kit/transport" ) // Handler wraps an endpoint. @@ -16,7 +17,7 @@ type Handler struct { after []HandlerResponseFunc errorEncoder ErrorEncoder finalizer []HandlerFinalizerFunc - logger log.Logger + errorHandler transport.ErrorHandler } // NewHandler constructs a new handler, which implements @@ -31,8 +32,8 @@ func NewHandler( e: e, dec: dec, enc: enc, - logger: log.NewNopLogger(), errorEncoder: DefaultErrorEncoder, + errorHandler: transport.NewLogErrorHandler(log.NewNopLogger()), } for _, option := range options { option(h) @@ -57,8 +58,15 @@ func HandlerAfter(after ...HandlerResponseFunc) HandlerOption { // HandlerErrorLogger is used to log non-terminal errors. // By default, no errors are logged. +// Deprecated: Use HandlerErrorHandler instead. func HandlerErrorLogger(logger log.Logger) HandlerOption { - return func(h *Handler) { h.logger = logger } + return func(h *Handler) { h.errorHandler = transport.NewLogErrorHandler(logger) } +} + +// HandlerErrorHandler is used to handle non-terminal errors. +// By default, non-terminal errors are ignored. +func HandlerErrorHandler(errorHandler transport.ErrorHandler) HandlerOption { + return func(h *Handler) { h.errorHandler = errorHandler } } // HandlerErrorEncoder is used to encode errors. @@ -97,13 +105,13 @@ func (h *Handler) Invoke( request, err := h.dec(ctx, payload) if err != nil { - h.logger.Log("err", err) + h.errorHandler.Handle(ctx, err) return h.errorEncoder(ctx, err) } response, err := h.e(ctx, request) if err != nil { - h.logger.Log("err", err) + h.errorHandler.Handle(ctx, err) return h.errorEncoder(ctx, err) } @@ -112,7 +120,7 @@ func (h *Handler) Invoke( } if resp, err = h.enc(ctx, response); err != nil { - h.logger.Log("err", err) + h.errorHandler.Handle(ctx, err) return h.errorEncoder(ctx, err) } diff --git a/transport/awslambda/handler_test.go b/transport/awslambda/handler_test.go index 8add6be19..cf7aaf7f8 100644 --- a/transport/awslambda/handler_test.go +++ b/transport/awslambda/handler_test.go @@ -9,6 +9,7 @@ import ( "github.com/aws/aws-lambda-go/events" "github.com/go-kit/kit/endpoint" "github.com/go-kit/kit/log" + "github.com/go-kit/kit/transport" ) type key int @@ -39,7 +40,7 @@ func TestInvokeHappyPath(t *testing.T) { makeTest01HelloEndpoint(svc), decodeHelloRequestWithTwoBefores, encodeResponse, - HandlerErrorLogger(log.NewNopLogger()), + HandlerErrorHandler(transport.NewLogErrorHandler(log.NewNopLogger())), HandlerBefore(func( ctx context.Context, payload []byte, diff --git a/transport/doc.go b/transport/doc.go index 6ac33ed56..f8382ae71 100644 --- a/transport/doc.go +++ b/transport/doc.go @@ -1,2 +1,2 @@ -// Package transport contains bindings to concrete transports. +// Package transport contains helpers applicable to all supported transports. package transport diff --git a/transport/error_handler.go b/transport/error_handler.go new file mode 100644 index 000000000..1095b9617 --- /dev/null +++ b/transport/error_handler.go @@ -0,0 +1,28 @@ +package transport + +import ( + "context" + + "github.com/go-kit/kit/log" +) + +// ErrorHandler receives a transport error to be processed for diagnostic purposes. +// Usually this means logging the error. +type ErrorHandler interface { + Handle(ctx context.Context, err error) +} + +// LogErrorHandler is a transport error handler implementation which logs an error. +type LogErrorHandler struct { + logger log.Logger +} + +func NewLogErrorHandler(logger log.Logger) *LogErrorHandler { + return &LogErrorHandler{ + logger: logger, + } +} + +func (h *LogErrorHandler) Handle(ctx context.Context, err error) { + h.logger.Log("err", err) +} diff --git a/transport/error_handler_test.go b/transport/error_handler_test.go new file mode 100644 index 000000000..48399a308 --- /dev/null +++ b/transport/error_handler_test.go @@ -0,0 +1,29 @@ +package transport_test + +import ( + "context" + "errors" + "testing" + + "github.com/go-kit/kit/log" + "github.com/go-kit/kit/transport" +) + +func TestLogErrorHandler(t *testing.T) { + var output []interface{} + + logger := log.Logger(log.LoggerFunc(func(keyvals ...interface{}) error { + output = append(output, keyvals...) + return nil + })) + + errorHandler := transport.NewLogErrorHandler(logger) + + err := errors.New("error") + + errorHandler.Handle(context.Background(), err) + + if output[1] != err { + t.Errorf("expected an error log event: have %v, want %v", output[1], err) + } +} diff --git a/transport/grpc/server.go b/transport/grpc/server.go index 7e66c9cc0..8fa629859 100644 --- a/transport/grpc/server.go +++ b/transport/grpc/server.go @@ -8,6 +8,7 @@ import ( "github.com/go-kit/kit/endpoint" "github.com/go-kit/kit/log" + "github.com/go-kit/kit/transport" ) // Handler which should be called from the gRPC binding of the service @@ -19,13 +20,13 @@ type Handler interface { // Server wraps an endpoint and implements grpc.Handler. type Server struct { - e endpoint.Endpoint - dec DecodeRequestFunc - enc EncodeResponseFunc - before []ServerRequestFunc - after []ServerResponseFunc - finalizer []ServerFinalizerFunc - logger log.Logger + e endpoint.Endpoint + dec DecodeRequestFunc + enc EncodeResponseFunc + before []ServerRequestFunc + after []ServerResponseFunc + finalizer []ServerFinalizerFunc + errorHandler transport.ErrorHandler } // NewServer constructs a new server, which implements wraps the provided @@ -40,10 +41,10 @@ func NewServer( options ...ServerOption, ) *Server { s := &Server{ - e: e, - dec: dec, - enc: enc, - logger: log.NewNopLogger(), + e: e, + dec: dec, + enc: enc, + errorHandler: transport.NewLogErrorHandler(log.NewNopLogger()), } for _, option := range options { option(s) @@ -68,8 +69,15 @@ func ServerAfter(after ...ServerResponseFunc) ServerOption { // ServerErrorLogger is used to log non-terminal errors. By default, no errors // are logged. +// Deprecated: Use ServerErrorHandler instead. func ServerErrorLogger(logger log.Logger) ServerOption { - return func(s *Server) { s.logger = logger } + return func(s *Server) { s.errorHandler = transport.NewLogErrorHandler(logger) } +} + +// ServerErrorHandler is used to handle non-terminal errors. By default, non-terminal errors +// are ignored. +func ServerErrorHandler(errorHandler transport.ErrorHandler) ServerOption { + return func(s *Server) { s.errorHandler = errorHandler } } // ServerFinalizer is executed at the end of every gRPC request. @@ -106,13 +114,13 @@ func (s Server) ServeGRPC(ctx context.Context, req interface{}) (retctx context. request, err = s.dec(ctx, req) if err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(ctx, err) return ctx, nil, err } response, err = s.e(ctx, request) if err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(ctx, err) return ctx, nil, err } @@ -123,20 +131,20 @@ func (s Server) ServeGRPC(ctx context.Context, req interface{}) (retctx context. grpcResp, err = s.enc(ctx, response) if err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(ctx, err) return ctx, nil, err } if len(mdHeader) > 0 { if err = grpc.SendHeader(ctx, mdHeader); err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(ctx, err) return ctx, nil, err } } if len(mdTrailer) > 0 { if err = grpc.SetTrailer(ctx, mdTrailer); err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(ctx, err) return ctx, nil, err } } diff --git a/transport/http/server.go b/transport/http/server.go index e01bf870b..34912d072 100644 --- a/transport/http/server.go +++ b/transport/http/server.go @@ -7,6 +7,7 @@ import ( "github.com/go-kit/kit/endpoint" "github.com/go-kit/kit/log" + "github.com/go-kit/kit/transport" ) // Server wraps an endpoint and implements http.Handler. @@ -18,7 +19,7 @@ type Server struct { after []ServerResponseFunc errorEncoder ErrorEncoder finalizer []ServerFinalizerFunc - logger log.Logger + errorHandler transport.ErrorHandler } // NewServer constructs a new server, which implements http.Handler and wraps @@ -34,7 +35,7 @@ func NewServer( dec: dec, enc: enc, errorEncoder: DefaultErrorEncoder, - logger: log.NewNopLogger(), + errorHandler: transport.NewLogErrorHandler(log.NewNopLogger()), } for _, option := range options { option(s) @@ -70,8 +71,18 @@ func ServerErrorEncoder(ee ErrorEncoder) ServerOption { // of error handling, including logging in more detail, should be performed in a // custom ServerErrorEncoder or ServerFinalizer, both of which have access to // the context. +// Deprecated: Use ServerErrorHandler instead. func ServerErrorLogger(logger log.Logger) ServerOption { - return func(s *Server) { s.logger = logger } + return func(s *Server) { s.errorHandler = transport.NewLogErrorHandler(logger) } +} + +// ServerErrorHandler is used to handle non-terminal errors. By default, non-terminal errors +// are ignored. This is intended as a diagnostic measure. Finer-grained control +// of error handling, including logging in more detail, should be performed in a +// custom ServerErrorEncoder or ServerFinalizer, both of which have access to +// the context. +func ServerErrorHandler(errorHandler transport.ErrorHandler) ServerOption { + return func(s *Server) { s.errorHandler = errorHandler } } // ServerFinalizer is executed at the end of every HTTP request. @@ -102,14 +113,14 @@ func (s Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { request, err := s.dec(ctx, r) if err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(ctx, err) s.errorEncoder(ctx, err, w) return } response, err := s.e(ctx, request) if err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(ctx, err) s.errorEncoder(ctx, err, w) return } @@ -119,7 +130,7 @@ func (s Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { } if err := s.enc(ctx, w, response); err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(ctx, err) s.errorEncoder(ctx, err, w) return } diff --git a/transport/nats/subscriber.go b/transport/nats/subscriber.go index 7cb8eb8ec..0cc542f94 100644 --- a/transport/nats/subscriber.go +++ b/transport/nats/subscriber.go @@ -6,6 +6,7 @@ import ( "github.com/go-kit/kit/endpoint" "github.com/go-kit/kit/log" + "github.com/go-kit/kit/transport" "github.com/nats-io/go-nats" ) @@ -19,7 +20,7 @@ type Subscriber struct { after []SubscriberResponseFunc errorEncoder ErrorEncoder finalizer []SubscriberFinalizerFunc - logger log.Logger + errorHandler transport.ErrorHandler } // NewSubscriber constructs a new subscriber, which provides nats.MsgHandler and wraps @@ -35,7 +36,7 @@ func NewSubscriber( dec: dec, enc: enc, errorEncoder: DefaultErrorEncoder, - logger: log.NewNopLogger(), + errorHandler: transport.NewLogErrorHandler(log.NewNopLogger()), } for _, option := range options { option(s) @@ -70,8 +71,17 @@ func SubscriberErrorEncoder(ee ErrorEncoder) SubscriberOption { // are logged. This is intended as a diagnostic measure. Finer-grained control // of error handling, including logging in more detail, should be performed in a // custom SubscriberErrorEncoder which has access to the context. +// Deprecated: Use SubscriberErrorHandler instead. func SubscriberErrorLogger(logger log.Logger) SubscriberOption { - return func(s *Subscriber) { s.logger = logger } + return func(s *Subscriber) { s.errorHandler = transport.NewLogErrorHandler(logger) } +} + +// SubscriberErrorHandler is used to handle non-terminal errors. By default, non-terminal errors +// are ignored. This is intended as a diagnostic measure. Finer-grained control +// of error handling, including logging in more detail, should be performed in a +// custom SubscriberErrorEncoder which has access to the context. +func SubscriberErrorHandler(errorHandler transport.ErrorHandler) SubscriberOption { + return func(s *Subscriber) { s.errorHandler = errorHandler } } // SubscriberFinalizer is executed at the end of every request from a publisher through NATS. @@ -100,7 +110,7 @@ func (s Subscriber) ServeMsg(nc *nats.Conn) func(msg *nats.Msg) { request, err := s.dec(ctx, msg) if err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(ctx, err) if msg.Reply == "" { return } @@ -110,7 +120,7 @@ func (s Subscriber) ServeMsg(nc *nats.Conn) func(msg *nats.Msg) { response, err := s.e(ctx, request) if err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(ctx, err) if msg.Reply == "" { return } @@ -127,7 +137,7 @@ func (s Subscriber) ServeMsg(nc *nats.Conn) func(msg *nats.Msg) { } if err := s.enc(ctx, msg.Reply, nc, response); err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(ctx, err) s.errorEncoder(ctx, err, msg.Reply, nc) return }