Skip to content

Commit

Permalink
Less verbose output for multierr
Browse files Browse the repository at this point in the history
Following the discussions on #460, uber-go/multierr#6, and
(most recently) uber-go/multierr#23, reduce log verbosity for
`multierr`. This is fully backward-compatible with the last released
version of zap.

The small changes introduced here do two things:

1. First, we either report `errorCauses` or `errorVerbose`, but not
   both.
2. Second, we prefer `errorCauses` to `errorVerbose`.

I think that this addresses our top-level wants without breaking any
interfaces or removing behavior we've already shipped.

If we ever decide to cut a new major release of zap, we should treat
errors like durations and times - they're special types for which users
choose a formatter.

In a future release, we can add an `ErrorEncoder` interface that the
JSON encoder and console encoder implement, and make the error field
attempt an upcast into that type. That would let the user supply their
own error encoder (much like they supply their own time and duration
encoders now). Even if we do that, though, I suspect that we'll want to
preserve the behavior here as the default.
  • Loading branch information
Akshay Shah committed Jul 6, 2017
1 parent 06ca39d commit d8df53e
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 64 deletions.
15 changes: 4 additions & 11 deletions zapcore/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ func encodeError(key string, err error, enc ObjectEncoder) error {
basic := err.Error()
enc.AddString(key, basic)

if group, ok := err.(errorGroup); ok {
return enc.AddArray(key+"Causes", errArray(group.Errors()))
}

if fancy, ok := err.(fmt.Formatter); ok {
verbose := fmt.Sprintf("%+v", fancy)
if verbose != basic {
Expand All @@ -54,17 +58,6 @@ func encodeError(key string, err error, enc ObjectEncoder) error {
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
}

Expand Down
56 changes: 3 additions & 53 deletions zapcore/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (

richErrors "github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.uber.org/multierr"
. "go.uber.org/zap/zapcore"
Expand Down Expand Up @@ -88,10 +87,6 @@ func TestErrorEncoding(t *testing.T) {
),
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"},
Expand All @@ -108,9 +103,6 @@ func TestErrorEncoding(t *testing.T) {
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"},
Expand All @@ -123,14 +115,8 @@ func TestErrorEncoding(t *testing.T) {
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": "failed: egad",
"kVerbose": "egad\nfailed",
},
},
{
Expand All @@ -145,42 +131,16 @@ func TestErrorEncoding(t *testing.T) {
),
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"},
},
},
map[string]interface{}{"error": "world: qux", "errorVerbose": "qux\nworld"},
},
},
},
Expand Down Expand Up @@ -214,14 +174,4 @@ func TestRichErrorSupport(t *testing.T) {
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")
}

0 comments on commit d8df53e

Please sign in to comment.