fix: deep review improvements — bug fixes, docs, and test coverage (#107)

* fix: deep review improvements — bug fixes, docs, and test coverage

- Fix README typos: undefined var `err` -> `err1`, `.Tag()` -> `.Tags()`
- Deep-copy tags in builder.copy() with slices.Clone to prevent shared-array bugs
- Add panic recovery to lazyValueEvaluation (produces descriptive string on panic)
- Replace lo.Assign with maps.Clone in builder.copy() (stdlib, idiomatic)
- Document all global config vars (AutoTraceID, DereferencePointers, Local) in README
- Document Wrapf + %w foot-gun behavior in README
- Add logrus formatter tests (was zero coverage)
- Add AsOops/AsError[T] helper tests
- Add Format %s/%q verb coverage
- Add benchmark suite for core operations

* oops

* fix: resolve lint errors and test failures

- fix unchecked errors on logger.Sync, AddObject, AddReflected
- replace unsafe type assertion with AsOops + ok check in logrus test
- normalize trailing whitespace on blank HTTP request lines in test
- add missing slog.KindAny case in builder
- modernize interface{} to any in zerolog formatter and kv test
- add nolint directives for intentional testifylint/paralleltest cases

* fix(builder): ensure context/userData/tenantData are never nil after copy()

maps.Clone(nil) returns nil, causing panics when callers like With,
WithContext, User, and Tenant attempt to assign into those maps.
Add nil-guards after cloning so the maps are always initialized.

Author: samber

README.md

@@ -348,7 +348,7 @@ oops.SourceFragmentsHidden = false
 
 err1 := oops.Errorf("permission denied")
 // ...
-err2 := oops.Wrapf(err, "something failed")
+err2 := oops.Wrapf(err1, "something failed")
 
 fmt.Println(err2.(oops.OopsError).Sources())
 ```
@@ -473,12 +473,15 @@ attr := slog.Error(err.Error(),
 // slog.Group("error", ...)
 ```
 
-#### Custom timezone
+#### Global configuration
 
-```go
-loc, _ := time.LoadLocation("Europe/Paris")
-oops.Local = loc
-```
+| Variable | Default | Description |
+|---|---|---|
+| `oops.StackTraceMaxDepth` | `10` | Max stack trace depth |
+| `oops.SourceFragmentsHidden` | `true` | Hide source code fragments |
+| `oops.Local` | `time.UTC` | Timezone for error timestamps |
+| `oops.AutoTraceID` | `true` | Auto-generate ULID trace IDs |
+| `oops.DereferencePointers` | `true` | Auto-dereference pointers in context |
 
 ### Go context
 
@@ -488,7 +491,7 @@ An `OopsErrorBuilder` can be transported in a go `context.Context` to reuse late
 func myFunc(ctx context.Context) {
     oops.
         FromContext(ctx).
-        Tag("auth").
+        Tags("auth").
         Errorf("not permitted")
 }
 
@@ -536,6 +539,8 @@ userMessage := oops.GetPublic(err, "Unexpected error")
 
 ### Wrap/Wrapf shortcut
 
+> **Note:** `oops.Wrapf(err, "msg: %w", otherErr)` does **not** chain `otherErr` — the `%w` verb is only used for string formatting. The error chain is always `result → err`. Use `oops.Errorf("msg: %w", otherErr)` if you need `%w` chaining.
+
 `oops.Wrap(...)` and `oops.Wrapf(...)` returns nil if the provided `error` is nil.
 
 ❌ So don't write:

benchmarks/benchmark_test.go

@@ -0,0 +1,95 @@
+package benchmarks
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/samber/oops"
+)
+
+func BenchmarkNew(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		_ = oops.New("an error")
+	}
+}
+
+func BenchmarkErrorfSimple(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		_ = oops.Errorf("an error: %s", "details")
+	}
+}
+
+func BenchmarkErrorfWrap(b *testing.B) {
+	inner := errors.New("inner")
+	for i := 0; i < b.N; i++ {
+		_ = oops.Errorf("wrapped: %w", inner)
+	}
+}
+
+func BenchmarkWrap(b *testing.B) {
+	inner := errors.New("inner")
+	for i := 0; i < b.N; i++ {
+		_ = oops.Wrap(inner)
+	}
+}
+
+func BenchmarkWrapf(b *testing.B) {
+	inner := errors.New("inner")
+	for i := 0; i < b.N; i++ {
+		_ = oops.Wrapf(inner, "context: %s", "details")
+	}
+}
+
+func BenchmarkBuilderWithContext(b *testing.B) {
+	inner := errors.New("inner")
+	for i := 0; i < b.N; i++ {
+		_ = oops.
+			In("database").
+			Code("db_error").
+			Tags("critical", "database").
+			With("query", "SELECT 1", "duration_ms", 42).
+			Hint("check connection pool").
+			Owner("backend-team").
+			Wrap(inner)
+	}
+}
+
+func BenchmarkChainTraversal(b *testing.B) {
+	err := oops.In("layer1").Wrap(
+		oops.In("layer2").Wrap(
+			oops.In("layer3").Errorf("root cause"),
+		),
+	)
+	oopsErr, _ := oops.AsOops(err)
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		_ = oopsErr.Domain()
+		_ = oopsErr.Context()
+		_ = oopsErr.Trace()
+		_ = oopsErr.Tags()
+	}
+}
+
+func BenchmarkToMap(b *testing.B) {
+	err := oops.
+		In("database").
+		Code("db_error").
+		Tags("critical").
+		With("query", "SELECT 1").
+		Owner("backend-team").
+		User("user-123", "name", "john").
+		Errorf("connection failed")
+	oopsErr, _ := oops.AsOops(err)
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		_ = oopsErr.ToMap()
+	}
+}
+
+func BenchmarkWrapNil(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		_ = oops.Wrap(nil)
+	}
+}

