diff --git a/error.go b/error.go index 56fc988..85f6e8f 100644 --- a/error.go +++ b/error.go @@ -286,13 +286,19 @@ func (o OopsError) Domain() string { // Tags are merged from all errors in the chain and deduplicated. func (o OopsError) Tags() []string { tags := make([]string, 0, 8) // reasonable initial capacity for tags + seen := map[string]struct{}{} recursive(o, func(e OopsError) bool { - tags = append(tags, e.tags...) + for _, tag := range e.tags { + if _, ok := seen[tag]; !ok { + tags = append(tags, tag) + seen[tag] = struct{}{} + } + } return true }) - return lo.Uniq(tags) + return tags } // HasTag checks if the error or any of its wrapped errors contain the specified tag. diff --git a/kv.go b/kv.go index 76c519d..a72810e 100644 --- a/kv.go +++ b/kv.go @@ -29,23 +29,44 @@ import ( // result := dereferencePointers(data) // // result["user"] will be User{Name: "John"} instead of *User // // result["count"] will be 42 instead of *int +func derefPtr[T any](p *T) any { + if p == nil { + return nil + } + return *p +} + func dereferencePointers(data map[string]any) map[string]any { if !DereferencePointers { return data } for key, value := range data { - // Fast path: only use reflect for types that could be pointers - switch value.(type) { + switch v := value.(type) { case nil, string, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, float32, float64, bool, []byte, map[string]any, []any: continue // not a pointer, skip - } - val := reflect.ValueOf(value) - if val.Kind() == reflect.Pointer { - data[key] = dereferencePointerRecursive(val, 0) + case *string: + data[key] = derefPtr(v) + case *int: + data[key] = derefPtr(v) + case *int64: + data[key] = derefPtr(v) + case *uint: + data[key] = derefPtr(v) + case *uint64: + data[key] = derefPtr(v) + case *float64: + data[key] = derefPtr(v) + case *bool: + data[key] = derefPtr(v) + default: + val := reflect.ValueOf(value) + if val.Kind() == reflect.Pointer { + data[key] = dereferencePointerRecursive(val, 0) + } } } diff --git a/kv_test.go b/kv_test.go index 273fe57..1c3ce1d 100644 --- a/kv_test.go +++ b/kv_test.go @@ -6,6 +6,7 @@ import ( "fmt" "reflect" "testing" + "time" "unsafe" "github.com/stretchr/testify/assert" @@ -124,12 +125,93 @@ func TestDereferencePointerEdgeCases(t *testing.T) { { name: "int_pointer", run: func(is *assert.Assertions) { - intValue := 42 - intPtr := &intValue - err := With("int", intPtr).Errorf(anErrorStr).(OopsError) + v := 42 + err := With("int", &v).Errorf(anErrorStr).(OopsError) is.Equal(map[string]any{"int": 42}, err.Context()) }, }, + { + name: "nil_int_fast_path", + run: func(is *assert.Assertions) { + err := With("v", (*int)(nil)).Errorf(anErrorStr).(OopsError) + is.Equal(map[string]any{"v": nil}, err.Context()) + }, + }, + { + name: "int64_pointer", + run: func(is *assert.Assertions) { + v := int64(100) + err := With("v", &v).Errorf(anErrorStr).(OopsError) + is.Equal(map[string]any{"v": int64(100)}, err.Context()) + }, + }, + { + name: "nil_int64_fast_path", + run: func(is *assert.Assertions) { + err := With("v", (*int64)(nil)).Errorf(anErrorStr).(OopsError) + is.Equal(map[string]any{"v": nil}, err.Context()) + }, + }, + { + name: "uint_pointer", + run: func(is *assert.Assertions) { + v := uint(7) + err := With("v", &v).Errorf(anErrorStr).(OopsError) + is.Equal(map[string]any{"v": uint(7)}, err.Context()) + }, + }, + { + name: "nil_uint_fast_path", + run: func(is *assert.Assertions) { + err := With("v", (*uint)(nil)).Errorf(anErrorStr).(OopsError) + is.Equal(map[string]any{"v": nil}, err.Context()) + }, + }, + { + name: "uint64_pointer", + run: func(is *assert.Assertions) { + v := uint64(999) + err := With("v", &v).Errorf(anErrorStr).(OopsError) + is.Equal(map[string]any{"v": uint64(999)}, err.Context()) + }, + }, + { + name: "nil_uint64_fast_path", + run: func(is *assert.Assertions) { + err := With("v", (*uint64)(nil)).Errorf(anErrorStr).(OopsError) + is.Equal(map[string]any{"v": nil}, err.Context()) + }, + }, + { + name: "float64_pointer", + run: func(is *assert.Assertions) { + v := 3.14 + err := With("v", &v).Errorf(anErrorStr).(OopsError) + is.Equal(map[string]any{"v": 3.14}, err.Context()) + }, + }, + { + name: "nil_float64_fast_path", + run: func(is *assert.Assertions) { + err := With("v", (*float64)(nil)).Errorf(anErrorStr).(OopsError) + is.Equal(map[string]any{"v": nil}, err.Context()) + }, + }, + { + name: "bool_pointer", + run: func(is *assert.Assertions) { + v := true + err := With("v", &v).Errorf(anErrorStr).(OopsError) + is.Equal(map[string]any{"v": true}, err.Context()) + }, + }, + { + name: "nil_bool_fast_path", + run: func(is *assert.Assertions) { + err := With("v", (*bool)(nil)).Errorf(anErrorStr).(OopsError) + is.Equal(map[string]any{"v": nil}, err.Context()) + }, + }, { name: "struct_pointer", run: func(is *assert.Assertions) { @@ -809,3 +891,176 @@ func TestCollectMaps(t *testing.T) { }) } } + +func TestSnapshot(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + run func(is *assert.Assertions) + }{ + { + name: "single_error_scalar_fields_preserved", + run: func(is *assert.Assertions) { + now := time.Now().Truncate(time.Second) + dur := 5 * time.Second + err := Code("E001"). + Time(now). + Duration(dur). + In("payments"). + Hint("check config"). + Public("payment failed"). + Owner("team-payments"). + With("key", "val"). + Tags("tag1", "tag2"). + Errorf("base error").(OopsError) + s := snapshot(err) + is.Equal("E001", s.code) + is.Equal(now, s.time) + is.Equal(dur, s.duration) + is.Equal("payments", s.domain) + is.Equal("check config", s.hint) + is.Equal("payment failed", s.public) + is.Equal("team-payments", s.owner) + is.Equal(map[string]any{"key": "val"}, s.context) + is.ElementsMatch([]string{"tag1", "tag2"}, s.tags) + }, + }, + { + name: "chained_innermost_scalar_wins", + run: func(is *assert.Assertions) { + inner := Code("INNER").In("inner-domain").Hint("inner-hint").Errorf("inner") + outer := Code("OUTER").In("outer-domain").Hint("outer-hint").Wrap(inner).(OopsError) + s := snapshot(outer) + is.Equal("INNER", s.code) + is.Equal("inner-domain", s.domain) + is.Equal("inner-hint", s.hint) + }, + }, + { + name: "chained_inner_code_with_outer_fallback", + run: func(is *assert.Assertions) { + inner := Errorf("inner") // no code + outer := Code("OUTER").Wrap(inner).(OopsError) + s := snapshot(outer) + is.Equal("OUTER", s.code) + }, + }, + { + name: "context_inner_key_wins_outer_key_merged", + run: func(is *assert.Assertions) { + inner := With("shared", "inner", "inner_only", 1).Errorf("inner") + outer := With("shared", "outer", "outer_only", 2).Wrap(inner).(OopsError) + s := snapshot(outer) + is.Equal("inner", s.context["shared"]) + is.Equal(1, s.context["inner_only"]) + is.Equal(2, s.context["outer_only"]) + }, + }, + { + name: "tags_deduplicated_across_chain", + run: func(is *assert.Assertions) { + inner := Tags("a", "b").Errorf("inner") + outer := Tags("b", "c").Wrap(inner).(OopsError) + s := snapshot(outer) + is.ElementsMatch([]string{"a", "b", "c"}, s.tags) + }, + }, + { + name: "explicit_trace_wins_over_auto_generated", + run: func(is *assert.Assertions) { + // outer has explicit trace; inner gets auto-generated + inner := Errorf("inner") // traceAutoGenerated=true + outer := Trace("explicit-trace-id").Wrap(inner).(OopsError) + s := snapshot(outer) + is.Equal("explicit-trace-id", s.trace) + }, + }, + { + name: "auto_trace_used_when_no_explicit", + run: func(is *assert.Assertions) { + err := Errorf("err").(OopsError) + s := snapshot(err) + // AutoTraceID is on by default: a non-empty trace should be present + is.NotEmpty(s.trace) + }, + }, + { + name: "context_lazy_value_evaluated", + run: func(is *assert.Assertions) { + err := With("computed", func() any { return "evaluated" }).Errorf("err").(OopsError) + s := snapshot(err) + is.Equal("evaluated", s.context["computed"]) + }, + }, + { + name: "context_pointer_dereferenced", + run: func(is *assert.Assertions) { + val := "deref_me" + err := With("ptr", &val).Errorf("err").(OopsError) + s := snapshot(err) + is.Equal("deref_me", s.context["ptr"]) + }, + }, + { + name: "user_innermost_wins_data_merged", + run: func(is *assert.Assertions) { + inner := User("uid-inner", "role", "admin", "name", "inner").Errorf("inner") + outer := User("uid-outer", "name", "outer").Wrap(inner).(OopsError) + s := snapshot(outer) + is.Equal("uid-inner", s.userID) + is.Equal("admin", s.userData["role"]) + is.Equal("inner", s.userData["name"]) // inner wins + }, + }, + { + name: "tenant_innermost_wins_data_merged", + run: func(is *assert.Assertions) { + inner := Tenant("tid-inner", "plan", "enterprise").Errorf("inner") + outer := Tenant("tid-outer", "plan", "free", "region", "us").Wrap(inner).(OopsError) + s := snapshot(outer) + is.Equal("tid-inner", s.tenantID) + is.Equal("enterprise", s.tenantData["plan"]) // inner wins + is.Equal("us", s.tenantData["region"]) // outer-only key preserved + }, + }, + { + name: "empty_error_zero_snapshot", + run: func(is *assert.Assertions) { + err := Errorf("bare").(OopsError) + s := snapshot(err) + is.Nil(s.code) + is.Empty(s.domain) + is.Empty(s.hint) + is.Empty(s.public) + is.Empty(s.owner) + is.Empty(s.userID) + is.Empty(s.tenantID) + is.Empty(s.tags) + is.Empty(s.context) + is.Empty(s.userData) + is.Empty(s.tenantData) + }, + }, + { + name: "snapshot_fields_not_cached_in_output", + run: func(is *assert.Assertions) { + // snapshot must not copy stacktrace, err, or cache fields + err := Code("X").With("k", "v").Errorf("msg").(OopsError) + s := snapshot(err) + is.NoError(s.err) + is.Nil(s.stacktrace) + is.Nil(s.cacheOnce) + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + is := assert.New(t) + tt.run(is) + }) + } +}