From 1c6d99b45538810b3e120bc5a1c29fc019918225 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Du=C5=A1an=20Kasan?= Date: Mon, 2 Jul 2018 21:46:01 +0200 Subject: [PATCH] Add custom error serialization support and provide sane defaults (#78) As per https://github.com/rs/zerolog/issues/9 and to offer a different approach from https://github.com/rs/zerolog/pull/11 and https://github.com/rs/zerolog/pull/35 this PR introduces custom error serialization with sane defaults without breaking the existing APIs. This is just a first draft and is missing tests. Also, a bit of code duplication which I feel could be reduced but it serves to get the idea across. It provides global error marshalling by exposing a `var ErrorMarshalFunc func(error) interface{}` in zerolog package that by default is a function that returns the passed argument. It should be overriden if you require custom error marshalling. Then in every function that accept error or array of errors `ErrorMarshalFunc` is called on the error and then the result of it is processed like this: - if it implements `LogObjectMarshaler`, serialize it as an object - if it is a string serialize as a string - if it is an error, serialize as a string with the result of `Error()` - else serialize it as an interface The side effect of this change is that the encoders don't need the `AppendError/s` methods anymore, as the errors are serialized directly to other types. --- array.go | 19 ++++++++++++-- context.go | 46 ++++++++++++++++++++++++---------- encoder.go | 2 -- event.go | 57 ++++++++++++++++++++++++++++-------------- fields.go | 40 +++++++++++++++++++++++++++-- internal/cbor/base.go | 36 +-------------------------- internal/json/base.go | 36 +-------------------------- log_test.go | 58 +++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 186 insertions(+), 108 deletions(-) diff --git a/array.go b/array.go index 4399c48..01b3eca 100644 --- a/array.go +++ b/array.go @@ -71,9 +71,24 @@ func (a *Array) Hex(val []byte) *Array { return a } -// Err append append the err as a string to the array. +// Err serializes and appends the err to the array. func (a *Array) Err(err error) *Array { - a.buf = enc.AppendError(enc.AppendArrayDelim(a.buf), err) + marshaled := ErrorMarshalFunc(err) + switch m := marshaled.(type) { + case LogObjectMarshaler: + e := newEvent(nil, 0) + e.buf = e.buf[:0] + e.appendObject(m) + a.buf = append(enc.AppendArrayDelim(a.buf), e.buf...) + eventPool.Put(e) + case error: + a.buf = enc.AppendString(enc.AppendArrayDelim(a.buf), m.Error()) + case string: + a.buf = enc.AppendString(enc.AppendArrayDelim(a.buf), m) + default: + a.buf = enc.AppendInterface(enc.AppendArrayDelim(a.buf), m) + } + return a } diff --git a/context.go b/context.go index fd26d5d..9800eb6 100644 --- a/context.go +++ b/context.go @@ -101,27 +101,47 @@ func (c Context) RawJSON(key string, b []byte) Context { return c } -// AnErr adds the field key with err as a string to the logger context. +// AnErr adds the field key with serialized err to the logger context. func (c Context) AnErr(key string, err error) Context { - if err != nil { - c.l.context = enc.AppendError(enc.AppendKey(c.l.context, key), err) + marshaled := ErrorMarshalFunc(err) + switch m := marshaled.(type) { + case nil: + return c + case LogObjectMarshaler: + return c.Object(key,m) + case error: + return c.Str(key, m.Error()) + case string: + return c.Str(key, m) + default: + return c.Interface(key, m) } - return c } -// Errs adds the field key with errs as an array of strings to the logger context. +// Errs adds the field key with errs as an array of serialized errors to the +// logger context. func (c Context) Errs(key string, errs []error) Context { - c.l.context = enc.AppendErrors(enc.AppendKey(c.l.context, key), errs) - return c + arr := Arr() + for _, err := range errs { + marshaled := ErrorMarshalFunc(err) + switch m := marshaled.(type) { + case LogObjectMarshaler: + arr = arr.Object(m) + case error: + arr = arr.Str(m.Error()) + case string: + arr = arr.Str(m) + default: + arr = arr.Interface(m) + } + } + + return c.Array(key, arr) } -// Err adds the field "error" with err as a string to the logger context. -// To customize the key name, change zerolog.ErrorFieldName. +// Err adds the field "error" with serialized err to the logger context. func (c Context) Err(err error) Context { - if err != nil { - c.l.context = enc.AppendError(enc.AppendKey(c.l.context, ErrorFieldName), err) - } - return c + return c.AnErr(ErrorFieldName, err) } // Bool adds the field key with val as a bool to the logger context. diff --git a/encoder.go b/encoder.go index 4bbe27a..09b24e8 100644 --- a/encoder.go +++ b/encoder.go @@ -16,8 +16,6 @@ type encoder interface { AppendDuration(dst []byte, d time.Duration, unit time.Duration, useInt bool) []byte AppendDurations(dst []byte, vals []time.Duration, unit time.Duration, useInt bool) []byte AppendEndMarker(dst []byte) []byte - AppendError(dst []byte, err error) []byte - AppendErrors(dst []byte, errs []error) []byte AppendFloat32(dst []byte, val float32) []byte AppendFloat64(dst []byte, val float64) []byte AppendFloats32(dst []byte, vals []float32) []byte diff --git a/event.go b/event.go index 1e191a3..8a7abf0 100644 --- a/event.go +++ b/event.go @@ -18,6 +18,11 @@ var eventPool = &sync.Pool{ }, } +// ErrorMarshalFunc allows customization of global error marshaling +var ErrorMarshalFunc = func (err error) interface{} { + return err +} + // Event represents a log event. It is instanced by one of the level method of // Logger and finalized by the Msg or Msgf method. type Event struct { @@ -239,39 +244,53 @@ func (e *Event) RawJSON(key string, b []byte) *Event { return e } -// AnErr adds the field key with err as a string to the *Event context. +// AnErr adds the field key with serialized err to the *Event context. // If err is nil, no field is added. func (e *Event) AnErr(key string, err error) *Event { - if e == nil { + marshaled := ErrorMarshalFunc(err) + switch m := marshaled.(type) { + case nil: return e + case LogObjectMarshaler: + return e.Object(key, m) + case error: + return e.Str(key, m.Error()) + case string: + return e.Str(key, m) + default: + return e.Interface(key, m) } - if err != nil { - e.buf = enc.AppendError(enc.AppendKey(e.buf, key), err) - } - return e } - -// Errs adds the field key with errs as an array of strings to the *Event context. -// If err is nil, no field is added. +// Errs adds the field key with errs as an array of serialized errors to the +// *Event context. func (e *Event) Errs(key string, errs []error) *Event { if e == nil { return e } - e.buf = enc.AppendErrors(enc.AppendKey(e.buf, key), errs) - return e + + arr := Arr() + for _, err := range errs { + marshaled := ErrorMarshalFunc(err) + switch m := marshaled.(type) { + case LogObjectMarshaler: + arr = arr.Object(m) + case error: + arr = arr.Err(m) + case string: + arr = arr.Str(m) + default: + arr = arr.Interface(m) + } + } + + return e.Array(key, arr) } -// Err adds the field "error" with err as a string to the *Event context. +// Err adds the field "error" with serialized err to the *Event context. // If err is nil, no field is added. // To customize the key name, change zerolog.ErrorFieldName. func (e *Event) Err(err error) *Event { - if e == nil { - return e - } - if err != nil { - e.buf = enc.AppendError(enc.AppendKey(e.buf, ErrorFieldName), err) - } - return e + return e.AnErr(ErrorFieldName, err) } // Bool adds the field key with val as a bool to the *Event context. diff --git a/fields.go b/fields.go index 25f0d47..336ecbf 100644 --- a/fields.go +++ b/fields.go @@ -29,9 +29,45 @@ func appendFields(dst []byte, fields map[string]interface{}) []byte { case []byte: dst = enc.AppendBytes(dst, val) case error: - dst = enc.AppendError(dst, val) + marshaled := ErrorMarshalFunc(val) + switch m := marshaled.(type) { + case LogObjectMarshaler: + e := newEvent(nil, 0) + e.buf = e.buf[:0] + e.appendObject(m) + dst = append(dst, e.buf...) + eventPool.Put(e) + case error: + dst = enc.AppendString(dst, m.Error()) + case string: + dst = enc.AppendString(dst, m) + default: + dst = enc.AppendInterface(dst, m) + } case []error: - dst = enc.AppendErrors(dst, val) + dst = enc.AppendArrayStart(dst) + for i, err := range val { + marshaled := ErrorMarshalFunc(err) + switch m := marshaled.(type) { + case LogObjectMarshaler: + e := newEvent(nil, 0) + e.buf = e.buf[:0] + e.appendObject(m) + dst = append(dst, e.buf...) + eventPool.Put(e) + case error: + dst = enc.AppendString(dst, m.Error()) + case string: + dst = enc.AppendString(dst, m) + default: + dst = enc.AppendInterface(dst, m) + } + + if i < (len(val) - 1) { + enc.AppendArrayDelim(dst) + } + } + dst = enc.AppendArrayEnd(dst) case bool: dst = enc.AppendBool(dst, val) case int: diff --git a/internal/cbor/base.go b/internal/cbor/base.go index 1b13278..58cd082 100644 --- a/internal/cbor/base.go +++ b/internal/cbor/base.go @@ -8,38 +8,4 @@ func (e Encoder) AppendKey(dst []byte, key string) []byte { dst = e.AppendBeginMarker(dst) } return e.AppendString(dst, key) -} - -// AppendError adds the Error to the log message if error is NOT nil -func (e Encoder) AppendError(dst []byte, err error) []byte { - if err == nil { - return append(dst, `null`...) - } - return e.AppendString(dst, err.Error()) -} - -// AppendErrors when given an array of errors, -// adds them to the log message if a specific error is nil, then -// Nil is added, or else the error string is added. -func (e Encoder) AppendErrors(dst []byte, errs []error) []byte { - if len(errs) == 0 { - return e.AppendArrayEnd(e.AppendArrayStart(dst)) - } - dst = e.AppendArrayStart(dst) - if errs[0] != nil { - dst = e.AppendString(dst, errs[0].Error()) - } else { - dst = e.AppendNil(dst) - } - if len(errs) > 1 { - for _, err := range errs[1:] { - if err == nil { - dst = e.AppendNil(dst) - continue - } - dst = e.AppendString(dst, err.Error()) - } - } - dst = e.AppendArrayEnd(dst) - return dst -} +} \ No newline at end of file diff --git a/internal/json/base.go b/internal/json/base.go index bad1c74..d6f8839 100644 --- a/internal/json/base.go +++ b/internal/json/base.go @@ -9,38 +9,4 @@ func (e Encoder) AppendKey(dst []byte, key string) []byte { } dst = e.AppendString(dst, key) return append(dst, ':') -} - -// AppendError encodes the error string to json and appends -// the encoded string to the input byte slice. -func (e Encoder) AppendError(dst []byte, err error) []byte { - if err == nil { - return append(dst, `null`...) - } - return e.AppendString(dst, err.Error()) -} - -// AppendErrors encodes the error strings to json and -// appends the encoded string list to the input byte slice. -func (e Encoder) AppendErrors(dst []byte, errs []error) []byte { - if len(errs) == 0 { - return append(dst, '[', ']') - } - dst = append(dst, '[') - if errs[0] != nil { - dst = e.AppendString(dst, errs[0].Error()) - } else { - dst = append(dst, "null"...) - } - if len(errs) > 1 { - for _, err := range errs[1:] { - if err == nil { - dst = append(dst, ",null"...) - continue - } - dst = e.AppendString(append(dst, ','), err.Error()) - } - } - dst = append(dst, ']') - return dst -} +} \ No newline at end of file diff --git a/log_test.go b/log_test.go index 1175af2..e013606 100644 --- a/log_test.go +++ b/log_test.go @@ -521,3 +521,61 @@ func TestOutputWithTimestamp(t *testing.T) { t.Errorf("invalid log output:\ngot: %v\nwant: %v", got, want) } } + +type loggableError struct { + error +} + +func (l loggableError) MarshalZerologObject(e *Event) { + e.Str("message", l.error.Error() + ": loggableError") +} + +func TestErrorMarshalFunc(t *testing.T) { + out := &bytes.Buffer{} + log := New(out) + + // test default behaviour + log.Log().Err(errors.New("err")).Msg("msg") + if got, want := decodeIfBinaryToString(out.Bytes()), `{"error":"err","message":"msg"}`+"\n"; got != want { + t.Errorf("invalid log output:\ngot: %v\nwant: %v", got, want) + } + out.Reset() + + log.Log().Err(loggableError{errors.New("err")}).Msg("msg") + if got, want := decodeIfBinaryToString(out.Bytes()), `{"error":{"message":"err: loggableError"},"message":"msg"}`+"\n"; got != want { + t.Errorf("invalid log output:\ngot: %v\nwant: %v", got, want) + } + out.Reset() + + // test overriding the ErrorMarshalFunc + originalErrorMarshalFunc := ErrorMarshalFunc + defer func(){ + ErrorMarshalFunc = originalErrorMarshalFunc + }() + + ErrorMarshalFunc = func(err error) interface{} { + return err.Error() + ": marshaled string" + } + log.Log().Err(errors.New("err")).Msg("msg") + if got, want := decodeIfBinaryToString(out.Bytes()), `{"error":"err: marshaled string","message":"msg"}`+"\n"; got != want { + t.Errorf("invalid log output:\ngot: %v\nwant: %v", got, want) + } + + out.Reset() + ErrorMarshalFunc = func(err error) interface{} { + return errors.New(err.Error() + ": new error") + } + log.Log().Err(errors.New("err")).Msg("msg") + if got, want := decodeIfBinaryToString(out.Bytes()), `{"error":"err: new error","message":"msg"}`+"\n"; got != want { + t.Errorf("invalid log output:\ngot: %v\nwant: %v", got, want) + } + + out.Reset() + ErrorMarshalFunc = func(err error) interface{} { + return loggableError{err} + } + log.Log().Err(errors.New("err")).Msg("msg") + if got, want := decodeIfBinaryToString(out.Bytes()), `{"error":{"message":"err: loggableError"},"message":"msg"}`+"\n"; got != want { + t.Errorf("invalid log output:\ngot: %v\nwant: %v", got, want) + } +}