diff --git a/error.go b/error.go index 0ee6182a6..2bff30d85 100644 --- a/error.go +++ b/error.go @@ -1,4 +1,4 @@ -// Copyright (c) 2016 Uber Technologies, Inc. +// 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 diff --git a/error_test.go b/error_test.go index 93de5952a..54ce4bbd7 100644 --- a/error_test.go +++ b/error_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2016 Uber Technologies, Inc. +// 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 diff --git a/glide.lock b/glide.lock index 54377a307..db50355e6 100644 --- a/glide.lock +++ b/glide.lock @@ -1,10 +1,10 @@ hash: 2b8be1588d6028696f61ce80eb1b50dbad19144b4d4ead760977ad54064f4f26 -updated: 2017-06-27T16:21:01.272833298-07:00 +updated: 2017-06-30T10:57:16.454459206-07:00 imports: - name: go.uber.org/atomic version: 4e336646b2ef9fc6e47be8e21594178f98e5ebcf - name: go.uber.org/multierr - version: ef109dc7f883f3a99db2d89edc701c25cac40571 + version: 3c4937480c32f4c13a875a1829af76c98ca3d40a 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: 0c52d4024a04395dabac42b65e5825d08d3335df + version: 9917269ff0e9fc93df46474b411ea1716195a61b 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: e790cca94e6cc75c7064b1332e63811d4aae1a53 + version: 1b00554d822231195d1babd97ff4a781231955c9 - name: github.com/pkg/errors - version: c605e284fe17294bda444b34710735b29d1a9d90 + version: 645ef00459ed84a119197bfb8d8205042c6df63d - 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: 3d4380f53a34dcdc95f0c1db702615992b38d9a4 + version: 68cec9f21fbf3ea8d8f98c044bc6ce05f17b267a - 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: 90796e5a05ce440b41c768bd9af257005e470461 + version: d4feaf1a7e61e1d9e79e6c4e76c6349e9cab0a03 subpackages: - unix - name: golang.org/x/tools - version: e6cb469339aef5b7be0c89de730d5f3cc8e47e50 + version: 1529f889eb4b594d1f047f2fb8d5b3cc85c8f006 subpackages: - cover - name: gopkg.in/inconshreveable/log15.v2 diff --git a/zapcore/error.go b/zapcore/error.go new file mode 100644 index 000000000..746bae13b --- /dev/null +++ b/zapcore/error.go @@ -0,0 +1,128 @@ +// 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 new file mode 100644 index 000000000..21f8e593b --- /dev/null +++ b/zapcore/error_test.go @@ -0,0 +1,227 @@ +// 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 4f722008f..6a5e33e2f 100644 --- a/zapcore/field.go +++ b/zapcore/field.go @@ -162,17 +162,7 @@ func (f Field) AddTo(enc ObjectEncoder) { case StringerType: enc.AddString(f.Key, f.Interface.(fmt.Stringer).String()) case ErrorType: - 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) - } - } + encodeError(f.Key, f.Interface.(error), enc) case SkipType: break default: diff --git a/zapcore/field_test.go b/zapcore/field_test.go index 24c2aec7d..326cf8dbb 100644 --- a/zapcore/field_test.go +++ b/zapcore/field_test.go @@ -23,14 +23,12 @@ 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" @@ -42,18 +40,6 @@ 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") @@ -129,7 +115,6 @@ 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)}, } @@ -146,24 +131,6 @@ 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