Skip to content

Commit

Permalink
Don't panic when a given field value is nil
Browse files Browse the repository at this point in the history
This commit ensures that zap.Stringer, zap.Object and zap.Any don't
cause a panic when the given field value is nil.
  • Loading branch information
joa committed Mar 5, 2019
1 parent d2a364d commit 619ba17
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 3 deletions.
22 changes: 19 additions & 3 deletions zapcore/field.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,17 @@ func (f Field) AddTo(enc ObjectEncoder) {

switch f.Type {
case ArrayMarshalerType:
err = enc.AddArray(f.Key, f.Interface.(ArrayMarshaler))
if am, ok := f.Interface.(ArrayMarshaler); ok && am != nil && !isNil(am) {
err = enc.AddArray(f.Key, am)
} else {
enc.AddString(f.Key, "nil")
}
case ObjectMarshalerType:
err = enc.AddObject(f.Key, f.Interface.(ObjectMarshaler))
if om, ok := f.Interface.(ObjectMarshaler); ok && om != nil && !isNil(om) {
err = enc.AddObject(f.Key, om)
} else {
enc.AddString(f.Key, "nil")
}
case BinaryType:
enc.AddBinary(f.Key, f.Interface.([]byte))
case BoolType:
Expand Down Expand Up @@ -160,7 +168,11 @@ func (f Field) AddTo(enc ObjectEncoder) {
case NamespaceType:
enc.OpenNamespace(f.Key)
case StringerType:
enc.AddString(f.Key, f.Interface.(fmt.Stringer).String())
if s, ok := f.Interface.(fmt.Stringer); ok && s != nil && !isNil(s) {
enc.AddString(f.Key, s.String())
} else {
enc.AddString(f.Key, "nil")
}
case ErrorType:
encodeError(f.Key, f.Interface.(error), enc)
case SkipType:
Expand Down Expand Up @@ -199,3 +211,7 @@ func addFields(enc ObjectEncoder, fields []Field) {
fields[i].AddTo(enc)
}
}

func isNil(ti interface{}) bool {
return reflect.ValueOf(ti) == reflect.Zero(reflect.TypeOf(ti))
}
18 changes: 18 additions & 0 deletions zapcore/field_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"errors"
"fmt"
"math"
"net/url"
"testing"
"time"

Expand Down Expand Up @@ -229,3 +230,20 @@ func TestEquals(t *testing.T) {
assert.Equal(t, tt.want, tt.b.Equals(tt.a), "b.Equals(a) a: %#v b: %#v", tt.a, tt.b)
}
}

func TestFieldNilValue(t *testing.T) {
tests := []struct {
t FieldType
iface interface{}
}{
{StringerType, (*url.URL)(nil)},
{ArrayMarshalerType, (ArrayMarshaler)(nil)},
{ObjectMarshalerType, (ObjectMarshaler)(nil)},
}

for _, tt := range tests {
f := Field{Key: "k", Interface: tt.iface, Type: tt.t}
enc := NewMapObjectEncoder()
assert.NotPanics(t, func() { f.AddTo(enc) }, "Unexpected panic when adding fields with nil value.")
}
}

0 comments on commit 619ba17

Please sign in to comment.