Repository: rs/zerolog
Pull request #560 (2023-07-04 14:38:53)

feat: display error stack when using `.Fields()`
Display error stack when using `.Fields()`. Right now, even if the `ErrorStackMarshaler` is set, the stack will not be displayed if there is an error in the Fields() values but only when using Err().


Patch:
diff --git a/README.md b/README.md
index f575a68..972b729 100644
--- a/README.md
+++ b/README.md
@@ -1,6 +1,6 @@
 # Zero Allocation JSON Logger
 
-[![godoc](http://img.shields.io/badge/godoc-reference-blue.svg?style=flat)](https://godoc.org/github.com/rs/zerolog) [![license](http://img.shields.io/badge/license-MIT-red.svg?style=flat)](https://raw.githubusercontent.com/rs/zerolog/master/LICENSE) [![Build Status](https://travis-ci.org/rs/zerolog.svg?branch=master)](https://travis-ci.org/rs/zerolog) [![Go Coverage](https://github.com/rs/zerolog/wiki/coverage.svg)](https://raw.githack.com/wiki/rs/zerolog/coverage.html)
+[![godoc](http://img.shields.io/badge/godoc-reference-blue.svg?style=flat)](https://godoc.org/github.com/rs/zerolog) [![license](http://img.shields.io/badge/license-MIT-red.svg?style=flat)](https://raw.githubusercontent.com/rs/zerolog/master/LICENSE) [![Build Status](https://github.com/rs/zerolog/actions/workflows/test.yml/badge.svg)](https://github.com/rs/zerolog/actions/workflows/test.yml) [![Go Coverage](https://github.com/rs/zerolog/wiki/coverage.svg)](https://raw.githack.com/wiki/rs/zerolog/coverage.html)
 
 The zerolog package provides a fast and simple logger dedicated to JSON output.
 
@@ -499,7 +499,7 @@ log.Ctx(ctx).Info().Msg("hello world")
 ### Set as standard logger output
 
 ```go
-stdlog := zerolog.New(os.Stdout).With().
+log := zerolog.New(os.Stdout).With().
     Str("foo", "bar").
     Logger()
 
diff --git a/console.go b/console.go
index 8b0e0c6..2827988 100644
--- a/console.go
+++ b/console.go
@@ -312,6 +312,11 @@ func needsQuote(s string) bool {
 
 // colorize returns the string s wrapped in ANSI code c, unless disabled is true.
 func colorize(s interface{}, c int, disabled bool) string {
+	e := os.Getenv("NO_COLOR")
+	if e != "" {
+		disabled = true
+	}
+
 	if disabled {
 		return fmt.Sprintf("%s", s)
 	}
diff --git a/context.go b/context.go
index fc62ad9..ff48b1f 100644
--- a/context.go
+++ b/context.go
@@ -23,7 +23,7 @@ func (c Context) Logger() Logger {
 // Only map[string]interface{} and []interface{} are accepted. []interface{} must
 // alternate string keys and arbitrary values, and extraneous ones are ignored.
 func (c Context) Fields(fields interface{}) Context {
-	c.l.context = appendFields(c.l.context, fields)
+	c.l.context = appendFields(c.l.context, fields, c.l.stack)
 	return c
 }
 
@@ -379,6 +379,12 @@ func (c Context) Interface(key string, i interface{}) Context {
 	return c
 }
 
+// Type adds the field key with val's type using reflection.
+func (c Context) Type(key string, val interface{}) Context {
+	c.l.context = enc.AppendType(enc.AppendKey(c.l.context, key), val)
+	return c
+}
+
 // Any is a wrapper around Context.Interface.
 func (c Context) Any(key string, i interface{}) Context {
 	return c.Interface(key, i)
diff --git a/event.go b/event.go
index 2a5d3b0..5c949f8 100644
--- a/event.go
+++ b/event.go
@@ -164,7 +164,7 @@ func (e *Event) Fields(fields interface{}) *Event {
 	if e == nil {
 		return e
 	}
-	e.buf = appendFields(e.buf, fields)
+	e.buf = appendFields(e.buf, fields, e.stack)
 	return e
 }
 
diff --git a/fields.go b/fields.go
index c1eb5ce..23606dd 100644
--- a/fields.go
+++ b/fields.go
@@ -12,13 +12,13 @@ func isNilValue(i interface{}) bool {
 	return (*[2]uintptr)(unsafe.Pointer(&i))[1] == 0
 }
 
-func appendFields(dst []byte, fields interface{}) []byte {
+func appendFields(dst []byte, fields interface{}, stack bool) []byte {
 	switch fields := fields.(type) {
 	case []interface{}:
 		if n := len(fields); n&0x1 == 1 { // odd number
 			fields = fields[:n-1]
 		}
-		dst = appendFieldList(dst, fields)
+		dst = appendFieldList(dst, fields, stack)
 	case map[string]interface{}:
 		keys := make([]string, 0, len(fields))
 		for key := range fields {
@@ -28,13 +28,13 @@ func appendFields(dst []byte, fields interface{}) []byte {
 		kv := make([]interface{}, 2)
 		for _, key := range keys {
 			kv[0], kv[1] = key, fields[key]
-			dst = appendFieldList(dst, kv)
+			dst = appendFieldList(dst, kv, stack)
 		}
 	}
 	return dst
 }
 
-func appendFieldList(dst []byte, kvList []interface{}) []byte {
+func appendFieldList(dst []byte, kvList []interface{}, stack bool) []byte {
 	for i, n := 0, len(kvList); i < n; i += 2 {
 		key, val := kvList[i], kvList[i+1]
 		if key, ok := key.(string); ok {
@@ -74,6 +74,21 @@ func appendFieldList(dst []byte, kvList []interface{}) []byte {
 			default:
 				dst = enc.AppendInterface(dst, m)
 			}
+
+			if stack && ErrorStackMarshaler != nil {
+				dst = enc.AppendKey(dst, ErrorStackFieldName)
+				switch m := ErrorStackMarshaler(val).(type) {
+				case nil:
+				case error:
+					if m != nil && !isNilValue(m) {
+						dst = enc.AppendString(dst, m.Error())
+					}
+				case string:
+					dst = enc.AppendString(dst, m)
+				default:
+					dst = enc.AppendInterface(dst, m)
+				}
+			}
 		case []error:
 			dst = enc.AppendArrayStart(dst)
 			for i, err := range val {
diff --git a/go.mod b/go.mod
index 6ec5abc..4385ff9 100644
--- a/go.mod
+++ b/go.mod
@@ -4,7 +4,9 @@ go 1.15
 
 require (
 	github.com/coreos/go-systemd/v22 v22.5.0
-	github.com/mattn/go-colorable v0.1.12
+	github.com/mattn/go-colorable v0.1.13
+	github.com/mattn/go-isatty v0.0.19 // indirect
 	github.com/pkg/errors v0.9.1
 	github.com/rs/xid v1.5.0
+	golang.org/x/sys v0.12.0 // indirect
 )
diff --git a/go.sum b/go.sum
index 50b6327..f2b8d84 100644
--- a/go.sum
+++ b/go.sum
@@ -3,8 +3,13 @@ github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSV
 github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
 github.com/mattn/go-colorable v0.1.12 h1:jF+Du6AlPIjs2BiUiQlKOX0rt3SujHxPnksPKZbaA40=
 github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4=
+github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=
+github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg=
 github.com/mattn/go-isatty v0.0.14 h1:yVuAays6BHfxijgZPzw+3Zlu5yQgKGP2/hcQbHb7S9Y=
 github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94=
+github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM=
+github.com/mattn/go-isatty v0.0.19 h1:JITubQf0MOLdlGRuRq+jtsDlekdYPia9ZFsB8h/APPA=
+github.com/mattn/go-isatty v0.0.19/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/rs/xid v1.5.0 h1:mKX4bl4iPYJtEIxp6CYiUuLQ/8DYMoz0PUdtGgMFRVc=
@@ -12,3 +17,7 @@ github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg=
 golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6 h1:foEbQz/B0Oz6YIqu/69kfXPYeFQAuuMYFkjaqXzl5Wo=
 golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.12.0 h1:CM0HF96J0hcLAwsHPJZjfdNzs0gftsLfgKt57wWHJ0o=
+golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index a289a26..93c0756 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -13,7 +13,7 @@ jobs:
       with:
         go-version: ${{ matrix.go-version }}
     - name: Checkout code
-      uses: actions/checkout@v3
+      uses: actions/checkout@v4
     - uses: actions/cache@v3
       with:
         path: ~/go/pkg/mod
diff --git a/console_test.go b/console_test.go
index 18c2db7..0265672 100644
--- a/console_test.go
+++ b/console_test.go
@@ -104,6 +104,25 @@ func TestConsoleWriter(t *testing.T) {
 		}
 	})
 
+	t.Run("NO_COLOR = true", func(t *testing.T) {
+		os.Setenv("NO_COLOR", "anything")
+
+		buf := &bytes.Buffer{}
+		w := zerolog.ConsoleWriter{Out: buf}
+
+		_, err := w.Write([]byte(`{"level": "warn", "message": "Foobar"}`))
+		if err != nil {
+			t.Errorf("Unexpected error when writing output: %s", err)
+		}
+
+		expectedOutput := "<nil> WRN Foobar\n"
+		actualOutput := buf.String()
+		if actualOutput != expectedOutput {
+			t.Errorf("Unexpected output %q, want: %q", actualOutput, expectedOutput)
+		}
+		os.Unsetenv("NO_COLOR")
+	})
+
 	t.Run("Write fields", func(t *testing.T) {
 		buf := &bytes.Buffer{}
 		w := zerolog.ConsoleWriter{Out: buf, NoColor: true}
diff --git a/pkgerrors/stacktrace_test.go b/pkgerrors/stacktrace_test.go
index 5a13832..a09b986 100644
--- a/pkgerrors/stacktrace_test.go
+++ b/pkgerrors/stacktrace_test.go
@@ -28,6 +28,22 @@ func TestLogStack(t *testing.T) {
 	}
 }
 
+func TestLogStackFields(t *testing.T) {
+	zerolog.ErrorStackMarshaler = MarshalStack
+
+	out := &bytes.Buffer{}
+	log := zerolog.New(out)
+
+	err := fmt.Errorf("from error: %w", errors.New("error message"))
+	log.Log().Stack().Fields([]interface{}{"error", err}).Msg("")
+
+	got := out.String()
+	want := `\{"error":"from error: error message","stack":\[\{"func":"TestLogStackFields","line":"37","source":"stacktrace_test.go"\},.*\]\}\n`
+	if ok, _ := regexp.MatchString(want, got); !ok {
+		t.Errorf("invalid log output:\ngot:  %v\nwant: %v", got, want)
+	}
+}
+
 func TestLogStackFromContext(t *testing.T) {
 	zerolog.ErrorStackMarshaler = MarshalStack
 
@@ -38,7 +54,7 @@ func TestLogStackFromContext(t *testing.T) {
 	log.Log().Err(err).Msg("") // not explicitly calling Stack()
 
 	got := out.String()
-	want := `\{"stack":\[\{"func":"TestLogStackFromContext","line":"37","source":"stacktrace_test.go"\},.*\],"error":"from error: error message"\}\n`
+	want := `\{"stack":\[\{"func":"TestLogStackFromContext","line":"53","source":"stacktrace_test.go"\},.*\],"error":"from error: error message"\}\n`
 	if ok, _ := regexp.MatchString(want, got); !ok {
 		t.Errorf("invalid log output:\ngot:  %v\nwant: %v", got, want)
 	}