Skip to content

Commit

Permalink
Show "<nil>" for nil Stringer. (#854)
Browse files Browse the repository at this point in the history
In encodeStringer, instead of returning an error when a panic
occurs when calling String() on a nil pointer, use the string value
"<nil>" like the fmt package does.

It is not always possible to handle this case by fixing the
implementation of String to not panic. This requires implementing
String with a pointer receiver, which doesn't work if you need to
be able to call String on non-addressable values.
  • Loading branch information
andy-retailnext committed Aug 28, 2020
1 parent 2657839 commit 663c590
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
7 changes: 6 additions & 1 deletion zapcore/field.go
Expand Up @@ -208,7 +208,12 @@ func addFields(enc ObjectEncoder, fields []Field) {
func encodeStringer(key string, stringer interface{}, enc ObjectEncoder) (err error) {
defer func() {
if v := recover(); v != nil {
err = fmt.Errorf("PANIC=%v", v)
val := reflect.ValueOf(stringer)
if val.Kind() == reflect.Ptr && val.IsNil() {
enc.AddString(key, "<nil>")
} else {
err = fmt.Errorf("PANIC=%v", v)
}
}
}()

Expand Down
3 changes: 2 additions & 1 deletion zapcore/field_test.go
Expand Up @@ -102,7 +102,6 @@ func TestFieldAddingError(t *testing.T) {
{t: StringerType, iface: &obj{1}, want: empty, err: "PANIC=panic with string"},
{t: StringerType, iface: &obj{2}, want: empty, err: "PANIC=panic with error"},
{t: StringerType, iface: &obj{3}, want: empty, err: "PANIC=<nil>"},
{t: StringerType, iface: (*url.URL)(nil), want: empty, err: "PANIC=runtime error: invalid memory address or nil pointer dereference"},
}
for _, tt := range tests {
f := Field{Key: "k", Interface: tt.iface, Type: tt.t}
Expand Down Expand Up @@ -149,6 +148,8 @@ func TestFields(t *testing.T) {
{t: StringerType, iface: &obj{}, want: "obj"},
{t: StringerType, iface: (*obj)(nil), want: "nil obj"},
{t: SkipType, want: interface{}(nil)},
{t: StringerType, iface: (*url.URL)(nil), want: "<nil>"},
{t: StringerType, iface: (*users)(nil), want: "<nil>"},
}

for _, tt := range tests {
Expand Down

0 comments on commit 663c590

Please sign in to comment.