From 913a66ab4af4155aecf825cee5ad27a45f7aa0c8 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 16 Apr 2019 07:49:03 +0200 Subject: [PATCH 01/14] Add error handler implementation to the log package --- log/error_handler.go | 16 ++++++++++++++++ log/error_handler_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 log/error_handler.go create mode 100644 log/error_handler_test.go diff --git a/log/error_handler.go b/log/error_handler.go new file mode 100644 index 000000000..c021e13e9 --- /dev/null +++ b/log/error_handler.go @@ -0,0 +1,16 @@ +package log + +// ErrorHandler is a transport error handler implementation which logs an error. +type ErrorHandler struct { + logger Logger +} + +func NewErrorHandler(logger Logger) *ErrorHandler { + return &ErrorHandler{ + logger: logger, + } +} + +func (h *ErrorHandler) Handle(err error) { + _ = h.logger.Log("err", err) +} diff --git a/log/error_handler_test.go b/log/error_handler_test.go new file mode 100644 index 000000000..28e4245c6 --- /dev/null +++ b/log/error_handler_test.go @@ -0,0 +1,27 @@ +package log_test + +import ( + "errors" + "testing" + + "github.com/go-kit/kit/log" +) + +func TestErrorHandler(t *testing.T) { + var output []interface{} + + logger := log.Logger(log.LoggerFunc(func(keyvals ...interface{}) error { + output = keyvals + return nil + })) + + errorHandler := log.NewErrorHandler(logger) + + err := errors.New("error") + + errorHandler.Handle(err) + + if output[1] != err { + t.Errorf("expected an error log event: have %v, want %v", output[1], err) + } +} From 1363386db86b2041f1a392893ff5c81ff1787a4f Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 16 Apr 2019 08:00:23 +0200 Subject: [PATCH 02/14] Add error handler to http transport --- transport/http/error_handler.go | 7 +++++++ transport/http/server.go | 26 ++++++++++++++++++++------ 2 files changed, 27 insertions(+), 6 deletions(-) create mode 100644 transport/http/error_handler.go diff --git a/transport/http/error_handler.go b/transport/http/error_handler.go new file mode 100644 index 000000000..c2d0cb132 --- /dev/null +++ b/transport/http/error_handler.go @@ -0,0 +1,7 @@ +package http + +// ErrorHandler receives a transport error to be processed for diagnostic purposes. +// Usually this means logging the error. +type ErrorHandler interface { + Handle(err error) +} diff --git a/transport/http/server.go b/transport/http/server.go index e01bf870b..4b9ffe68a 100644 --- a/transport/http/server.go +++ b/transport/http/server.go @@ -18,7 +18,7 @@ type Server struct { after []ServerResponseFunc errorEncoder ErrorEncoder finalizer []ServerFinalizerFunc - logger log.Logger + errorHandler ErrorHandler } // NewServer constructs a new server, which implements http.Handler and wraps @@ -34,7 +34,7 @@ func NewServer( dec: dec, enc: enc, errorEncoder: DefaultErrorEncoder, - logger: log.NewNopLogger(), + errorHandler: log.NewErrorHandler(log.NewNopLogger()), } for _, option := range options { option(s) @@ -70,8 +70,22 @@ 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) { + if s.errorHandler == nil { + s.errorHandler = log.NewErrorHandler(logger) + } + } +} + +// ServerErrorHandler is used to handle non-terminal errors. By default, no errors +// are handled. 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 ErrorHandler) ServerOption { + return func(s *Server) { s.errorHandler = errorHandler } } // ServerFinalizer is executed at the end of every HTTP request. @@ -102,14 +116,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(err) s.errorEncoder(ctx, err, w) return } response, err := s.e(ctx, request) if err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(err) s.errorEncoder(ctx, err, w) return } @@ -119,7 +133,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(err) s.errorEncoder(ctx, err, w) return } From 9b720a56af6ace549f58d7068d3eb08890d2072f Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 16 Apr 2019 08:06:05 +0200 Subject: [PATCH 03/14] Add error handler to amqp transport --- transport/amqp/error_handler.go | 7 +++++++ transport/amqp/subscriber.go | 27 ++++++++++++++++++++------- 2 files changed, 27 insertions(+), 7 deletions(-) create mode 100644 transport/amqp/error_handler.go diff --git a/transport/amqp/error_handler.go b/transport/amqp/error_handler.go new file mode 100644 index 000000000..b63a4c670 --- /dev/null +++ b/transport/amqp/error_handler.go @@ -0,0 +1,7 @@ +package amqp + +// ErrorHandler receives a transport error to be processed for diagnostic purposes. +// Usually this means logging the error. +type ErrorHandler interface { + Handle(err error) +} diff --git a/transport/amqp/subscriber.go b/transport/amqp/subscriber.go index 346042a77..6f2bee0b1 100644 --- a/transport/amqp/subscriber.go +++ b/transport/amqp/subscriber.go @@ -19,7 +19,7 @@ type Subscriber struct { after []SubscriberResponseFunc responsePublisher ResponsePublisher errorEncoder ErrorEncoder - logger log.Logger + errorHandler ErrorHandler } // NewSubscriber constructs a new subscriber, which provides a handler @@ -36,7 +36,7 @@ func NewSubscriber( enc: enc, responsePublisher: DefaultResponsePublisher, errorEncoder: DefaultErrorEncoder, - logger: log.NewNopLogger(), + errorHandler: log.NewErrorHandler(log.NewNopLogger()), } for _, option := range options { option(s) @@ -78,8 +78,21 @@ 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) { + if s.errorHandler == nil { + s.errorHandler = log.NewErrorHandler(logger) + } + } +} + +// SubscriberErrorHandler is used to handle non-terminal errors. By default, no errors +// are handled. 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 ErrorHandler) SubscriberOption { + return func(s *Subscriber) { s.errorHandler = errorHandler } } // ServeDelivery handles AMQP Delivery messages @@ -98,14 +111,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(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(err) s.errorEncoder(ctx, err, deliv, ch, &pub) return } @@ -115,13 +128,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(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(err) s.errorEncoder(ctx, err, deliv, ch, &pub) return } From d40f4a137b979ccaf91af824248a2ac8799fe987 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 16 Apr 2019 08:12:05 +0200 Subject: [PATCH 04/14] Add error handler to awslambda transport --- transport/awslambda/error_handler.go | 7 +++++++ transport/awslambda/handler.go | 23 +++++++++++++++++------ transport/awslambda/handler_test.go | 2 +- 3 files changed, 25 insertions(+), 7 deletions(-) create mode 100644 transport/awslambda/error_handler.go diff --git a/transport/awslambda/error_handler.go b/transport/awslambda/error_handler.go new file mode 100644 index 000000000..818e67705 --- /dev/null +++ b/transport/awslambda/error_handler.go @@ -0,0 +1,7 @@ +package awslambda + +// ErrorHandler receives a transport error to be processed for diagnostic purposes. +// Usually this means logging the error. +type ErrorHandler interface { + Handle(err error) +} diff --git a/transport/awslambda/handler.go b/transport/awslambda/handler.go index 1aedb286c..5132c8a4e 100644 --- a/transport/awslambda/handler.go +++ b/transport/awslambda/handler.go @@ -16,7 +16,7 @@ type Handler struct { after []HandlerResponseFunc errorEncoder ErrorEncoder finalizer []HandlerFinalizerFunc - logger log.Logger + errorHandler ErrorHandler } // NewHandler constructs a new handler, which implements @@ -31,8 +31,8 @@ func NewHandler( e: e, dec: dec, enc: enc, - logger: log.NewNopLogger(), errorEncoder: DefaultErrorEncoder, + errorHandler: log.NewErrorHandler(log.NewNopLogger()), } for _, option := range options { option(h) @@ -57,8 +57,19 @@ 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) { + if h.errorHandler == nil { + h.errorHandler = log.NewErrorHandler(logger) + } + } +} + +// HandlerErrorHandler is used to handle non-terminal errors. +// By default, no errors are handled. +func HandlerErrorHandler(errorHandler ErrorHandler) HandlerOption { + return func(h *Handler) { h.errorHandler = errorHandler } } // HandlerErrorEncoder is used to encode errors. @@ -97,13 +108,13 @@ func (h *Handler) Invoke( request, err := h.dec(ctx, payload) if err != nil { - h.logger.Log("err", err) + h.errorHandler.Handle(err) return h.errorEncoder(ctx, err) } response, err := h.e(ctx, request) if err != nil { - h.logger.Log("err", err) + h.errorHandler.Handle(err) return h.errorEncoder(ctx, err) } @@ -112,7 +123,7 @@ func (h *Handler) Invoke( } if resp, err = h.enc(ctx, response); err != nil { - h.logger.Log("err", err) + h.errorHandler.Handle(err) return h.errorEncoder(ctx, err) } diff --git a/transport/awslambda/handler_test.go b/transport/awslambda/handler_test.go index 8add6be19..39d4b520a 100644 --- a/transport/awslambda/handler_test.go +++ b/transport/awslambda/handler_test.go @@ -39,7 +39,7 @@ func TestInvokeHappyPath(t *testing.T) { makeTest01HelloEndpoint(svc), decodeHelloRequestWithTwoBefores, encodeResponse, - HandlerErrorLogger(log.NewNopLogger()), + HandlerErrorHandler(log.NewErrorHandler(log.NewNopLogger())), HandlerBefore(func( ctx context.Context, payload []byte, From d04ede968575237d4e144cf696847d421e8ae404 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 16 Apr 2019 08:15:44 +0200 Subject: [PATCH 05/14] Add error handler to grpc transport --- transport/grpc/error_handler.go | 7 +++++ transport/grpc/server.go | 45 ++++++++++++++++++++------------- 2 files changed, 35 insertions(+), 17 deletions(-) create mode 100644 transport/grpc/error_handler.go diff --git a/transport/grpc/error_handler.go b/transport/grpc/error_handler.go new file mode 100644 index 000000000..036441637 --- /dev/null +++ b/transport/grpc/error_handler.go @@ -0,0 +1,7 @@ +package grpc + +// ErrorHandler receives a transport error to be processed for diagnostic purposes. +// Usually this means logging the error. +type ErrorHandler interface { + Handle(err error) +} diff --git a/transport/grpc/server.go b/transport/grpc/server.go index 7e66c9cc0..b73d09d28 100644 --- a/transport/grpc/server.go +++ b/transport/grpc/server.go @@ -19,13 +19,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 ErrorHandler } // NewServer constructs a new server, which implements wraps the provided @@ -40,10 +40,10 @@ func NewServer( options ...ServerOption, ) *Server { s := &Server{ - e: e, - dec: dec, - enc: enc, - logger: log.NewNopLogger(), + e: e, + dec: dec, + enc: enc, + errorHandler: log.NewErrorHandler(log.NewNopLogger()), } for _, option := range options { option(s) @@ -68,8 +68,19 @@ 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) { + if s.errorHandler == nil { + s.errorHandler = log.NewErrorHandler(logger) + } + } +} + +// ServerErrorHandler is used to handle non-terminal errors. By default, no errors +// are handled. +func ServerErrorHandler(errorHandler ErrorHandler) ServerOption { + return func(s *Server) { s.errorHandler = errorHandler } } // ServerFinalizer is executed at the end of every gRPC request. @@ -106,13 +117,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(err) return ctx, nil, err } response, err = s.e(ctx, request) if err != nil { - s.logger.Log("err", err) + s.errorHandler.Handle(err) return ctx, nil, err } @@ -123,20 +134,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(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(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(err) return ctx, nil, err } } From fbb4816f7d18e8adacae834ff5ac1bb2bf4b6f3c Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 16 Apr 2019 08:18:12 +0200 Subject: [PATCH 06/14] Add error handler to nats transport --- transport/nats/error_handler.go | 7 +++++++ transport/nats/subscriber.go | 25 +++++++++++++++++++------ 2 files changed, 26 insertions(+), 6 deletions(-) create mode 100644 transport/nats/error_handler.go diff --git a/transport/nats/error_handler.go b/transport/nats/error_handler.go new file mode 100644 index 000000000..ff851703b --- /dev/null +++ b/transport/nats/error_handler.go @@ -0,0 +1,7 @@ +package nats + +// ErrorHandler receives a transport error to be processed for diagnostic purposes. +// Usually this means logging the error. +type ErrorHandler interface { + Handle(err error) +} diff --git a/transport/nats/subscriber.go b/transport/nats/subscriber.go index 7cb8eb8ec..45e8cf1eb 100644 --- a/transport/nats/subscriber.go +++ b/transport/nats/subscriber.go @@ -19,7 +19,7 @@ type Subscriber struct { after []SubscriberResponseFunc errorEncoder ErrorEncoder finalizer []SubscriberFinalizerFunc - logger log.Logger + errorHandler ErrorHandler } // NewSubscriber constructs a new subscriber, which provides nats.MsgHandler and wraps @@ -35,7 +35,7 @@ func NewSubscriber( dec: dec, enc: enc, errorEncoder: DefaultErrorEncoder, - logger: log.NewNopLogger(), + errorHandler: log.NewErrorHandler(log.NewNopLogger()), } for _, option := range options { option(s) @@ -70,8 +70,21 @@ 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) { + if s.errorHandler == nil { + s.errorHandler = log.NewErrorHandler(logger) + } + } +} + +// SubscriberErrorHandler is used to handle non-terminal errors. By default, no errors +// are handled. 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 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 +113,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(err) if msg.Reply == "" { return } @@ -110,7 +123,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(err) if msg.Reply == "" { return } @@ -127,7 +140,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(err) s.errorEncoder(ctx, err, msg.Reply, nc) return } From 295ec349277418cda79dbad482712fb4de6f2fbf Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 16 Apr 2019 11:47:47 +0200 Subject: [PATCH 07/14] Move error handler interfaces to transport package --- transport/amqp/subscriber.go | 5 +++-- transport/awslambda/error_handler.go | 7 ------- transport/awslambda/handler.go | 5 +++-- transport/{amqp => }/error_handler.go | 2 +- transport/grpc/error_handler.go | 7 ------- transport/grpc/server.go | 5 +++-- transport/http/error_handler.go | 7 ------- transport/http/server.go | 5 +++-- transport/nats/error_handler.go | 7 ------- transport/nats/subscriber.go | 5 +++-- 10 files changed, 16 insertions(+), 39 deletions(-) delete mode 100644 transport/awslambda/error_handler.go rename transport/{amqp => }/error_handler.go (90%) delete mode 100644 transport/grpc/error_handler.go delete mode 100644 transport/http/error_handler.go delete mode 100644 transport/nats/error_handler.go diff --git a/transport/amqp/subscriber.go b/transport/amqp/subscriber.go index 6f2bee0b1..18a0c783d 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 - errorHandler ErrorHandler + errorHandler transport.ErrorHandler } // NewSubscriber constructs a new subscriber, which provides a handler @@ -91,7 +92,7 @@ func SubscriberErrorLogger(logger log.Logger) SubscriberOption { // are handled. 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 ErrorHandler) SubscriberOption { +func SubscriberErrorHandler(errorHandler transport.ErrorHandler) SubscriberOption { return func(s *Subscriber) { s.errorHandler = errorHandler } } diff --git a/transport/awslambda/error_handler.go b/transport/awslambda/error_handler.go deleted file mode 100644 index 818e67705..000000000 --- a/transport/awslambda/error_handler.go +++ /dev/null @@ -1,7 +0,0 @@ -package awslambda - -// ErrorHandler receives a transport error to be processed for diagnostic purposes. -// Usually this means logging the error. -type ErrorHandler interface { - Handle(err error) -} diff --git a/transport/awslambda/handler.go b/transport/awslambda/handler.go index 5132c8a4e..6d8d72fda 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 - errorHandler ErrorHandler + errorHandler transport.ErrorHandler } // NewHandler constructs a new handler, which implements @@ -68,7 +69,7 @@ func HandlerErrorLogger(logger log.Logger) HandlerOption { // HandlerErrorHandler is used to handle non-terminal errors. // By default, no errors are handled. -func HandlerErrorHandler(errorHandler ErrorHandler) HandlerOption { +func HandlerErrorHandler(errorHandler transport.ErrorHandler) HandlerOption { return func(h *Handler) { h.errorHandler = errorHandler } } diff --git a/transport/amqp/error_handler.go b/transport/error_handler.go similarity index 90% rename from transport/amqp/error_handler.go rename to transport/error_handler.go index b63a4c670..e36b3f153 100644 --- a/transport/amqp/error_handler.go +++ b/transport/error_handler.go @@ -1,4 +1,4 @@ -package amqp +package transport // ErrorHandler receives a transport error to be processed for diagnostic purposes. // Usually this means logging the error. diff --git a/transport/grpc/error_handler.go b/transport/grpc/error_handler.go deleted file mode 100644 index 036441637..000000000 --- a/transport/grpc/error_handler.go +++ /dev/null @@ -1,7 +0,0 @@ -package grpc - -// ErrorHandler receives a transport error to be processed for diagnostic purposes. -// Usually this means logging the error. -type ErrorHandler interface { - Handle(err error) -} diff --git a/transport/grpc/server.go b/transport/grpc/server.go index b73d09d28..078e0b4e9 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 @@ -25,7 +26,7 @@ type Server struct { before []ServerRequestFunc after []ServerResponseFunc finalizer []ServerFinalizerFunc - errorHandler ErrorHandler + errorHandler transport.ErrorHandler } // NewServer constructs a new server, which implements wraps the provided @@ -79,7 +80,7 @@ func ServerErrorLogger(logger log.Logger) ServerOption { // ServerErrorHandler is used to handle non-terminal errors. By default, no errors // are handled. -func ServerErrorHandler(errorHandler ErrorHandler) ServerOption { +func ServerErrorHandler(errorHandler transport.ErrorHandler) ServerOption { return func(s *Server) { s.errorHandler = errorHandler } } diff --git a/transport/http/error_handler.go b/transport/http/error_handler.go deleted file mode 100644 index c2d0cb132..000000000 --- a/transport/http/error_handler.go +++ /dev/null @@ -1,7 +0,0 @@ -package http - -// ErrorHandler receives a transport error to be processed for diagnostic purposes. -// Usually this means logging the error. -type ErrorHandler interface { - Handle(err error) -} diff --git a/transport/http/server.go b/transport/http/server.go index 4b9ffe68a..ffd1cc0b2 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 - errorHandler ErrorHandler + errorHandler transport.ErrorHandler } // NewServer constructs a new server, which implements http.Handler and wraps @@ -84,7 +85,7 @@ func ServerErrorLogger(logger log.Logger) 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. -func ServerErrorHandler(errorHandler ErrorHandler) ServerOption { +func ServerErrorHandler(errorHandler transport.ErrorHandler) ServerOption { return func(s *Server) { s.errorHandler = errorHandler } } diff --git a/transport/nats/error_handler.go b/transport/nats/error_handler.go deleted file mode 100644 index ff851703b..000000000 --- a/transport/nats/error_handler.go +++ /dev/null @@ -1,7 +0,0 @@ -package nats - -// ErrorHandler receives a transport error to be processed for diagnostic purposes. -// Usually this means logging the error. -type ErrorHandler interface { - Handle(err error) -} diff --git a/transport/nats/subscriber.go b/transport/nats/subscriber.go index 45e8cf1eb..a79a34e4f 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 - errorHandler ErrorHandler + errorHandler transport.ErrorHandler } // NewSubscriber constructs a new subscriber, which provides nats.MsgHandler and wraps @@ -83,7 +84,7 @@ func SubscriberErrorLogger(logger log.Logger) SubscriberOption { // are handled. 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 ErrorHandler) SubscriberOption { +func SubscriberErrorHandler(errorHandler transport.ErrorHandler) SubscriberOption { return func(s *Subscriber) { s.errorHandler = errorHandler } } From 9085cc5a33568c87a5092309706156c0ef249990 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 16 Apr 2019 11:54:12 +0200 Subject: [PATCH 08/14] Move log error handler to transport package --- log/error_handler.go | 16 ---------------- transport/amqp/subscriber.go | 4 ++-- transport/awslambda/handler.go | 4 ++-- transport/awslambda/handler_test.go | 3 ++- transport/error_handler.go | 19 +++++++++++++++++++ {log => transport}/error_handler_test.go | 9 +++++---- transport/grpc/server.go | 4 ++-- transport/http/server.go | 4 ++-- transport/nats/subscriber.go | 4 ++-- 9 files changed, 36 insertions(+), 31 deletions(-) delete mode 100644 log/error_handler.go rename {log => transport}/error_handler_test.go (64%) diff --git a/log/error_handler.go b/log/error_handler.go deleted file mode 100644 index c021e13e9..000000000 --- a/log/error_handler.go +++ /dev/null @@ -1,16 +0,0 @@ -package log - -// ErrorHandler is a transport error handler implementation which logs an error. -type ErrorHandler struct { - logger Logger -} - -func NewErrorHandler(logger Logger) *ErrorHandler { - return &ErrorHandler{ - logger: logger, - } -} - -func (h *ErrorHandler) Handle(err error) { - _ = h.logger.Log("err", err) -} diff --git a/transport/amqp/subscriber.go b/transport/amqp/subscriber.go index 18a0c783d..fe5eaf34d 100644 --- a/transport/amqp/subscriber.go +++ b/transport/amqp/subscriber.go @@ -37,7 +37,7 @@ func NewSubscriber( enc: enc, responsePublisher: DefaultResponsePublisher, errorEncoder: DefaultErrorEncoder, - errorHandler: log.NewErrorHandler(log.NewNopLogger()), + errorHandler: transport.NewLogErrorHandler(log.NewNopLogger()), } for _, option := range options { option(s) @@ -83,7 +83,7 @@ func SubscriberErrorEncoder(ee ErrorEncoder) SubscriberOption { func SubscriberErrorLogger(logger log.Logger) SubscriberOption { return func(s *Subscriber) { if s.errorHandler == nil { - s.errorHandler = log.NewErrorHandler(logger) + s.errorHandler = transport.NewLogErrorHandler(logger) } } } diff --git a/transport/awslambda/handler.go b/transport/awslambda/handler.go index 6d8d72fda..627b79344 100644 --- a/transport/awslambda/handler.go +++ b/transport/awslambda/handler.go @@ -33,7 +33,7 @@ func NewHandler( dec: dec, enc: enc, errorEncoder: DefaultErrorEncoder, - errorHandler: log.NewErrorHandler(log.NewNopLogger()), + errorHandler: transport.NewLogErrorHandler(log.NewNopLogger()), } for _, option := range options { option(h) @@ -62,7 +62,7 @@ func HandlerAfter(after ...HandlerResponseFunc) HandlerOption { func HandlerErrorLogger(logger log.Logger) HandlerOption { return func(h *Handler) { if h.errorHandler == nil { - h.errorHandler = log.NewErrorHandler(logger) + h.errorHandler = transport.NewLogErrorHandler(logger) } } } diff --git a/transport/awslambda/handler_test.go b/transport/awslambda/handler_test.go index 39d4b520a..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, - HandlerErrorHandler(log.NewErrorHandler(log.NewNopLogger())), + HandlerErrorHandler(transport.NewLogErrorHandler(log.NewNopLogger())), HandlerBefore(func( ctx context.Context, payload []byte, diff --git a/transport/error_handler.go b/transport/error_handler.go index e36b3f153..093bdc3ee 100644 --- a/transport/error_handler.go +++ b/transport/error_handler.go @@ -1,7 +1,26 @@ package transport +import ( + "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(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(err error) { + _ = h.logger.Log("err", err) +} diff --git a/log/error_handler_test.go b/transport/error_handler_test.go similarity index 64% rename from log/error_handler_test.go rename to transport/error_handler_test.go index 28e4245c6..a2597cdca 100644 --- a/log/error_handler_test.go +++ b/transport/error_handler_test.go @@ -1,21 +1,22 @@ -package log_test +package transport_test import ( "errors" "testing" "github.com/go-kit/kit/log" + "github.com/go-kit/kit/transport" ) -func TestErrorHandler(t *testing.T) { +func TestLogErrorHandler(t *testing.T) { var output []interface{} logger := log.Logger(log.LoggerFunc(func(keyvals ...interface{}) error { - output = keyvals + output = append(output, keyvals...) return nil })) - errorHandler := log.NewErrorHandler(logger) + errorHandler := transport.NewLogErrorHandler(logger) err := errors.New("error") diff --git a/transport/grpc/server.go b/transport/grpc/server.go index 078e0b4e9..2d213d108 100644 --- a/transport/grpc/server.go +++ b/transport/grpc/server.go @@ -44,7 +44,7 @@ func NewServer( e: e, dec: dec, enc: enc, - errorHandler: log.NewErrorHandler(log.NewNopLogger()), + errorHandler: transport.NewLogErrorHandler(log.NewNopLogger()), } for _, option := range options { option(s) @@ -73,7 +73,7 @@ func ServerAfter(after ...ServerResponseFunc) ServerOption { func ServerErrorLogger(logger log.Logger) ServerOption { return func(s *Server) { if s.errorHandler == nil { - s.errorHandler = log.NewErrorHandler(logger) + s.errorHandler = transport.NewLogErrorHandler(logger) } } } diff --git a/transport/http/server.go b/transport/http/server.go index ffd1cc0b2..1f1cd610e 100644 --- a/transport/http/server.go +++ b/transport/http/server.go @@ -35,7 +35,7 @@ func NewServer( dec: dec, enc: enc, errorEncoder: DefaultErrorEncoder, - errorHandler: log.NewErrorHandler(log.NewNopLogger()), + errorHandler: transport.NewLogErrorHandler(log.NewNopLogger()), } for _, option := range options { option(s) @@ -75,7 +75,7 @@ func ServerErrorEncoder(ee ErrorEncoder) ServerOption { func ServerErrorLogger(logger log.Logger) ServerOption { return func(s *Server) { if s.errorHandler == nil { - s.errorHandler = log.NewErrorHandler(logger) + s.errorHandler = transport.NewLogErrorHandler(logger) } } } diff --git a/transport/nats/subscriber.go b/transport/nats/subscriber.go index a79a34e4f..7cecb80b7 100644 --- a/transport/nats/subscriber.go +++ b/transport/nats/subscriber.go @@ -36,7 +36,7 @@ func NewSubscriber( dec: dec, enc: enc, errorEncoder: DefaultErrorEncoder, - errorHandler: log.NewErrorHandler(log.NewNopLogger()), + errorHandler: transport.NewLogErrorHandler(log.NewNopLogger()), } for _, option := range options { option(s) @@ -75,7 +75,7 @@ func SubscriberErrorEncoder(ee ErrorEncoder) SubscriberOption { func SubscriberErrorLogger(logger log.Logger) SubscriberOption { return func(s *Subscriber) { if s.errorHandler == nil { - s.errorHandler = log.NewErrorHandler(logger) + s.errorHandler = transport.NewLogErrorHandler(logger) } } } From 66de09e3da8a63dc7213c283c3aa68e20a77f49c Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 16 Apr 2019 11:56:05 +0200 Subject: [PATCH 09/14] Remove error logger precedence --- transport/amqp/subscriber.go | 6 +----- transport/awslambda/handler.go | 6 +----- transport/grpc/server.go | 6 +----- transport/http/server.go | 6 +----- transport/nats/subscriber.go | 6 +----- 5 files changed, 5 insertions(+), 25 deletions(-) diff --git a/transport/amqp/subscriber.go b/transport/amqp/subscriber.go index fe5eaf34d..b80e4a224 100644 --- a/transport/amqp/subscriber.go +++ b/transport/amqp/subscriber.go @@ -81,11 +81,7 @@ func SubscriberErrorEncoder(ee ErrorEncoder) SubscriberOption { // custom SubscriberErrorEncoder which has access to the context. // Deprecated: Use SubscriberErrorHandler instead. func SubscriberErrorLogger(logger log.Logger) SubscriberOption { - return func(s *Subscriber) { - if s.errorHandler == nil { - s.errorHandler = transport.NewLogErrorHandler(logger) - } - } + return func(s *Subscriber) { s.errorHandler = transport.NewLogErrorHandler(logger) } } // SubscriberErrorHandler is used to handle non-terminal errors. By default, no errors diff --git a/transport/awslambda/handler.go b/transport/awslambda/handler.go index 627b79344..b1f0ce672 100644 --- a/transport/awslambda/handler.go +++ b/transport/awslambda/handler.go @@ -60,11 +60,7 @@ func HandlerAfter(after ...HandlerResponseFunc) HandlerOption { // By default, no errors are logged. // Deprecated: Use HandlerErrorHandler instead. func HandlerErrorLogger(logger log.Logger) HandlerOption { - return func(h *Handler) { - if h.errorHandler == nil { - h.errorHandler = transport.NewLogErrorHandler(logger) - } - } + return func(h *Handler) { h.errorHandler = transport.NewLogErrorHandler(logger) } } // HandlerErrorHandler is used to handle non-terminal errors. diff --git a/transport/grpc/server.go b/transport/grpc/server.go index 2d213d108..c4923122e 100644 --- a/transport/grpc/server.go +++ b/transport/grpc/server.go @@ -71,11 +71,7 @@ func ServerAfter(after ...ServerResponseFunc) ServerOption { // are logged. // Deprecated: Use ServerErrorHandler instead. func ServerErrorLogger(logger log.Logger) ServerOption { - return func(s *Server) { - if s.errorHandler == nil { - s.errorHandler = transport.NewLogErrorHandler(logger) - } - } + return func(s *Server) { s.errorHandler = transport.NewLogErrorHandler(logger) } } // ServerErrorHandler is used to handle non-terminal errors. By default, no errors diff --git a/transport/http/server.go b/transport/http/server.go index 1f1cd610e..709fd16bf 100644 --- a/transport/http/server.go +++ b/transport/http/server.go @@ -73,11 +73,7 @@ func ServerErrorEncoder(ee ErrorEncoder) ServerOption { // the context. // Deprecated: Use ServerErrorHandler instead. func ServerErrorLogger(logger log.Logger) ServerOption { - return func(s *Server) { - if s.errorHandler == nil { - s.errorHandler = transport.NewLogErrorHandler(logger) - } - } + return func(s *Server) { s.errorHandler = transport.NewLogErrorHandler(logger) } } // ServerErrorHandler is used to handle non-terminal errors. By default, no errors diff --git a/transport/nats/subscriber.go b/transport/nats/subscriber.go index 7cecb80b7..4145b3491 100644 --- a/transport/nats/subscriber.go +++ b/transport/nats/subscriber.go @@ -73,11 +73,7 @@ func SubscriberErrorEncoder(ee ErrorEncoder) SubscriberOption { // custom SubscriberErrorEncoder which has access to the context. // Deprecated: Use SubscriberErrorHandler instead. func SubscriberErrorLogger(logger log.Logger) SubscriberOption { - return func(s *Subscriber) { - if s.errorHandler == nil { - s.errorHandler = transport.NewLogErrorHandler(logger) - } - } + return func(s *Subscriber) { s.errorHandler = transport.NewLogErrorHandler(logger) } } // SubscriberErrorHandler is used to handle non-terminal errors. By default, no errors From de8921ef4939d18f39b389858c2cf7b5c2969e29 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 16 Apr 2019 12:00:58 +0200 Subject: [PATCH 10/14] Improve documentation wording --- transport/amqp/subscriber.go | 4 ++-- transport/awslambda/handler.go | 2 +- transport/grpc/server.go | 4 ++-- transport/http/server.go | 4 ++-- transport/nats/subscriber.go | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/transport/amqp/subscriber.go b/transport/amqp/subscriber.go index b80e4a224..93189000b 100644 --- a/transport/amqp/subscriber.go +++ b/transport/amqp/subscriber.go @@ -84,8 +84,8 @@ func SubscriberErrorLogger(logger log.Logger) SubscriberOption { return func(s *Subscriber) { s.errorHandler = transport.NewLogErrorHandler(logger) } } -// SubscriberErrorHandler is used to handle non-terminal errors. By default, no errors -// are handled. This is intended as a diagnostic measure. Finer-grained control +// 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 { diff --git a/transport/awslambda/handler.go b/transport/awslambda/handler.go index b1f0ce672..8a912449d 100644 --- a/transport/awslambda/handler.go +++ b/transport/awslambda/handler.go @@ -64,7 +64,7 @@ func HandlerErrorLogger(logger log.Logger) HandlerOption { } // HandlerErrorHandler is used to handle non-terminal errors. -// By default, no errors are handled. +// By default, non-terminal errors are ignored. func HandlerErrorHandler(errorHandler transport.ErrorHandler) HandlerOption { return func(h *Handler) { h.errorHandler = errorHandler } } diff --git a/transport/grpc/server.go b/transport/grpc/server.go index c4923122e..2b44b27fd 100644 --- a/transport/grpc/server.go +++ b/transport/grpc/server.go @@ -74,8 +74,8 @@ func ServerErrorLogger(logger log.Logger) ServerOption { return func(s *Server) { s.errorHandler = transport.NewLogErrorHandler(logger) } } -// ServerErrorHandler is used to handle non-terminal errors. By default, no errors -// are handled. +// 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 } } diff --git a/transport/http/server.go b/transport/http/server.go index 709fd16bf..f2e80926f 100644 --- a/transport/http/server.go +++ b/transport/http/server.go @@ -76,8 +76,8 @@ func ServerErrorLogger(logger log.Logger) ServerOption { return func(s *Server) { s.errorHandler = transport.NewLogErrorHandler(logger) } } -// ServerErrorHandler is used to handle non-terminal errors. By default, no errors -// are handled. This is intended as a diagnostic measure. Finer-grained control +// 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. diff --git a/transport/nats/subscriber.go b/transport/nats/subscriber.go index 4145b3491..ab4275758 100644 --- a/transport/nats/subscriber.go +++ b/transport/nats/subscriber.go @@ -76,8 +76,8 @@ func SubscriberErrorLogger(logger log.Logger) SubscriberOption { return func(s *Subscriber) { s.errorHandler = transport.NewLogErrorHandler(logger) } } -// SubscriberErrorHandler is used to handle non-terminal errors. By default, no errors -// are handled. This is intended as a diagnostic measure. Finer-grained control +// 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 { From 2961581083f87932debff90ec19441c5df155fd8 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 16 Apr 2019 13:39:07 +0200 Subject: [PATCH 11/14] Adjust transport package documentation --- transport/doc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From 13f01e727684bad96bdb304cd6b2ffbb274f7b13 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 16 Apr 2019 13:39:19 +0200 Subject: [PATCH 12/14] Remove ignore error --- transport/error_handler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transport/error_handler.go b/transport/error_handler.go index 093bdc3ee..2935b6475 100644 --- a/transport/error_handler.go +++ b/transport/error_handler.go @@ -22,5 +22,5 @@ func NewLogErrorHandler(logger log.Logger) *LogErrorHandler { } func (h *LogErrorHandler) Handle(err error) { - _ = h.logger.Log("err", err) + h.logger.Log("err", err) } From 38bc66029c3783b8a694a1909f0741794f9598a6 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 16 Apr 2019 13:59:16 +0200 Subject: [PATCH 13/14] Update examples --- examples/addsvc/pkg/addtransport/grpc.go | 3 ++- examples/addsvc/pkg/addtransport/http.go | 3 ++- examples/profilesvc/transport.go | 3 ++- examples/shipping/booking/transport.go | 3 ++- examples/shipping/handling/transport.go | 3 ++- examples/shipping/tracking/transport.go | 3 ++- 6 files changed, 12 insertions(+), 6 deletions(-) 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), } From 087defbe542fd674c4d1159fbf8bd5a3ddd32c4c Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Tue, 16 Apr 2019 14:04:47 +0200 Subject: [PATCH 14/14] Add context to the error handler signature --- transport/amqp/subscriber.go | 8 ++++---- transport/awslambda/handler.go | 6 +++--- transport/error_handler.go | 6 ++++-- transport/error_handler_test.go | 3 ++- transport/grpc/server.go | 10 +++++----- transport/http/server.go | 6 +++--- transport/nats/subscriber.go | 6 +++--- 7 files changed, 24 insertions(+), 21 deletions(-) diff --git a/transport/amqp/subscriber.go b/transport/amqp/subscriber.go index 93189000b..2e76c5e4c 100644 --- a/transport/amqp/subscriber.go +++ b/transport/amqp/subscriber.go @@ -108,14 +108,14 @@ func (s Subscriber) ServeDelivery(ch Channel) func(deliv *amqp.Delivery) { request, err := s.dec(ctx, deliv) if err != nil { - s.errorHandler.Handle(err) + s.errorHandler.Handle(ctx, err) s.errorEncoder(ctx, err, deliv, ch, &pub) return } response, err := s.e(ctx, request) if err != nil { - s.errorHandler.Handle(err) + s.errorHandler.Handle(ctx, err) s.errorEncoder(ctx, err, deliv, ch, &pub) return } @@ -125,13 +125,13 @@ func (s Subscriber) ServeDelivery(ch Channel) func(deliv *amqp.Delivery) { } if err := s.enc(ctx, &pub, response); err != nil { - s.errorHandler.Handle(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.errorHandler.Handle(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 8a912449d..e03e60069 100644 --- a/transport/awslambda/handler.go +++ b/transport/awslambda/handler.go @@ -105,13 +105,13 @@ func (h *Handler) Invoke( request, err := h.dec(ctx, payload) if err != nil { - h.errorHandler.Handle(err) + h.errorHandler.Handle(ctx, err) return h.errorEncoder(ctx, err) } response, err := h.e(ctx, request) if err != nil { - h.errorHandler.Handle(err) + h.errorHandler.Handle(ctx, err) return h.errorEncoder(ctx, err) } @@ -120,7 +120,7 @@ func (h *Handler) Invoke( } if resp, err = h.enc(ctx, response); err != nil { - h.errorHandler.Handle(err) + h.errorHandler.Handle(ctx, err) return h.errorEncoder(ctx, err) } diff --git a/transport/error_handler.go b/transport/error_handler.go index 2935b6475..1095b9617 100644 --- a/transport/error_handler.go +++ b/transport/error_handler.go @@ -1,13 +1,15 @@ 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(err error) + Handle(ctx context.Context, err error) } // LogErrorHandler is a transport error handler implementation which logs an error. @@ -21,6 +23,6 @@ func NewLogErrorHandler(logger log.Logger) *LogErrorHandler { } } -func (h *LogErrorHandler) Handle(err error) { +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 index a2597cdca..48399a308 100644 --- a/transport/error_handler_test.go +++ b/transport/error_handler_test.go @@ -1,6 +1,7 @@ package transport_test import ( + "context" "errors" "testing" @@ -20,7 +21,7 @@ func TestLogErrorHandler(t *testing.T) { err := errors.New("error") - errorHandler.Handle(err) + 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 2b44b27fd..8fa629859 100644 --- a/transport/grpc/server.go +++ b/transport/grpc/server.go @@ -114,13 +114,13 @@ func (s Server) ServeGRPC(ctx context.Context, req interface{}) (retctx context. request, err = s.dec(ctx, req) if err != nil { - s.errorHandler.Handle(err) + s.errorHandler.Handle(ctx, err) return ctx, nil, err } response, err = s.e(ctx, request) if err != nil { - s.errorHandler.Handle(err) + s.errorHandler.Handle(ctx, err) return ctx, nil, err } @@ -131,20 +131,20 @@ func (s Server) ServeGRPC(ctx context.Context, req interface{}) (retctx context. grpcResp, err = s.enc(ctx, response) if err != nil { - s.errorHandler.Handle(err) + s.errorHandler.Handle(ctx, err) return ctx, nil, err } if len(mdHeader) > 0 { if err = grpc.SendHeader(ctx, mdHeader); err != nil { - s.errorHandler.Handle(err) + s.errorHandler.Handle(ctx, err) return ctx, nil, err } } if len(mdTrailer) > 0 { if err = grpc.SetTrailer(ctx, mdTrailer); err != nil { - s.errorHandler.Handle(err) + s.errorHandler.Handle(ctx, err) return ctx, nil, err } } diff --git a/transport/http/server.go b/transport/http/server.go index f2e80926f..34912d072 100644 --- a/transport/http/server.go +++ b/transport/http/server.go @@ -113,14 +113,14 @@ func (s Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { request, err := s.dec(ctx, r) if err != nil { - s.errorHandler.Handle(err) + s.errorHandler.Handle(ctx, err) s.errorEncoder(ctx, err, w) return } response, err := s.e(ctx, request) if err != nil { - s.errorHandler.Handle(err) + s.errorHandler.Handle(ctx, err) s.errorEncoder(ctx, err, w) return } @@ -130,7 +130,7 @@ func (s Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { } if err := s.enc(ctx, w, response); err != nil { - s.errorHandler.Handle(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 ab4275758..0cc542f94 100644 --- a/transport/nats/subscriber.go +++ b/transport/nats/subscriber.go @@ -110,7 +110,7 @@ func (s Subscriber) ServeMsg(nc *nats.Conn) func(msg *nats.Msg) { request, err := s.dec(ctx, msg) if err != nil { - s.errorHandler.Handle(err) + s.errorHandler.Handle(ctx, err) if msg.Reply == "" { return } @@ -120,7 +120,7 @@ func (s Subscriber) ServeMsg(nc *nats.Conn) func(msg *nats.Msg) { response, err := s.e(ctx, request) if err != nil { - s.errorHandler.Handle(err) + s.errorHandler.Handle(ctx, err) if msg.Reply == "" { return } @@ -137,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.errorHandler.Handle(err) + s.errorHandler.Handle(ctx, err) s.errorEncoder(ctx, err, msg.Reply, nc) return }