SugaredLogger: Turn error into zap.Error (#1185)

Craig Pastro · web-flow · commit 9b86a50a3e27 · 2022-10-15T09:13:25.000-07:00
diff --git a/sugar.go b/sugar.go
@@ -31,6 +31,7 @@ import (
 const (
 	_oddNumberErrMsg    = "Ignored key without a value."
 	_nonStringKeyErrMsg = "Ignored key-value pairs with non-string keys."
+	_multipleErrMsg     = "Multiple errors without a key."
 )
 
 // A SugaredLogger wraps the base Logger functionality in a slower, but less
@@ -336,10 +337,13 @@ func (s *SugaredLogger) sweetenFields(args []interface{}) []Field {
 		return nil
 	}
 
-	// Allocate enough space for the worst case; if users pass only structured
-	// fields, we shouldn't penalize them with extra allocations.
-	fields := make([]Field, 0, len(args))
-	var invalid invalidPairs
+	var (
+		// Allocate enough space for the worst case; if users pass only structured
+		// fields, we shouldn't penalize them with extra allocations.
+		fields    = make([]Field, 0, len(args))
+		invalid   invalidPairs
+		seenError bool
+	)
 
 	for i := 0; i < len(args); {
 		// This is a strongly-typed field. Consume it and move on.
@@ -349,6 +353,18 @@ func (s *SugaredLogger) sweetenFields(args []interface{}) []Field {
 			continue
 		}
 
+		// If it is an error, consume it and move on.
+		if err, ok := args[i].(error); ok {
+			if !seenError {
+				seenError = true
+				fields = append(fields, Error(err))
+			} else {
+				s.base.Error(_multipleErrMsg, Error(err))
+			}
+			i++
+			continue
+		}
+
 		// Make sure this element isn't a dangling key.
 		if i == len(args)-1 {
 			s.base.Error(_oddNumberErrMsg, Any("ignored", args[i]))
diff --git a/sugar_test.go b/sugar_test.go
@@ -21,6 +21,7 @@
 package zap
 
 import (
+	"errors"
 	"testing"
 
 	"go.uber.org/zap/internal/exit"
@@ -46,6 +47,12 @@ func TestSugarWith(t *testing.T) {
 			Context: []Field{Array("invalid", invalidPairs(pairs))},
 		}
 	}
+	ignoredError := func(err error) observer.LoggedEntry {
+		return observer.LoggedEntry{
+			Entry:   zapcore.Entry{Level: ErrorLevel, Message: _multipleErrMsg},
+			Context: []Field{Error(err)},
+		}
+	}
 
 	tests := []struct {
 		desc     string
@@ -122,6 +129,15 @@ func TestSugarWith(t *testing.T) {
 				nonString(invalidPair{2, true, "bar"}, invalidPair{5, 42, "reversed"}),
 			},
 		},
+		{
+			desc:     "multiple errors",
+			args:     []interface{}{errors.New("first"), errors.New("second"), errors.New("third")},
+			expected: []Field{Error(errors.New("first"))},
+			errLogs: []observer.LoggedEntry{
+				ignoredError(errors.New("second")),
+				ignoredError(errors.New("third")),
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -198,10 +214,13 @@ func TestSugarStructuredLogging(t *testing.T) {
 	}
 
 	// Common to all test cases.
-	context := []interface{}{"foo", "bar"}
-	extra := []interface{}{"baz", false}
-	expectedFields := []Field{String("foo", "bar"), Bool("baz", false)}
-
+	var (
+		err            = errors.New("qux")
+		context        = []interface{}{"foo", "bar"}
+		extra          = []interface{}{err, "baz", false}
+		expectedFields = []Field{String("foo", "bar"), Error(err), Bool("baz", false)}
+	)
+	
 	for _, tt := range tests {
 		withSugar(t, DebugLevel, nil, func(logger *SugaredLogger, logs *observer.ObservedLogs) {
 			logger.With(context...).Debugw(tt.msg, extra...)
