Skip to content

Commit

Permalink
Move error logging code to error.go (uber-go#457)
Browse files Browse the repository at this point in the history
This gives errors their own file like array.go before we add more
complex handling for error types.

I also moved the tests for errors to error_test to be consistent.

Feel free to close this if you don't want to organize the code this way,
in which case the multierr integration will be in field.go.
  • Loading branch information
abhinav authored and akshayjshah committed Jun 28, 2017
1 parent 02029bc commit 0a01722
Show file tree
Hide file tree
Showing 6 changed files with 179 additions and 91 deletions.
35 changes: 0 additions & 35 deletions array.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
29 changes: 0 additions & 29 deletions array_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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}},
Expand All @@ -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 {
Expand All @@ -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.")
}
80 changes: 80 additions & 0 deletions error.go
Original file line number Diff line number Diff line change
@@ -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
}
99 changes: 99 additions & 0 deletions error_test.go
Original file line number Diff line number Diff line change
@@ -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.")
}
19 changes: 0 additions & 19 deletions field.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions field_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
package zap

import (
"errors"
"net"
"sync"
"testing"
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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)},
Expand Down

0 comments on commit 0a01722

Please sign in to comment.