benchmarks/benchmarks.go

@@ -0,0 +1,2 @@
+// Package benchmarks contains performance benchmarks for the oops package.
+package benchmarks

benchmarks/go.mod

@@ -0,0 +1,15 @@
+module github.com/samber/oops/benchmarks
+
+go 1.21
+
+replace github.com/samber/oops => ../
+
+require github.com/samber/oops v0.0.0
+
+require (
+	github.com/oklog/ulid/v2 v2.1.1 // indirect
+	github.com/samber/lo v1.53.0 // indirect
+	go.opentelemetry.io/otel v1.29.0 // indirect
+	go.opentelemetry.io/otel/trace v1.29.0 // indirect
+	golang.org/x/text v0.22.0 // indirect
+)

benchmarks/go.sum

@@ -0,0 +1,23 @@
+github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
+github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
+github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
+github.com/oklog/ulid/v2 v2.1.1 h1:suPZ4ARWLOJLegGFiZZ1dFAkqzhMjL3J1TzI+5wHz8s=
+github.com/oklog/ulid/v2 v2.1.1/go.mod h1:rcEKHmBBKfef9DhnvX7y1HZBYxjXb0cP5ExxNsTT1QQ=
+github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o=
+github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
+github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/samber/lo v1.53.0 h1:t975lj2py4kJPQ6haz1QMgtId2gtmfktACxIXArw3HM=
+github.com/samber/lo v1.53.0/go.mod h1:4+MXEGsJzbKGaUEQFKBq2xtfuznW9oz/WrgyzMzRoM0=
+github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U=
+github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U=
+go.opentelemetry.io/otel v1.29.0 h1:PdomN/Al4q/lN6iBJEN3AwPvUiHPMlt93c8bqTG5Llw=
+go.opentelemetry.io/otel v1.29.0/go.mod h1:N/WtXPs1CNCUEx+Agz5uouwCba+i+bJGFicT8SR4NP8=
+go.opentelemetry.io/otel/trace v1.29.0 h1:J/8ZNK4XgR7a21DZUAsbF8pZ5Jcw1VhACmnYt39JTi4=
+go.opentelemetry.io/otel/trace v1.29.0/go.mod h1:eHl3w0sp3paPkYstJOmAimxhiFXPg+MMTlEh3nsQgWQ=
+go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
+go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
+golang.org/x/text v0.22.0 h1:bofq7m3/HAFvbF51jz3Q9wLg3jkvSPuiZu/pD1XwgtM=
+golang.org/x/text v0.22.0/go.mod h1:YRoo4H8PVmsu+E3Ou7cqLVH8oXWIHVoX0jqUWALQhfY=
+gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
+gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

builder.go

@@ -5,7 +5,9 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"maps"
 	"net/http"
+	"slices"
 	"time"
 
 	"github.com/oklog/ulid/v2"
