diff --git a/error.go b/error.go index 2bff30d85..0ee6182a6 100644 --- a/error.go +++ b/error.go @@ -1,4 +1,4 @@ -// Copyright (c) 2017 Uber Technologies, Inc. +// 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 diff --git a/error_test.go b/error_test.go index 54ce4bbd7..93de5952a 100644 --- a/error_test.go +++ b/error_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2017 Uber Technologies, Inc. +// 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 diff --git a/glide.lock b/glide.lock index db50355e6..54377a307 100644 --- a/glide.lock +++ b/glide.lock @@ -1,10 +1,10 @@ hash: 2b8be1588d6028696f61ce80eb1b50dbad19144b4d4ead760977ad54064f4f26 -updated: 2017-06-30T10:57:16.454459206-07:00 +updated: 2017-06-27T16:21:01.272833298-07:00 imports: - name: go.uber.org/atomic version: 4e336646b2ef9fc6e47be8e21594178f98e5ebcf - name: go.uber.org/multierr - version: 3c4937480c32f4c13a875a1829af76c98ca3d40a + version: ef109dc7f883f3a99db2d89edc701c25cac40571 testImports: - name: github.com/apex/log version: 8f3a15d95392c8fc202d1e1059f46df21dff2992 @@ -21,7 +21,7 @@ testImports: - name: github.com/fatih/color version: 62e9147c64a1ed519147b62a56a14e83e2be02c1 - name: github.com/go-kit/kit - version: 9917269ff0e9fc93df46474b411ea1716195a61b + version: 0c52d4024a04395dabac42b65e5825d08d3335df subpackages: - log - name: github.com/go-logfmt/logfmt @@ -41,9 +41,9 @@ testImports: - name: github.com/mattn/goveralls version: a2cbbd7cdce4f5e051016fedf639c64bb05ef031 - name: github.com/pborman/uuid - version: 1b00554d822231195d1babd97ff4a781231955c9 + version: e790cca94e6cc75c7064b1332e63811d4aae1a53 - name: github.com/pkg/errors - version: 645ef00459ed84a119197bfb8d8205042c6df63d + version: c605e284fe17294bda444b34710735b29d1a9d90 - name: github.com/pmezard/go-difflib version: 792786c7400a136282c1664665ae0a8db921c6c2 subpackages: @@ -51,7 +51,7 @@ testImports: - name: github.com/satori/go.uuid version: 5bf94b69c6b68ee1b541973bb8e1144db23a194b - name: github.com/sirupsen/logrus - version: 68cec9f21fbf3ea8d8f98c044bc6ce05f17b267a + version: 3d4380f53a34dcdc95f0c1db702615992b38d9a4 - name: github.com/stretchr/testify version: f6abca593680b2315d2075e0f5e2a9751e3f431a subpackages: @@ -60,11 +60,11 @@ testImports: - name: go.pedge.io/lion version: 87958e8713f1fa138d993087133b97e976642159 - name: golang.org/x/sys - version: d4feaf1a7e61e1d9e79e6c4e76c6349e9cab0a03 + version: 90796e5a05ce440b41c768bd9af257005e470461 subpackages: - unix - name: golang.org/x/tools - version: 1529f889eb4b594d1f047f2fb8d5b3cc85c8f006 + version: e6cb469339aef5b7be0c89de730d5f3cc8e47e50 subpackages: - cover - name: gopkg.in/inconshreveable/log15.v2 diff --git a/zapcore/error.go b/zapcore/error.go deleted file mode 100644 index 746bae13b..000000000 --- a/zapcore/error.go +++ /dev/null @@ -1,128 +0,0 @@ -// Copyright (c) 2017 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 zapcore - -import ( - "fmt" - "sync" -) - -// Encodes the given error into fields of an object. A field with the given -// name is added for the error message. -// -// If the error implements fmt.Formatter, a field with the name ${key}Verbose -// is also added with the full verbose error message. -// -// Finally, if the error implements errorGroup (from go.uber.org/multierr) or -// causer (from github.com/pkg/errors), a ${key}Causes field is added with an -// array of objects containing the errors this error was comprised of. -// -// { -// "error": err.Error(), -// "errorVerbose": fmt.Sprintf("%+v", err), -// "errorCauses": [ -// ... -// ], -// } -func encodeError(key string, err error, enc ObjectEncoder) error { - basic := err.Error() - enc.AddString(key, basic) - - if fancy, ok := err.(fmt.Formatter); ok { - verbose := fmt.Sprintf("%+v", fancy) - if verbose != basic { - // This is a rich error type, like those produced by - // github.com/pkg/errors. - enc.AddString(key+"Verbose", verbose) - } - } - - switch e := err.(type) { - case errorGroup: - return enc.AddArray(key+"Causes", errArray(e.Errors())) - case causer: - el := newErrArrayElem(e.Cause()) - err := enc.AddArray(key+"Causes", el) - el.Free() - return err - } - - return nil -} - -type errorGroup interface { - // Provides read-only access to the underlying list of errors, preferably - // without causing any allocs. - Errors() []error -} - -type causer interface { - // Provides access to the error that caused this error. - Cause() error -} - -// Note that errArry and errArrayElem are very similar to the version -// implemented in the top-level error.go file. We can't re-use this because -// that would require exporting errArray as part of the zapcore API. - -// Encodes a list of errors using the standard error encoding logic. -type errArray []error - -func (errs errArray) MarshalLogArray(arr ArrayEncoder) error { - for i := range errs { - if errs[i] == nil { - continue - } - - el := newErrArrayElem(errs[i]) - arr.AppendObject(el) - el.Free() - } - return nil -} - -var _errArrayElemPool = sync.Pool{New: func() interface{} { - return &errArrayElem{} -}} - -// Encodes any error into a {"error": ...} re-using the same errors logic. -// -// May be passed in place of an array to build a single-element array. -type errArrayElem struct{ err error } - -func newErrArrayElem(err error) *errArrayElem { - e := _errArrayElemPool.Get().(*errArrayElem) - e.err = err - return e -} - -func (e *errArrayElem) MarshalLogArray(arr ArrayEncoder) error { - return arr.AppendObject(e) -} - -func (e *errArrayElem) MarshalLogObject(enc ObjectEncoder) error { - return encodeError("error", e.err, enc) -} - -func (e *errArrayElem) Free() { - e.err = nil - _errArrayElemPool.Put(e) -} diff --git a/zapcore/error_test.go b/zapcore/error_test.go deleted file mode 100644 index 21f8e593b..000000000 --- a/zapcore/error_test.go +++ /dev/null @@ -1,227 +0,0 @@ -// Copyright (c) 2017 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 zapcore_test - -import ( - "errors" - "fmt" - "io" - "testing" - - richErrors "github.com/pkg/errors" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "go.uber.org/multierr" - . "go.uber.org/zap/zapcore" -) - -type errTooManyUsers int - -func (e errTooManyUsers) Error() string { - return fmt.Sprintf("%d too many users", int(e)) -} - -func (e errTooManyUsers) Format(s fmt.State, verb rune) { - // Implement fmt.Formatter, but don't add any information beyond the basic - // Error method. - if verb == 'v' && s.Flag('+') { - io.WriteString(s, e.Error()) - } -} - -type customMultierr struct{} - -func (e customMultierr) Error() string { - return "great sadness" -} - -func (e customMultierr) Errors() []error { - return []error{ - errors.New("foo"), - nil, - multierr.Append( - errors.New("bar"), - errors.New("baz"), - ), - } -} - -func TestErrorEncoding(t *testing.T) { - tests := []struct { - k string - t FieldType // defaults to ErrorType - iface interface{} - want map[string]interface{} - }{ - { - k: "k", - iface: errTooManyUsers(2), - want: map[string]interface{}{ - "k": "2 too many users", - }, - }, - { - k: "err", - iface: multierr.Combine( - errors.New("foo"), - errors.New("bar"), - errors.New("baz"), - ), - want: map[string]interface{}{ - "err": "foo; bar; baz", - "errVerbose": "the following errors occurred:\n" + - " - foo\n" + - " - bar\n" + - " - baz", - "errCauses": []interface{}{ - map[string]interface{}{"error": "foo"}, - map[string]interface{}{"error": "bar"}, - map[string]interface{}{"error": "baz"}, - }, - }, - }, - { - k: "e", - iface: customMultierr{}, - want: map[string]interface{}{ - "e": "great sadness", - "eCauses": []interface{}{ - map[string]interface{}{"error": "foo"}, - map[string]interface{}{ - "error": "bar; baz", - "errorVerbose": "the following errors occurred:\n" + - " - bar\n" + - " - baz", - "errorCauses": []interface{}{ - map[string]interface{}{"error": "bar"}, - map[string]interface{}{"error": "baz"}, - }, - }, - }, - }, - }, - { - k: "k", - iface: richErrors.WithMessage(errors.New("egad"), "failed"), - want: map[string]interface{}{ - "k": "failed: egad", - "kVerbose": "egad\n" + - "failed", - "kCauses": []interface{}{ - map[string]interface{}{ - "error": "egad", - }, - }, - }, - }, - { - k: "error", - iface: multierr.Combine( - richErrors.WithMessage( - multierr.Combine(errors.New("foo"), errors.New("bar")), - "hello", - ), - errors.New("baz"), - richErrors.WithMessage(errors.New("qux"), "world"), - ), - want: map[string]interface{}{ - "error": "hello: foo; bar; baz; world: qux", - "errorVerbose": "the following errors occurred:\n" + - " - the following errors occurred:\n" + - " - foo\n" + - " - bar\n" + - " hello\n" + - " - baz\n" + - " - qux\n" + - " world", - "errorCauses": []interface{}{ - map[string]interface{}{ - "error": "hello: foo; bar", - "errorVerbose": "the following errors occurred:\n" + - " - foo\n" + - " - bar\n" + - "hello", - "errorCauses": []interface{}{ - map[string]interface{}{ - "error": "foo; bar", - "errorVerbose": "the following errors occurred:\n" + - " - foo\n" + - " - bar", - "errorCauses": []interface{}{ - map[string]interface{}{"error": "foo"}, - map[string]interface{}{"error": "bar"}, - }, - }, - }, - }, - map[string]interface{}{"error": "baz"}, - map[string]interface{}{ - "error": "world: qux", - "errorVerbose": "qux\nworld", - "errorCauses": []interface{}{ - map[string]interface{}{"error": "qux"}, - }, - }, - }, - }, - }, - } - - for _, tt := range tests { - if tt.t == UnknownType { - tt.t = ErrorType - } - - enc := NewMapObjectEncoder() - f := Field{Key: tt.k, Type: tt.t, Interface: tt.iface} - f.AddTo(enc) - assert.Equal(t, tt.want, enc.Fields, "Unexpected output from field %+v.", f) - } -} - -func TestRichErrorSupport(t *testing.T) { - f := Field{ - Type: ErrorType, - Interface: richErrors.WithMessage(richErrors.New("egad"), "failed"), - Key: "k", - } - enc := NewMapObjectEncoder() - f.AddTo(enc) - assert.Equal(t, "failed: egad", enc.Fields["k"], "Unexpected basic error message.") - - serialized := enc.Fields["kVerbose"] - // Don't assert the exact format used by a third-party package, but ensure - // that some critical elements are present. - assert.Regexp(t, `egad`, serialized, "Expected original error message to be present.") - assert.Regexp(t, `failed`, serialized, "Expected error annotation to be present.") - assert.Regexp(t, `TestRichErrorSupport`, serialized, "Expected calling function to be present in stacktrace.") - - arr, ok := enc.Fields["kCauses"].([]interface{}) - require.True(t, ok, "Expected causes array to be present") - require.Len(t, arr, 1, "Expected a single cause") - - cause, ok := arr[0].(map[string]interface{}) - require.True(t, ok, "Expected cause object") - assert.Equal(t, "egad", cause["error"], "Expected error message to match") - assert.Regexp(t, `egad`, cause["errorVerbose"], "Expected verbose message to match") - assert.Regexp(t, `TestRichErrorSupport`, cause["errorVerbose"], "Expected verbose message to match") -} diff --git a/zapcore/field.go b/zapcore/field.go index 6a5e33e2f..4f722008f 100644 --- a/zapcore/field.go +++ b/zapcore/field.go @@ -162,7 +162,17 @@ func (f Field) AddTo(enc ObjectEncoder) { case StringerType: enc.AddString(f.Key, f.Interface.(fmt.Stringer).String()) case ErrorType: - encodeError(f.Key, f.Interface.(error), enc) + val := f.Interface.(error) + basic := val.Error() + enc.AddString(f.Key, basic) + if fancy, ok := val.(fmt.Formatter); ok { + verbose := fmt.Sprintf("%+v", fancy) + if verbose != basic { + // This is a rich error type, like those produced by + // github.com/pkg/errors. + enc.AddString(f.Key+"Verbose", verbose) + } + } case SkipType: break default: diff --git a/zapcore/field_test.go b/zapcore/field_test.go index 2d8a45f9e..2251099e7 100644 --- a/zapcore/field_test.go +++ b/zapcore/field_test.go @@ -23,12 +23,14 @@ package zapcore_test import ( "errors" "fmt" + "io" "math" "testing" "time" "go.uber.org/zap" + richErrors "github.com/pkg/errors" "github.com/stretchr/testify/assert" . "go.uber.org/zap/zapcore" @@ -40,6 +42,18 @@ func (u users) String() string { return fmt.Sprintf("%d users", int(u)) } +func (u users) Error() string { + return fmt.Sprintf("%d too many users", int(u)) +} + +func (u users) Format(s fmt.State, verb rune) { + // Implement fmt.Formatter, but don't add any information beyond the basic + // Error method. + if verb == 'v' && s.Flag('+') { + io.WriteString(s, u.Error()) + } +} + func (u users) MarshalLogObject(enc ObjectEncoder) error { if int(u) < 0 { return errors.New("too few users") @@ -116,6 +130,7 @@ func TestFields(t *testing.T) { {t: ReflectType, iface: users(2), want: users(2)}, {t: NamespaceType, want: map[string]interface{}{}}, {t: StringerType, iface: users(2), want: "2 users"}, + {t: ErrorType, iface: users(2), want: "2 too many users"}, {t: SkipType, want: interface{}(nil)}, } @@ -132,6 +147,24 @@ func TestFields(t *testing.T) { } } +func TestRichErrorSupport(t *testing.T) { + f := Field{ + Type: ErrorType, + Interface: richErrors.WithMessage(richErrors.New("egad"), "failed"), + Key: "k", + } + enc := NewMapObjectEncoder() + f.AddTo(enc) + assert.Equal(t, "failed: egad", enc.Fields["k"], "Unexpected basic error message.") + + serialized := enc.Fields["kVerbose"] + // Don't assert the exact format used by a third-party package, but ensure + // that some critical elements are present. + assert.Regexp(t, `egad`, serialized, "Expected original error message to be present.") + assert.Regexp(t, `failed`, serialized, "Expected error annotation to be present.") + assert.Regexp(t, `TestRichErrorSupport`, serialized, "Expected calling function to be present in stacktrace.") +} + func TestEquals(t *testing.T) { tests := []struct { a, b Field