From 261fb0e996cd067947972f17a4c061d338e5a470 Mon Sep 17 00:00:00 2001 From: Maxence Yang Date: Wed, 6 May 2026 11:41:40 +0800 Subject: [PATCH 1/5] feat!: Err and ErrAny return a single inline zap.Field 1.[]Field broke inline log call style -> return one Field 2.zap.Inline preserves flat dotted keys -> output unchanged 3.nil input would force conditional append -> return zap.Skip --- CHANGELOG.md | 29 +++++ docs/ecs-coverage.md | 2 +- docs/related-projects.md | 8 +- example/main.go | 3 +- zap/error.go | 141 +++++++++++++++---------- zap/error_test.go | 222 ++++++++++++--------------------------- 6 files changed, 187 insertions(+), 218 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5077486..e0fa8cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,35 @@ ## [Unreleased] +## [0.3.0] + +### Changed (BREAKING) + +- `Err(err) zap.Field` and `ErrAny(v) zap.Field` now return a single inline + `zap.Field` (was `[]zap.Field`). Output JSON is unchanged — the same flat + dotted ECS keys (`error.message`, `error.type`, `error.stack_trace`) are + emitted via `zap.Inline` plus a custom `ObjectMarshaler`. + + Migration: + + ```go + // Before (v0.2.0): + fields := []zap.Field{ServiceName("auth"), EventAction("login")} + fields = append(fields, Err(err)...) + logger.Error("failed", fields...) + + // After (v0.3.0): + logger.Error("failed", + ServiceName("auth"), + EventAction("login"), + Err(err), + ) + ``` + + `nil` input still skips cleanly: `Err(nil)` and `ErrAny(nil)` now return + `zap.Skip()` instead of a `nil` slice; the field is no-op when added to a + log entry, so unconditional inline use is safe. + ## [0.2.0] ### Added diff --git a/docs/ecs-coverage.md b/docs/ecs-coverage.md index ec6ee97..1183b02 100644 --- a/docs/ecs-coverage.md +++ b/docs/ecs-coverage.md @@ -12,7 +12,7 @@ covered, deferred, or out of scope. | `host.*` (top-level) | 9 | `HostIP` / `HostMAC` are variadic; `HostUptime` emits seconds | | `process.*` (top-level) | 11 | `ProcessUptime` emits seconds; `ProcessStart` is `time.Time`; endpoint-security subtrees excluded | | `event.*` | 22 | `event.duration` emits **nanoseconds**; `event.original` is bytes; typed enums for `kind`/`outcome`/`category`/`type` | -| `error.*` + `Err()` / `ErrAny()` | 5 + 2 | `Err(error)` and `ErrAny(any)` both extract `error.message` / `error.type` always, plus `error.stack_trace` when the source implements either `StackTrace() []byte` (samber/oops) or `StackTrace() pkgerrors.StackTrace` (github.com/pkg/errors). `ErrAny` accepts `recover()` payloads (typed as `any`) and delegates to `Err` when the value satisfies `error`. | +| `error.*` + `Err()` / `ErrAny()` | 5 + 2 | `Err(error) zap.Field` and `ErrAny(any) zap.Field` both return a single inline field that emits `error.message` / `error.type` always, plus `error.stack_trace` when the source implements either `StackTrace() []byte` (samber/oops) or `StackTrace() pkgerrors.StackTrace` (github.com/pkg/errors). `ErrAny` accepts `recover()` payloads (typed as `any`) and delegates to `Err` when the value satisfies `error`. | | `log.*` | 6 | Includes `log.origin.*` | | `trace.id`, `span.id`, `transaction.id` | 3 | APM correlation | | `http.*` | 13 | Bytes are `int64`, status code is `int` | diff --git a/docs/related-projects.md b/docs/related-projects.md index 5d5b4af..7a0586c 100644 --- a/docs/related-projects.md +++ b/docs/related-projects.md @@ -45,11 +45,11 @@ section. | Goal | Use | | ------------------------------------------------ | ------------------------- | -| Log a Go `error` with full ECS shape | `ecsf.Err(err)...` | -| Log a `recover()` payload (typed `any`) | `ecsf.ErrAny(v)...` | +| Log a Go `error` with full ECS shape | `ecsf.Err(err)` | +| Log a `recover()` payload (typed `any`) | `ecsf.ErrAny(v)` | | Just need message + stack and already on ecszap | `zap.Error(err)` is fine | -| Need `error.type` populated for Kibana filtering | `ecsf.Err(err)...` | -| Run without ecszap (e.g. console encoder) | `ecsf.Err(err)...` | +| Need `error.type` populated for Kibana filtering | `ecsf.Err(err)` | +| Run without ecszap (e.g. console encoder) | `ecsf.Err(err)` | ### Detailed comparison diff --git a/example/main.go b/example/main.go index 4447eff..b4ba20d 100644 --- a/example/main.go +++ b/example/main.go @@ -55,8 +55,9 @@ func main() { ecsf.Label("tenant", "acme"), ecsf.Tags("login", "audit"), + + ecsf.Err(err), } - fields = append(fields, ecsf.Err(err)...) logger.Info("login attempt failed", fields...) } diff --git a/zap/error.go b/zap/error.go index c5c202e..6c87d95 100644 --- a/zap/error.go +++ b/zap/error.go @@ -3,8 +3,10 @@ // ECS reference (8.17): https://www.elastic.co/guide/en/ecs/8.17/ecs-error.html // // The package exposes single-field constructors (uniform with the rest of the -// library) plus one multi-field convenience helper, Err(), which extracts ECS -// error.* fields from a Go error without requiring any specific zap encoder. +// library) plus two multi-field convenience helpers, Err() and ErrAny(), that +// pack the standard error.* fields into a single inline zap.Field. Callers do +// not need any specific zap encoder — output is flat dotted ECS keys, the +// same shape every other constructor in this package emits. package zap @@ -47,7 +49,7 @@ type stackTracerPCs interface { StackTrace() pkgerrors.StackTrace } -// Err extracts ECS error.* fields from a Go error. It returns: +// Err returns a single inline zap.Field that emits ECS error.* fields: // // - error.message: always (err.Error()) // - error.type: always (fmt.Sprintf("%T", err)) @@ -56,21 +58,88 @@ type stackTracerPCs interface { // 1. interface{ StackTrace() []byte } (samber/oops) // 2. interface{ StackTrace() pkgerrors.StackTrace } (github.com/pkg/errors) // -// Err is the only multi-field constructor in the library, provided so callers -// do not need any specific zap encoder (e.g. ecszap) to obtain a stack trace. -// Returns nil if err is nil so the caller can splat it unconditionally. -func Err(err error) []zapcore.Field { +// Returns zap.Skip() if err is nil so the caller can pass the result +// unconditionally: +// +// logger.Error("operation failed", +// ServiceName("auth-api"), +// EventAction("user.login"), +// Err(err), +// ) +// +// The output JSON is flat dotted ECS keys (error.message, error.type, +// error.stack_trace) — identical to manually composing single-field helpers, +// but as one Field so it composes naturally with sibling fields. +func Err(err error) zapcore.Field { if err == nil { - return nil + return zap.Skip() + } + return zap.Inline(errMarshaler{err: err}) +} + +// ErrAny returns a single inline zap.Field that emits ECS error.* fields from +// any value, intended for cases where the input is not statically typed as +// error — most commonly the result of recover() during panic handling. +// Behavior by input type: +// +// - nil: returns zap.Skip() (no fields emitted) +// - typed-nil error: error.type emitted, error.message = "" — never +// calls Error() on the typed-nil receiver, which would panic +// - error: same fields as Err(err), including error.stack_trace +// when the error implements either StackTrace() []byte (samber/oops) or +// StackTrace() pkgerrors.StackTrace (github.com/pkg/errors) +// - other: error.message = fmt.Sprint(v); error.type = fmt.Sprintf("%T", v) +// +// ErrAny intentionally does not call runtime/debug.Stack() itself. To attach +// the panic stack, pass it explicitly — callers may want to skip the cost or +// use a different stack source. +// +// Typical panic recovery: +// +// defer func() { +// if r := recover(); r != nil { +// logger.Error("panic recovered", +// ErrAny(r), +// ErrorStackTrace(debug.Stack()), +// ) +// } +// }() +func ErrAny(v any) zapcore.Field { + if v == nil { + return zap.Skip() } - fields := []zapcore.Field{ - ErrorMessage(err.Error()), - ErrorType(fmt.Sprintf("%T", err)), + return zap.Inline(errAnyMarshaler{v: v}) +} + +// errMarshaler renders an error into ECS error.* keys at the encoder's +// current namespace (no nesting). Used by Err via zap.Inline. +type errMarshaler struct{ err error } + +func (m errMarshaler) MarshalLogObject(enc zapcore.ObjectEncoder) error { + enc.AddString("error.message", m.err.Error()) + enc.AddString("error.type", fmt.Sprintf("%T", m.err)) + if stack := extractStackTrace(m.err); len(stack) > 0 { + enc.AddByteString("error.stack_trace", stack) } - if stack := extractStackTrace(err); len(stack) > 0 { - fields = append(fields, ErrorStackTrace(stack)) + return nil +} + +// errAnyMarshaler renders any value into ECS error.* keys, handling the +// typed-nil error gotcha and non-error values. Used by ErrAny via zap.Inline. +type errAnyMarshaler struct{ v any } + +func (m errAnyMarshaler) MarshalLogObject(enc zapcore.ObjectEncoder) error { + if err, ok := m.v.(error); ok { + if isTypedNil(err) { + enc.AddString("error.message", "") + enc.AddString("error.type", fmt.Sprintf("%T", err)) + return nil + } + return errMarshaler{err: err}.MarshalLogObject(enc) } - return fields + enc.AddString("error.message", fmt.Sprint(m.v)) + enc.AddString("error.type", fmt.Sprintf("%T", m.v)) + return nil } // extractStackTrace walks the error chain and returns the first stack trace @@ -95,50 +164,6 @@ func extractStackTrace(err error) []byte { return nil } -// ErrAny extracts ECS error.* fields from any value, intended for cases where -// the input is not statically typed as error — most commonly the result of -// recover() during panic handling. Behavior by input type: -// -// - nil: returns nil (no fields) -// - typed-nil error: error.type emitted, error.message = "" — never -// calls Error() on the typed-nil receiver, which would panic -// - error: delegates to Err(err) — error.stack_trace included if -// the error implements either StackTrace() []byte (samber/oops) or -// StackTrace() pkgerrors.StackTrace (github.com/pkg/errors) -// - other: error.message = fmt.Sprint(v); error.type = fmt.Sprintf("%T", v) -// -// ErrAny intentionally does not call runtime/debug.Stack() itself. To attach -// the panic stack, append ErrorStackTrace(debug.Stack()) at the call site — -// callers may want to skip the cost or use a different stack source. -// -// Typical panic recovery: -// -// defer func() { -// if r := recover(); r != nil { -// fields := ErrAny(r) -// fields = append(fields, ErrorStackTrace(debug.Stack())) -// logger.Error("panic recovered", fields...) -// } -// }() -func ErrAny(v any) []zapcore.Field { - if v == nil { - return nil - } - if err, ok := v.(error); ok { - if isTypedNil(err) { - return []zapcore.Field{ - ErrorMessage(""), - ErrorType(fmt.Sprintf("%T", err)), - } - } - return Err(err) - } - return []zapcore.Field{ - ErrorMessage(fmt.Sprint(v)), - ErrorType(fmt.Sprintf("%T", v)), - } -} - // isTypedNil reports whether v is non-nil at the interface level but holds a // nil concrete value (e.g. (*MyErr)(nil) cast to error). Calling methods that // dereference the receiver on such a value panics, so ErrAny short-circuits diff --git a/zap/error_test.go b/zap/error_test.go index 6665c78..a24c5b1 100644 --- a/zap/error_test.go +++ b/zap/error_test.go @@ -4,7 +4,6 @@ package zap_test import ( "errors" - "fmt" "testing" pkgerrors "github.com/pkg/errors" @@ -15,6 +14,14 @@ import ( ecszap "github.com/maxence2997/ecsfields/zap" ) +// emit applies the field to a MapObjectEncoder and returns the resulting key +// map. Inline fields (Err / ErrAny) flatten their sub-keys into this map. +func emit(f zapcore.Field) map[string]any { + enc := zapcore.NewMapObjectEncoder() + f.AddTo(enc) + return enc.Fields +} + func TestErrorMessage(t *testing.T) { f := ecszap.ErrorMessage("boom") assert.Equal(t, "error.message", f.Key) @@ -33,10 +40,7 @@ func TestErrorStackTrace(t *testing.T) { f := ecszap.ErrorStackTrace([]byte("goroutine 1")) assert.Equal(t, "error.stack_trace", f.Key) assert.Equal(t, zapcore.ByteStringType, f.Type) - - enc := zapcore.NewMapObjectEncoder() - f.AddTo(enc) - assert.Equal(t, "goroutine 1", enc.Fields["error.stack_trace"]) + assert.Equal(t, "goroutine 1", emit(f)["error.stack_trace"]) } func TestErrorCode(t *testing.T) { @@ -53,27 +57,19 @@ func TestErrorID(t *testing.T) { assert.Equal(t, "err-1", f.String) } -func TestErr_Nil(t *testing.T) { - got := ecszap.Err(nil) - assert.Nil(t, got) +func TestErr_Nil_ReturnsSkip(t *testing.T) { + f := ecszap.Err(nil) + assert.Equal(t, zapcore.SkipType, f.Type) + assert.Empty(t, emit(f)) } func TestErr_StdlibError_NoStackTrace(t *testing.T) { err := errors.New("boom") - got := ecszap.Err(err) - require.Len(t, got, 2) - - keys := []string{got[0].Key, got[1].Key} - assert.ElementsMatch(t, []string{"error.message", "error.type"}, keys) - - for _, f := range got { - switch f.Key { - case "error.message": - assert.Equal(t, "boom", f.String) - case "error.type": - assert.Equal(t, fmt.Sprintf("%T", err), f.String) - } - } + got := emit(ecszap.Err(err)) + + assert.Equal(t, "boom", got["error.message"]) + assert.Equal(t, "*errors.errorString", got["error.type"]) + assert.NotContains(t, got, "error.stack_trace") } type fakeStackTracer struct { @@ -84,99 +80,65 @@ type fakeStackTracer struct { func (f *fakeStackTracer) Error() string { return f.msg } func (f *fakeStackTracer) StackTrace() []byte { return f.stack } -func TestErr_StackTracer_EmitsStackTrace(t *testing.T) { +func TestErr_BytesStackTracer_EmitsStackTrace(t *testing.T) { err := &fakeStackTracer{msg: "kaboom", stack: []byte("goroutine 1...")} - got := ecszap.Err(err) - require.Len(t, got, 3) - - var keys []string - for _, f := range got { - keys = append(keys, f.Key) - } - assert.ElementsMatch(t, - []string{"error.message", "error.type", "error.stack_trace"}, - keys, - ) - - for _, f := range got { - switch f.Key { - case "error.message": - assert.Equal(t, "kaboom", f.String) - case "error.stack_trace": - assert.Equal(t, zapcore.ByteStringType, f.Type) - enc := zapcore.NewMapObjectEncoder() - f.AddTo(enc) - assert.Equal(t, "goroutine 1...", enc.Fields["error.stack_trace"]) - } - } + got := emit(ecszap.Err(err)) + + assert.Equal(t, "kaboom", got["error.message"]) + assert.Equal(t, "*zap_test.fakeStackTracer", got["error.type"]) + assert.Equal(t, "goroutine 1...", got["error.stack_trace"]) } -func TestErrAny_Nil(t *testing.T) { - assert.Nil(t, ecszap.ErrAny(nil)) +func TestErr_PkgErrorsStackTracer_EmitsStackTrace(t *testing.T) { + err := pkgerrors.New("boom") + got := emit(ecszap.Err(err)) + + assert.Equal(t, "boom", got["error.message"]) + assert.NotEmpty(t, got["error.type"]) + + stack, ok := got["error.stack_trace"].(string) + require.True(t, ok, "error.stack_trace should be string-coercible") + assert.NotEmpty(t, stack) + assert.Contains(t, stack, "TestErr_PkgErrorsStackTracer_EmitsStackTrace", + "stack should reference the test function frame") +} + +func TestErrAny_Nil_ReturnsSkip(t *testing.T) { + f := ecszap.ErrAny(nil) + assert.Equal(t, zapcore.SkipType, f.Type) + assert.Empty(t, emit(f)) } func TestErrAny_Error_DelegatesToErr(t *testing.T) { err := errors.New("boom") - got := ecszap.ErrAny(err) - require.Len(t, got, 2) - - keys := []string{got[0].Key, got[1].Key} - assert.ElementsMatch(t, []string{"error.message", "error.type"}, keys) - for _, f := range got { - switch f.Key { - case "error.message": - assert.Equal(t, "boom", f.String) - case "error.type": - assert.Equal(t, fmt.Sprintf("%T", err), f.String) - } - } + got := emit(ecszap.ErrAny(err)) + + assert.Equal(t, "boom", got["error.message"]) + assert.Equal(t, "*errors.errorString", got["error.type"]) + assert.NotContains(t, got, "error.stack_trace") } func TestErrAny_StackTracerError_IncludesStackTrace(t *testing.T) { err := &fakeStackTracer{msg: "kaboom", stack: []byte("goroutine 1...")} - got := ecszap.ErrAny(err) - require.Len(t, got, 3) - - var keys []string - for _, f := range got { - keys = append(keys, f.Key) - } - assert.ElementsMatch(t, - []string{"error.message", "error.type", "error.stack_trace"}, - keys, - ) + got := emit(ecszap.ErrAny(err)) + + assert.Equal(t, "kaboom", got["error.message"]) + assert.Equal(t, "*zap_test.fakeStackTracer", got["error.type"]) + assert.Equal(t, "goroutine 1...", got["error.stack_trace"]) } func TestErrAny_String(t *testing.T) { - got := ecszap.ErrAny("oops") - require.Len(t, got, 2) - - for _, f := range got { - switch f.Key { - case "error.message": - assert.Equal(t, "oops", f.String) - case "error.type": - assert.Equal(t, "string", f.String) - default: - t.Fatalf("unexpected key %q", f.Key) - } - } + got := emit(ecszap.ErrAny("oops")) + + assert.Equal(t, "oops", got["error.message"]) + assert.Equal(t, "string", got["error.type"]) } func TestErrAny_Int(t *testing.T) { - got := ecszap.ErrAny(42) - require.Len(t, got, 2) - - for _, f := range got { - switch f.Key { - case "error.message": - assert.Equal(t, "42", f.String) - case "error.type": - assert.Equal(t, "int", f.String) - default: - t.Fatalf("unexpected key %q", f.Key) - } - } + got := emit(ecszap.ErrAny(42)) + + assert.Equal(t, "42", got["error.message"]) + assert.Equal(t, "int", got["error.type"]) } type panicPayload struct { @@ -184,49 +146,10 @@ type panicPayload struct { } func TestErrAny_Struct(t *testing.T) { - got := ecszap.ErrAny(panicPayload{Reason: "deadlocked"}) - require.Len(t, got, 2) - - for _, f := range got { - switch f.Key { - case "error.message": - assert.Equal(t, "{deadlocked}", f.String) - case "error.type": - assert.Equal(t, "zap_test.panicPayload", f.String) - default: - t.Fatalf("unexpected key %q", f.Key) - } - } -} + got := emit(ecszap.ErrAny(panicPayload{Reason: "deadlocked"})) -func TestErr_PkgErrorsStackTracer_EmitsStackTrace(t *testing.T) { - err := pkgerrors.New("boom") - got := ecszap.Err(err) - require.Len(t, got, 3) - - var keys []string - for _, f := range got { - keys = append(keys, f.Key) - } - assert.ElementsMatch(t, - []string{"error.message", "error.type", "error.stack_trace"}, - keys, - ) - - for _, f := range got { - switch f.Key { - case "error.message": - assert.Equal(t, "boom", f.String) - case "error.stack_trace": - assert.Equal(t, zapcore.ByteStringType, f.Type) - enc := zapcore.NewMapObjectEncoder() - f.AddTo(enc) - stack := enc.Fields["error.stack_trace"].(string) - assert.NotEmpty(t, stack) - assert.Contains(t, stack, "TestErr_PkgErrorsStackTracer_EmitsStackTrace", - "stack should reference the test function frame") - } - } + assert.Equal(t, "{deadlocked}", got["error.message"]) + assert.Equal(t, "zap_test.panicPayload", got["error.type"]) } // derefingErr panics on Error() if the receiver is nil — emulating the common @@ -240,20 +163,11 @@ func TestErrAny_TypedNilPointerError_DoesNotPanic(t *testing.T) { var typedNil *derefingErr var asInterface error = typedNil - var got []zapcore.Field + var got map[string]any require.NotPanics(t, func() { - got = ecszap.ErrAny(asInterface) + got = emit(ecszap.ErrAny(asInterface)) }) - require.Len(t, got, 2) - - for _, f := range got { - switch f.Key { - case "error.message": - assert.Equal(t, "", f.String) - case "error.type": - assert.Equal(t, "*zap_test.derefingErr", f.String) - default: - t.Fatalf("unexpected key %q", f.Key) - } - } + + assert.Equal(t, "", got["error.message"]) + assert.Equal(t, "*zap_test.derefingErr", got["error.type"]) } From 97ed61524a80d6d385490ca9e1148e822465573e Mon Sep 17 00:00:00 2001 From: Maxence Yang Date: Wed, 6 May 2026 11:54:15 +0800 Subject: [PATCH 2/5] fix: extend typed-nil guard to Err and dedupe marshaler logic 1.Err(typedNil) panicked in errMarshaler.Error -> guard moved 2.Two marshalers checked typed-nil -> single check in errMarshaler 3.Tests hard-coded *errors.errorString -> derive via %T --- zap/error.go | 24 +++++++++++++++++------- zap/error_test.go | 41 +++++++++++++++++++++++++++++------------ 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/zap/error.go b/zap/error.go index 6c87d95..e3209ff 100644 --- a/zap/error.go +++ b/zap/error.go @@ -58,6 +58,10 @@ type stackTracerPCs interface { // 1. interface{ StackTrace() []byte } (samber/oops) // 2. interface{ StackTrace() pkgerrors.StackTrace } (github.com/pkg/errors) // +// Typed-nil errors (a non-nil interface holding a nil pointer, e.g. +// (*MyErr)(nil) cast to error) are handled safely: error.type is emitted, +// error.message = "", and Error() is never called on the nil receiver. +// // Returns zap.Skip() if err is nil so the caller can pass the result // unconditionally: // @@ -113,9 +117,18 @@ func ErrAny(v any) zapcore.Field { // errMarshaler renders an error into ECS error.* keys at the encoder's // current namespace (no nesting). Used by Err via zap.Inline. +// +// Guards against typed-nil errors (non-nil interface holding a nil pointer) +// by checking before calling Error() — that call would otherwise panic on +// the nil receiver. type errMarshaler struct{ err error } func (m errMarshaler) MarshalLogObject(enc zapcore.ObjectEncoder) error { + if isTypedNil(m.err) { + enc.AddString("error.message", "") + enc.AddString("error.type", fmt.Sprintf("%T", m.err)) + return nil + } enc.AddString("error.message", m.err.Error()) enc.AddString("error.type", fmt.Sprintf("%T", m.err)) if stack := extractStackTrace(m.err); len(stack) > 0 { @@ -124,17 +137,14 @@ func (m errMarshaler) MarshalLogObject(enc zapcore.ObjectEncoder) error { return nil } -// errAnyMarshaler renders any value into ECS error.* keys, handling the -// typed-nil error gotcha and non-error values. Used by ErrAny via zap.Inline. +// errAnyMarshaler renders any value into ECS error.* keys, routing error +// values through errMarshaler (which handles typed-nil) and falling back to +// fmt.Sprint / fmt.Sprintf("%T") for non-error values. Used by ErrAny via +// zap.Inline. type errAnyMarshaler struct{ v any } func (m errAnyMarshaler) MarshalLogObject(enc zapcore.ObjectEncoder) error { if err, ok := m.v.(error); ok { - if isTypedNil(err) { - enc.AddString("error.message", "") - enc.AddString("error.type", fmt.Sprintf("%T", err)) - return nil - } return errMarshaler{err: err}.MarshalLogObject(enc) } enc.AddString("error.message", fmt.Sprint(m.v)) diff --git a/zap/error_test.go b/zap/error_test.go index a24c5b1..524cd39 100644 --- a/zap/error_test.go +++ b/zap/error_test.go @@ -4,6 +4,7 @@ package zap_test import ( "errors" + "fmt" "testing" pkgerrors "github.com/pkg/errors" @@ -68,7 +69,7 @@ func TestErr_StdlibError_NoStackTrace(t *testing.T) { got := emit(ecszap.Err(err)) assert.Equal(t, "boom", got["error.message"]) - assert.Equal(t, "*errors.errorString", got["error.type"]) + assert.Equal(t, fmt.Sprintf("%T", err), got["error.type"]) assert.NotContains(t, got, "error.stack_trace") } @@ -85,7 +86,7 @@ func TestErr_BytesStackTracer_EmitsStackTrace(t *testing.T) { got := emit(ecszap.Err(err)) assert.Equal(t, "kaboom", got["error.message"]) - assert.Equal(t, "*zap_test.fakeStackTracer", got["error.type"]) + assert.Equal(t, fmt.Sprintf("%T", err), got["error.type"]) assert.Equal(t, "goroutine 1...", got["error.stack_trace"]) } @@ -114,7 +115,7 @@ func TestErrAny_Error_DelegatesToErr(t *testing.T) { got := emit(ecszap.ErrAny(err)) assert.Equal(t, "boom", got["error.message"]) - assert.Equal(t, "*errors.errorString", got["error.type"]) + assert.Equal(t, fmt.Sprintf("%T", err), got["error.type"]) assert.NotContains(t, got, "error.stack_trace") } @@ -123,22 +124,24 @@ func TestErrAny_StackTracerError_IncludesStackTrace(t *testing.T) { got := emit(ecszap.ErrAny(err)) assert.Equal(t, "kaboom", got["error.message"]) - assert.Equal(t, "*zap_test.fakeStackTracer", got["error.type"]) + assert.Equal(t, fmt.Sprintf("%T", err), got["error.type"]) assert.Equal(t, "goroutine 1...", got["error.stack_trace"]) } func TestErrAny_String(t *testing.T) { - got := emit(ecszap.ErrAny("oops")) + v := "oops" + got := emit(ecszap.ErrAny(v)) - assert.Equal(t, "oops", got["error.message"]) - assert.Equal(t, "string", got["error.type"]) + assert.Equal(t, v, got["error.message"]) + assert.Equal(t, fmt.Sprintf("%T", v), got["error.type"]) } func TestErrAny_Int(t *testing.T) { - got := emit(ecszap.ErrAny(42)) + v := 42 + got := emit(ecszap.ErrAny(v)) assert.Equal(t, "42", got["error.message"]) - assert.Equal(t, "int", got["error.type"]) + assert.Equal(t, fmt.Sprintf("%T", v), got["error.type"]) } type panicPayload struct { @@ -146,10 +149,11 @@ type panicPayload struct { } func TestErrAny_Struct(t *testing.T) { - got := emit(ecszap.ErrAny(panicPayload{Reason: "deadlocked"})) + v := panicPayload{Reason: "deadlocked"} + got := emit(ecszap.ErrAny(v)) assert.Equal(t, "{deadlocked}", got["error.message"]) - assert.Equal(t, "zap_test.panicPayload", got["error.type"]) + assert.Equal(t, fmt.Sprintf("%T", v), got["error.type"]) } // derefingErr panics on Error() if the receiver is nil — emulating the common @@ -169,5 +173,18 @@ func TestErrAny_TypedNilPointerError_DoesNotPanic(t *testing.T) { }) assert.Equal(t, "", got["error.message"]) - assert.Equal(t, "*zap_test.derefingErr", got["error.type"]) + assert.Equal(t, fmt.Sprintf("%T", asInterface), got["error.type"]) +} + +func TestErr_TypedNilPointerError_DoesNotPanic(t *testing.T) { + var typedNil *derefingErr + var asInterface error = typedNil + + var got map[string]any + require.NotPanics(t, func() { + got = emit(ecszap.Err(asInterface)) + }) + + assert.Equal(t, "", got["error.message"]) + assert.Equal(t, fmt.Sprintf("%T", asInterface), got["error.type"]) } From 09cf6cf56680604ef971f35b6824c0d24c0f4b65 Mon Sep 17 00:00:00 2001 From: Maxence Yang Date: Wed, 6 May 2026 15:28:46 +0800 Subject: [PATCH 3/5] fix: guard extractStackTrace against typed-nil chain match 1.errors.As matched typed-nil tracer -> StackTrace panicked 2.Fix only safe up to first typed-nil -> document limitation 3.Wrapped typed-nil scenario lacked test -> add safeWrapper case --- zap/error.go | 14 ++++++++++++-- zap/error_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/zap/error.go b/zap/error.go index e3209ff..11e49ca 100644 --- a/zap/error.go +++ b/zap/error.go @@ -155,15 +155,25 @@ func (m errAnyMarshaler) MarshalLogObject(enc zapcore.ObjectEncoder) error { // extractStackTrace walks the error chain and returns the first stack trace // found, in []byte form ready for ErrorStackTrace. Returns nil if no error in // the chain carries a stack trace. +// +// Each errors.As match is guarded against typed-nil: if the matched value is +// a non-nil interface holding a nil concrete value, calling StackTrace() on +// it would panic on the nil receiver, so we treat that match as "no stack". +// +// Limitation: if the error chain contains a typed-nil stack-bearing error in +// front of a real stack-bearing error, only the first match is considered — +// errors.As stops there and we treat it as "no stack" rather than risk +// further traversal past a typed-nil. In practice typed-nil errors appear at +// the leaf, so this is rarely observable. func extractStackTrace(err error) []byte { var bytesST stackTracerBytes - if errors.As(err, &bytesST) { + if errors.As(err, &bytesST) && !isTypedNil(bytesST) { if s := bytesST.StackTrace(); len(s) > 0 { return s } } var pcsST stackTracerPCs - if errors.As(err, &pcsST) { + if errors.As(err, &pcsST) && !isTypedNil(pcsST) { if s := pcsST.StackTrace(); len(s) > 0 { // pkg/errors.StackTrace implements fmt.Formatter; %+v renders each // frame as "function\n\tfile:line", matching what users expect to diff --git a/zap/error_test.go b/zap/error_test.go index 524cd39..f5d688a 100644 --- a/zap/error_test.go +++ b/zap/error_test.go @@ -188,3 +188,29 @@ func TestErr_TypedNilPointerError_DoesNotPanic(t *testing.T) { assert.Equal(t, "", got["error.message"]) assert.Equal(t, fmt.Sprintf("%T", asInterface), got["error.type"]) } + +// safeWrapper wraps an error without dereferencing it in Error() — so a +// typed-nil error can be wrapped and the wrapper itself stays usable. Models +// the fmt.Errorf("ctx: %w", typedNilErr) pattern without fmt.Errorf's own +// formatting deref panic. +type safeWrapper struct { + msg string + inner error +} + +func (w *safeWrapper) Error() string { return w.msg } +func (w *safeWrapper) Unwrap() error { return w.inner } + +func TestErr_TypedNilStackTracerInChain_DoesNotPanic(t *testing.T) { + var typedNilTracer *fakeStackTracer + wrapped := &safeWrapper{msg: "outer", inner: typedNilTracer} + + var got map[string]any + require.NotPanics(t, func() { + got = emit(ecszap.Err(wrapped)) + }) + + assert.Equal(t, "outer", got["error.message"]) + assert.Equal(t, fmt.Sprintf("%T", wrapped), got["error.type"]) + assert.NotContains(t, got, "error.stack_trace") +} From 6fa4e9faf0cf46b4bb4e686178a82c21cca488ae Mon Sep 17 00:00:00 2001 From: Maxence Yang Date: Wed, 6 May 2026 15:40:27 +0800 Subject: [PATCH 4/5] doc: drop stale multi-field wording in error.go header 1.v0.3.0 returns single inline Field -> wording contradicted code 2.Multi-field tag added no info -> describe behavior directly --- zap/error.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/zap/error.go b/zap/error.go index 11e49ca..56cdc5e 100644 --- a/zap/error.go +++ b/zap/error.go @@ -3,10 +3,10 @@ // ECS reference (8.17): https://www.elastic.co/guide/en/ecs/8.17/ecs-error.html // // The package exposes single-field constructors (uniform with the rest of the -// library) plus two multi-field convenience helpers, Err() and ErrAny(), that -// pack the standard error.* fields into a single inline zap.Field. Callers do -// not need any specific zap encoder — output is flat dotted ECS keys, the -// same shape every other constructor in this package emits. +// library) plus two convenience helpers, Err() and ErrAny(), that pack the +// standard error.* fields into a single inline zap.Field via zap.Inline. +// Callers do not need any specific zap encoder — output is flat dotted ECS +// keys, the same shape every other constructor in this package emits. package zap From 02dbc4db6ec61eeb5049663ec7380858f963f248 Mon Sep 17 00:00:00 2001 From: Maxence Yang Date: Wed, 6 May 2026 15:58:55 +0800 Subject: [PATCH 5/5] doc: qualify CHANGELOG migration snippet with ecsf alias 1.Snippet used unqualified names -> add ecsf prefix 2.Mentioned no import context -> note the recommended alias --- CHANGELOG.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0fa8cc..b6ce059 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,17 +13,20 @@ Migration: + Migration assumes the recommended import alias + `ecsf "github.com/maxence2997/ecsfields/zap"`: + ```go // Before (v0.2.0): - fields := []zap.Field{ServiceName("auth"), EventAction("login")} - fields = append(fields, Err(err)...) + fields := []zap.Field{ecsf.ServiceName("auth"), ecsf.EventAction("login")} + fields = append(fields, ecsf.Err(err)...) logger.Error("failed", fields...) // After (v0.3.0): logger.Error("failed", - ServiceName("auth"), - EventAction("login"), - Err(err), + ecsf.ServiceName("auth"), + ecsf.EventAction("login"), + ecsf.Err(err), ) ```