@@ -97,16 +99,16 @@ func newBuilder() OopsErrorBuilder {
 // It performs deep copying of maps to ensure that modifications to the new
 // builder don't affect the original.
 func (o OopsErrorBuilder) copy() OopsErrorBuilder {
-	return OopsErrorBuilder{
+	o2 := OopsErrorBuilder{
 		// err:      err,  // Not copied as it's set by error creation methods
 		// msg:      o.msg, // Not copied as it's set by error creation methods
 		code:     o.code,
 		time:     o.time,
 		duration: o.duration,
 
 		domain:  o.domain,
-		tags:    o.tags,
-		context: lo.Assign(map[string]any{}, o.context), // Deep copy of context map (pointer values are not copied)
+		tags:    slices.Clone(o.tags),
+		context: maps.Clone(o.context), // Deep copy of context map (pointer values are not copied)
 
 		trace:              o.trace,
 		traceAutoGenerated: o.traceAutoGenerated,
@@ -117,15 +119,27 @@ func (o OopsErrorBuilder) copy() OopsErrorBuilder {
 		owner:  o.owner,
 
 		userID:     o.userID,
-		userData:   lo.Assign(map[string]any{}, o.userData), // Deep copy of user data (pointer values are not copied)
+		userData:   maps.Clone(o.userData), // Deep copy of user data (pointer values are not copied)
 		tenantID:   o.tenantID,
-		tenantData: lo.Assign(map[string]any{}, o.tenantData), // Deep copy of tenant data (pointer values are not copied)
+		tenantData: maps.Clone(o.tenantData), // Deep copy of tenant data (pointer values are not copied)
 
 		req: o.req,
 		res: o.res,
 
 		// stacktrace: o.stacktrace, // Not copied as it's generated per error
 	}
+	// maps.Clone(nil) returns nil; ensure maps are never nil so callers can
+	// assign into them without panicking (e.g. With, WithContext, User, Tenant).
+	if o2.context == nil {
+		o2.context = map[string]any{}
+	}
+	if o2.userData == nil {
+		o2.userData = map[string]any{}
+	}
+	if o2.tenantData == nil {
+		o2.tenantData = map[string]any{}
+	}
+	return o2
 }
 
 // Wrap wraps an existing error into an OopsError with the current builder's context.
@@ -625,6 +639,8 @@ func slogValueToAny(value slog.Value) any {
 		return group
 	case slog.KindLogValuer:
 		return slogValueToAny(value.LogValuer().LogValue())
+	case slog.KindAny:
+		return value.Any()
 	default:
 		return value.Any()
 	}

error_test.go

@@ -43,8 +43,8 @@ func TestErrorsIs(t *testing.T) {
 	// Previously, Is() returned true for any OopsError target regardless of identity.
 	err1 := Errorf("error 1")
 	err2 := Errorf("error 2")
-	is.False(errors.Is(err1, err2))
-	is.False(errors.Is(err2, err1))
+	is.False(errors.Is(err1, err2)) //nolint:testifylint
+	is.False(errors.Is(err2, err1)) //nolint:testifylint
 }
 
 func TestErrorsAs(t *testing.T) {

examples/zap/example.go

@@ -49,7 +49,7 @@ func main() {
 	config.EncoderConfig.TimeKey = "timestamp"
 	config.EncoderConfig.EncodeTime = zapcore.RFC3339TimeEncoder
 	logger, _ := config.Build()
-	defer logger.Sync()
+	defer func() { _ = logger.Sync() }()
 
 	err := a()
 

go.work

@@ -20,4 +20,7 @@ use (
 
 	// recovery middlewares
 	./recovery/gin
+
+	// benchmarks
+	./benchmarks
 )

helpers_test.go

@@ -0,0 +1,67 @@
+package oops
+
+import (
+	"errors"
+	"fmt"
+	"io/fs"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAsOops(t *testing.T) {
+	t.Parallel()
+	is := assert.New(t)
+
+	// OopsError is found
+	err := In("test").With("key", "value").Errorf("oops error")
+	oopsErr, ok := AsOops(err)
+	is.True(ok)
+	is.Equal("test", oopsErr.Domain())
+	is.Equal(map[string]any{"key": "value"}, oopsErr.Context())
+
+	// Non-oops error returns false
+	plainErr := errors.New("plain error")
+	_, ok = AsOops(plainErr)
+	is.False(ok)
+
+	// Nil error returns false
+	_, ok = AsOops(nil)
+	is.False(ok)
+
+	// OopsError wrapped by fmt.Errorf is found
+	wrapped := fmt.Errorf("wrapper: %w", err)
+	oopsErr, ok = AsOops(wrapped)
+	is.True(ok)
+	is.Equal("test", oopsErr.Domain())
+
+	// Deeply nested OopsError is found
+	deepWrapped := fmt.Errorf("outer: %w", fmt.Errorf("inner: %w", err))
+	oopsErr, ok = AsOops(deepWrapped)
+	is.True(ok)
+	is.Equal("test", oopsErr.Domain())
+}
+
+func TestAsError(t *testing.T) {
+	t.Parallel()
+	is := assert.New(t)
+
+	// Matches concrete error type
+	err := Wrapf(fs.ErrExist, "wrapped")
+	fsErr, ok := AsError[*fs.PathError](fmt.Errorf("path: %w", &fs.PathError{Op: "open", Path: "/tmp", Err: fs.ErrExist}))
+	is.True(ok)
+	is.Equal("open", fsErr.Op)
+
+	// OopsError matched via AsError
+	oopsErr, ok := AsError[OopsError](err)
+	is.True(ok)
+	is.NotEmpty(oopsErr.Span())
+
+	// Non-matching type returns false
+	_, ok = AsError[*fs.PathError](errors.New("not a path error"))
+	is.False(ok)
+
+	// Nil error returns false
+	_, ok = AsError[OopsError](nil)
+	is.False(ok)
+}