diff --git a/array.go b/array.go index 784c6df0f..3d4d49f47 100644 --- a/array.go +++ b/array.go @@ -21,16 +21,11 @@ package zap import ( - "sync" "time" "go.uber.org/zap/zapcore" ) -var _errArrayElemPool = sync.Pool{New: func() interface{} { - return &errArrayElem{} -}} - // Array constructs a field with the given key and ArrayMarshaler. It provides // a flexible, but still type-safe and efficient, way to add array-like types // to the logging context. The struct's MarshalLogArray method is called lazily. @@ -323,33 +318,3 @@ func (nums uintptrs) MarshalLogArray(arr zapcore.ArrayEncoder) error { } return nil } - -type errArray []error - -func (errs errArray) MarshalLogArray(arr zapcore.ArrayEncoder) error { - for i := range errs { - if errs[i] == nil { - continue - } - // To represent each error as an object with an "error" attribute and - // potentially an "errorVerbose" attribute, we need to wrap it in a - // type that implements LogObjectMarshaler. To prevent this from - // allocating, pool the wrapper type. - elem := _errArrayElemPool.Get().(*errArrayElem) - elem.error = errs[i] - arr.AppendObject(elem) - elem.error = nil - _errArrayElemPool.Put(elem) - } - return nil -} - -type errArrayElem struct { - error -} - -func (e *errArrayElem) MarshalLogObject(enc zapcore.ObjectEncoder) error { - // Re-use the error field's logic, which supports non-standard error types. - Error(e.error).AddTo(enc) - return nil -} diff --git a/array_test.go b/array_test.go index 8f1bf0a50..df84d9213 100644 --- a/array_test.go +++ b/array_test.go @@ -21,15 +21,12 @@ package zap import ( - "errors" "testing" "time" "go.uber.org/zap/zapcore" - richErrors "github.com/pkg/errors" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func BenchmarkBoolsArrayMarshaler(b *testing.B) { @@ -78,7 +75,6 @@ func TestArrayWrappers(t *testing.T) { {"empty uint16s", Uint16s("", []uint16{}), []interface{}(nil)}, {"empty uint8s", Uint8s("", []uint8{}), []interface{}(nil)}, {"empty uintptrs", Uintptrs("", []uintptr{}), []interface{}(nil)}, - {"empty errors", Errors("", []error{}), []interface{}(nil)}, {"bools", Bools("", []bool{true, false}), []interface{}{true, false}}, {"byte strings", ByteStrings("", [][]byte{{1, 2}, {3, 4}}), []interface{}{[]byte{1, 2}, []byte{3, 4}}}, {"complex128s", Complex128s("", []complex128{1 + 2i, 3 + 4i}), []interface{}{1 + 2i, 3 + 4i}}, @@ -99,11 +95,6 @@ func TestArrayWrappers(t *testing.T) { {"uint16s", Uint16s("", []uint16{1, 2}), []interface{}{uint16(1), uint16(2)}}, {"uint8s", Uint8s("", []uint8{1, 2}), []interface{}{uint8(1), uint8(2)}}, {"uintptrs", Uintptrs("", []uintptr{1, 2}), []interface{}{uintptr(1), uintptr(2)}}, - { - "errors", - Errors("", []error{nil, errors.New("foo"), nil, errors.New("bar")}), - []interface{}{map[string]interface{}{"error": "foo"}, map[string]interface{}{"error": "bar"}}, - }, } for _, tt := range tests { @@ -114,23 +105,3 @@ func TestArrayWrappers(t *testing.T) { assert.Equal(t, 1, len(enc.Fields), "%s: found extra keys in map: %v", tt.desc, enc.Fields) } } - -func TestErrorsArraysHandleRichErrors(t *testing.T) { - errs := []error{richErrors.New("egad")} - - enc := zapcore.NewMapObjectEncoder() - Errors("k", errs).AddTo(enc) - assert.Equal(t, 1, len(enc.Fields), "Expected only top-level field.") - - val := enc.Fields["k"] - arr, ok := val.([]interface{}) - require.True(t, ok, "Expected top-level field to be an array.") - require.Equal(t, 1, len(arr), "Expected only one error object in array.") - - serialized := arr[0] - errMap, ok := serialized.(map[string]interface{}) - require.True(t, ok, "Expected serialized error to be a map, got %T.", serialized) - assert.Equal(t, "egad", errMap["error"], "Unexpected standard error string.") - assert.Contains(t, errMap["errorVerbose"], "egad", "Verbose error string should be a superset of standard error.") - assert.Contains(t, errMap["errorVerbose"], "TestErrorsArraysHandleRichErrors", "Verbose error string should contain a stacktrace.") -} diff --git a/error.go b/error.go new file mode 100644 index 000000000..0ee6182a6 --- /dev/null +++ b/error.go @@ -0,0 +1,80 @@ +// Copyright (c) 2016 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package zap + +import ( + "sync" + + "go.uber.org/zap/zapcore" +) + +var _errArrayElemPool = sync.Pool{New: func() interface{} { + return &errArrayElem{} +}} + +// Error is shorthand for the common idiom NamedError("error", err). +func Error(err error) zapcore.Field { + return NamedError("error", err) +} + +// NamedError constructs a field that lazily stores err.Error() under the +// provided key. Errors which also implement fmt.Formatter (like those produced +// by github.com/pkg/errors) will also have their verbose representation stored +// under key+"Verbose". If passed a nil error, the field is a no-op. +// +// For the common case in which the key is simply "error", the Error function +// is shorter and less repetitive. +func NamedError(key string, err error) zapcore.Field { + if err == nil { + return Skip() + } + return zapcore.Field{Key: key, Type: zapcore.ErrorType, Interface: err} +} + +type errArray []error + +func (errs errArray) MarshalLogArray(arr zapcore.ArrayEncoder) error { + for i := range errs { + if errs[i] == nil { + continue + } + // To represent each error as an object with an "error" attribute and + // potentially an "errorVerbose" attribute, we need to wrap it in a + // type that implements LogObjectMarshaler. To prevent this from + // allocating, pool the wrapper type. + elem := _errArrayElemPool.Get().(*errArrayElem) + elem.error = errs[i] + arr.AppendObject(elem) + elem.error = nil + _errArrayElemPool.Put(elem) + } + return nil +} + +type errArrayElem struct { + error +} + +func (e *errArrayElem) MarshalLogObject(enc zapcore.ObjectEncoder) error { + // Re-use the error field's logic, which supports non-standard error types. + Error(e.error).AddTo(enc) + return nil +} diff --git a/error_test.go b/error_test.go new file mode 100644 index 000000000..93de5952a --- /dev/null +++ b/error_test.go @@ -0,0 +1,99 @@ +// Copyright (c) 2016 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package zap + +import ( + "errors" + "testing" + + "go.uber.org/zap/zapcore" + + richErrors "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestErrorConstructors(t *testing.T) { + fail := errors.New("fail") + + tests := []struct { + name string + field zapcore.Field + expect zapcore.Field + }{ + {"Error", Skip(), Error(nil)}, + {"Error", zapcore.Field{Key: "error", Type: zapcore.ErrorType, Interface: fail}, Error(fail)}, + {"NamedError", Skip(), NamedError("foo", nil)}, + {"NamedError", zapcore.Field{Key: "foo", Type: zapcore.ErrorType, Interface: fail}, NamedError("foo", fail)}, + {"Any:Error", Any("k", errors.New("v")), NamedError("k", errors.New("v"))}, + {"Any:Errors", Any("k", []error{errors.New("v")}), Errors("k", []error{errors.New("v")})}, + } + + for _, tt := range tests { + if !assert.Equal(t, tt.expect, tt.field, "Unexpected output from convenience field constructor %s.", tt.name) { + t.Logf("type expected: %T\nGot: %T", tt.expect.Interface, tt.field.Interface) + } + assertCanBeReused(t, tt.field) + } +} + +func TestErrorArrayConstructor(t *testing.T) { + tests := []struct { + desc string + field zapcore.Field + expected []interface{} + }{ + {"empty errors", Errors("", []error{}), []interface{}(nil)}, + { + "errors", + Errors("", []error{nil, errors.New("foo"), nil, errors.New("bar")}), + []interface{}{map[string]interface{}{"error": "foo"}, map[string]interface{}{"error": "bar"}}, + }, + } + + for _, tt := range tests { + enc := zapcore.NewMapObjectEncoder() + tt.field.Key = "k" + tt.field.AddTo(enc) + assert.Equal(t, tt.expected, enc.Fields["k"], "%s: unexpected map contents.", tt.desc) + assert.Equal(t, 1, len(enc.Fields), "%s: found extra keys in map: %v", tt.desc, enc.Fields) + } +} + +func TestErrorsArraysHandleRichErrors(t *testing.T) { + errs := []error{richErrors.New("egad")} + + enc := zapcore.NewMapObjectEncoder() + Errors("k", errs).AddTo(enc) + assert.Equal(t, 1, len(enc.Fields), "Expected only top-level field.") + + val := enc.Fields["k"] + arr, ok := val.([]interface{}) + require.True(t, ok, "Expected top-level field to be an array.") + require.Equal(t, 1, len(arr), "Expected only one error object in array.") + + serialized := arr[0] + errMap, ok := serialized.(map[string]interface{}) + require.True(t, ok, "Expected serialized error to be a map, got %T.", serialized) + assert.Equal(t, "egad", errMap["error"], "Unexpected standard error string.") + assert.Contains(t, errMap["errorVerbose"], "egad", "Verbose error string should be a superset of standard error.") + assert.Contains(t, errMap["errorVerbose"], "TestErrorsArraysHandleRichErrors", "Verbose error string should contain a stacktrace.") +} diff --git a/field.go b/field.go index c81a13d7f..1592158b6 100644 --- a/field.go +++ b/field.go @@ -178,25 +178,6 @@ func Time(key string, val time.Time) zapcore.Field { return zapcore.Field{Key: key, Type: zapcore.TimeType, Integer: val.UnixNano(), Interface: val.Location()} } -// Error is shorthand for the common idiom NamedError("error", err). -func Error(err error) zapcore.Field { - return NamedError("error", err) -} - -// NamedError constructs a field that lazily stores err.Error() under the -// provided key. Errors which also implement fmt.Formatter (like those produced -// by github.com/pkg/errors) will also have their verbose representation stored -// under key+"Verbose". If passed a nil error, the field is a no-op. -// -// For the common case in which the key is simply "error", the Error function -// is shorter and less repetitive. -func NamedError(key string, err error) zapcore.Field { - if err == nil { - return Skip() - } - return zapcore.Field{Key: key, Type: zapcore.ErrorType, Interface: err} -} - // Stack constructs a field that stores a stacktrace of the current goroutine // under provided key. Keep in mind that taking a stacktrace is eager and // expensive (relatively speaking); this function both makes an allocation and diff --git a/field_test.go b/field_test.go index f19c4ae28..8979073f1 100644 --- a/field_test.go +++ b/field_test.go @@ -21,7 +21,6 @@ package zap import ( - "errors" "net" "sync" "testing" @@ -60,7 +59,6 @@ func assertCanBeReused(t testing.TB, field zapcore.Field) { func TestFieldConstructors(t *testing.T) { // Interface types. - fail := errors.New("fail") addr := net.ParseIP("1.2.3.4") name := username("phil") ints := []int{5, 6} @@ -93,17 +91,11 @@ func TestFieldConstructors(t *testing.T) { {"Uint8", zapcore.Field{Key: "k", Type: zapcore.Uint8Type, Integer: 1}, Uint8("k", 1)}, {"Uintptr", zapcore.Field{Key: "k", Type: zapcore.UintptrType, Integer: 10}, Uintptr("k", 0xa)}, {"Reflect", zapcore.Field{Key: "k", Type: zapcore.ReflectType, Interface: ints}, Reflect("k", ints)}, - {"Error", Skip(), Error(nil)}, - {"Error", zapcore.Field{Key: "error", Type: zapcore.ErrorType, Interface: fail}, Error(fail)}, - {"NamedError", Skip(), NamedError("foo", nil)}, - {"NamedError", zapcore.Field{Key: "foo", Type: zapcore.ErrorType, Interface: fail}, NamedError("foo", fail)}, {"Stringer", zapcore.Field{Key: "k", Type: zapcore.StringerType, Interface: addr}, Stringer("k", addr)}, {"Object", zapcore.Field{Key: "k", Type: zapcore.ObjectMarshalerType, Interface: name}, Object("k", name)}, {"Any:ObjectMarshaler", Any("k", name), Object("k", name)}, {"Any:ArrayMarshaler", Any("k", bools([]bool{true})), Array("k", bools([]bool{true}))}, {"Any:Stringer", Any("k", addr), Stringer("k", addr)}, - {"Any:Error", Any("k", errors.New("v")), NamedError("k", errors.New("v"))}, - {"Any:Errors", Any("k", []error{errors.New("v")}), Errors("k", []error{errors.New("v")})}, {"Any:Bool", Any("k", true), Bool("k", true)}, {"Any:Bools", Any("k", []bool{true}), Bools("k", []bool{true})}, {"Any:Byte", Any("k", byte(1)), Uint8("k", 1)},