Add custom error serialization support and provide sane defaults (rs#78)

As per rs#9 and to offer a different approach from rs#11 and rs#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.
markdessain · Jul 2, 2018 · 1c6d99b · 1c6d99b
1 parent 1a88fbf
commit 1c6d99b
Show file tree

Hide file tree

Showing 8 changed files with 188 additions and 110 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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 {
+	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)
+	}
+}
+// 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
 	}
-	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.
-func (e *Event) Errs(key string, errs []error) *Event {
-	if e == nil {
-		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)
+		}
 	}
-	e.buf = enc.AppendErrors(enc.AppendKey(e.buf, key), errs)
-	return e
+
+	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
@@ -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
@@ -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
-}
+}
diff --git 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
-}
+}
diff --git 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)
+	}
+}