Less verbose output for multierr (#470)

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.

zapcore/error.go

@@ -46,25 +46,17 @@ 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)
+	switch e := err.(type) {
+	case errorGroup:
+		return enc.AddArray(key+"Causes", errArray(e.Errors()))
+	case fmt.Formatter:
+		verbose := fmt.Sprintf("%+v", e)
 		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
 }
 

zapcore/error_test.go

@@ -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"
@@ -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"},
@@ -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"},
@@ -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",
 			},
 		},
 		{
@@ -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"},
 				},
 			},
 		},
@@ -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")
 }