Update and refactor error handling: enhance MultiError, add thread-safety, improve DataError functionality, and extend test coverage

Author: tomas-novotny

doc.go

@@ -1 +1,6 @@
+// Package errors provides an extended error type with structured key-value
+// fields, stack traces, and cause chaining, along with a thread-safe
+// multi-error container. It is a drop-in superset of the standard library
+// errors package: Unwrap, Is, and As are re-exported so callers only need to
+// import this package.
 package errors

error.go

@@ -4,26 +4,41 @@ import (
 	"errors"
 	"fmt"
 	"reflect"
+	"runtime"
 )
 
+// DataError is an error enriched with structured key-value fields, an optional
+// cause chain, and a captured stack trace. All methods that add data return a
+// new copy; the original is never mutated.
 type DataError struct {
 	err   error
 	data  map[string]any
 	cause error
+	stack []uintptr
 }
 
+// New creates a DataError from the given text and captures the current stack
+// trace.
 func New(text string) *DataError {
 	return &DataError{
-		err: errors.New(text),
+		err:   errors.New(text),
+		stack: callers(1),
 	}
 }
 
+// Newf creates a DataError from a formatted string and captures the current
+// stack trace.
 func Newf(format string, v ...any) *DataError {
 	return &DataError{
-		err: fmt.Errorf(format, v...),
+		err:   fmt.Errorf(format, v...),
+		stack: callers(1),
 	}
 }
 
+// Wrap converts any value into a *DataError and captures the current stack
+// trace. If err is nil, Wrap returns nil. If err is already a *DataError it is
+// returned unchanged. If err implements error it is wrapped directly; otherwise
+// its string representation is used as the error message.
 func Wrap(err any) *DataError {
 	if err == nil {
 		return nil
@@ -40,22 +55,45 @@ func Wrap(err any) *DataError {
 	}
 
 	return &DataError{
-		err: e,
+		err:   e,
+		stack: callers(1),
 	}
 }
 
+// callers returns up to 32 program counters starting skip frames above the
+// caller.
+func callers(skip int) []uintptr {
+	stack := make([]uintptr, 32)
+	n := runtime.Callers(skip+2, stack)
+
+	return stack[:n]
+}
+
+// Error returns the error message string.
 func (e *DataError) Error() string {
 	return e.err.Error()
 }
 
+// Fields returns the structured key-value data attached to this error.
 func (e *DataError) Fields() map[string]any {
 	return e.data
 }
 
+// StackTrace returns the program counters captured when the error was created.
+func (e *DataError) StackTrace() []uintptr {
+	return e.stack
+}
+
+// WithField returns a copy of the error with the given key-value field added.
+// The original error is not modified.
 func (e *DataError) WithField(key string, value any) *DataError {
 	return e.WithFields(map[string]any{key: value})
 }
 
+// WithFields returns a copy of the error with the given fields merged in.
+// The original error is not modified. Function values (including pointers to
+// functions) are silently ignored because they are not safely comparable or
+// serialisable.
 func (e *DataError) WithFields(values map[string]any) *DataError {
 	data := make(map[string]any, len(e.data)+len(values))
 	for k, v := range e.data {
@@ -77,17 +115,23 @@ func (e *DataError) WithFields(values map[string]any) *DataError {
 		err:   e.err,
 		data:  data,
 		cause: e.cause,
+		stack: e.stack,
 	}
 }
 
+// WithCause returns a copy of the error with the given cause attached. The
+// cause is returned by Unwrap, making it visible to errors.Is and errors.As.
 func (e *DataError) WithCause(err error) *DataError {
 	return &DataError{
 		err:   e.err,
 		data:  e.data,
 		cause: err,
+		stack: e.stack,
 	}
 }
 
+// Unwrap returns the cause if one was set via WithCause; otherwise it returns
+// the underlying error created by New, Newf, or Wrap.
 func (e *DataError) Unwrap() error {
 	if e.cause != nil {
 		return e.cause
@@ -96,13 +140,18 @@ func (e *DataError) Unwrap() error {
 	return e.err
 }
 
+// Is reports whether e matches target. Two *DataError values are considered
+// equal when their messages match and every field present in target also
+// appears in e with the same value. This allows errors.Is to find a sentinel
+// DataError anywhere in a chain, optionally scoped by fields.
 func (e *DataError) Is(target error) bool {
-	err, ok := target.(*DataError)
+	var err *DataError
+	ok := errors.As(target, &err)
 	if !ok {
 		return false
 	}
 
-	if e.Error() != target.Error() {
+	if e.Error() != err.Error() {
 		return false
 	}
 

error_test.go

@@ -8,88 +8,155 @@ import (
 	"github.com/gravitton/assert"
 )
 
-func TestFields(t *testing.T) {
-	err := Newf("Test DataError #%d: %s", 5, "failed to spawn")
-	assert.Equal(t, err.Error(), "Test DataError #5: failed to spawn")
-	assert.Length(t, err.Fields(), 0)
+func TestNew(t *testing.T) {
+	err := New("test")
 
-	err = Wrap("test")
 	assert.Equal(t, err.Error(), "test")
+	assert.Empty(t, err.Fields())
 
-	err = New("test3").WithField("action", "call")
-	assert.Equal(t, err.Error(), "test3")
-	assert.Equal(t, err.Fields()["action"], "call")
-	assert.NotContains(t, err.Fields(), "type")
+	cause := err.Unwrap()
+	assert.Equal(t, reflect.TypeOf(cause).String(), "*errors.errorString")
+}
 
-	err1 := Wrap(err)
-	assert.Same(t, err, err1)
+func TestNewf(t *testing.T) {
+	err := Newf("Test DataError #%d: %s", 5, "failed to spawn")
 
-	err2 := err.WithFields(map[string]any{"type": "warning"})
-	assert.NotSame(t, err1, err2)
-	assert.NotContains(t, err.Fields(), "type")
-	assert.Equal(t, err.Error(), "test3")
-	assert.Equal(t, err2.Fields()["type"], "warning")
-
-	err3 := err.WithField("action", "send")
-	assert.Equal(t, err2.Fields()["action"], "call")
-	assert.Equal(t, err3.Fields()["action"], "send")
-
-	err4 := New("error")
-	err5 := New("error")
-	assert.Equal(t, err4, err5)
-	assert.NotSame(t, err4, err5)
+	assert.Equal(t, err.Error(), "Test DataError #5: failed to spawn")
+	assert.Empty(t, err.Fields())
+
+	cause := err.Unwrap()
+	assert.Equal(t, reflect.TypeOf(cause).String(), "*errors.errorString")
+}
+
+func TestWrapNil(t *testing.T) {
+	err := Wrap(nil)
+
+	assert.NoError(t, err)
 }
 
-func TestUnwrap(t *testing.T) {
-	err := New("original error 1")
+func TestWrapNonError(t *testing.T) {
+	err := Wrap("something went wrong")
+
+	assert.Equal(t, err.Error(), "something went wrong")
 
 	cause := err.Unwrap()
+	assert.Equal(t, reflect.TypeOf(cause).String(), "*errors.errorString")
+}
+
+func TestWrapNonErrorNonString(t *testing.T) {
+	err := Wrap(159.5)
 
+	assert.Equal(t, err.Error(), "159.5")
+
+	cause := err.Unwrap()
 	assert.Equal(t, reflect.TypeOf(cause).String(), "*errors.errorString")
-	assert.Equal(t, "original error 1", cause.Error())
+}
+
+func TestWrapError(t *testing.T) {
+	original := errors.New("original")
+	err := Wrap(original)
+
+	assert.Equal(t, err.Error(), "original")
+
+	cause := err.Unwrap()
+	assert.Equal(t, cause, original)
+}
+
+func TestWrapDataError(t *testing.T) {
+	original := New("original")
+	err := Wrap(original)
+
+	assert.Same(t, err, original)
+}
+
+func TestStackTrace(t *testing.T) {
+	err1 := New("test")
+	err2 := Newf("test %d", 1)
+	err3 := Wrap(errors.New("std"))
+
+	assert.NotEmpty(t, err1.StackTrace())
+	assert.NotEmpty(t, err2.StackTrace())
+	assert.NotEmpty(t, err3.StackTrace())
+}
 
-	oErr := errors.New("original error 2")
-	err = Wrap(oErr)
+func TestFields(t *testing.T) {
+	err1 := New("test")
+
+	assert.Empty(t, err1.Fields())
 
-	assert.Same(t, oErr, err.Unwrap())
+	err2 := err1.WithField("action", "call")
+
+	assert.NotSame(t, err1, err2)
+	assert.Empty(t, err1.Fields())
+	assert.Equal(t, err2.Fields(), map[string]any{"action": "call"})
 
-	oErr2 := errors.New("original error 3")
-	err = err.WithCause(oErr2)
-	assert.Same(t, oErr2, err.Unwrap())
+	err3 := err2.WithFields(map[string]any{"type": "warning"})
+
+	assert.NotSame(t, err2, err3)
+	assert.Equal(t, err2.Fields(), map[string]any{"action": "call"})
+	assert.Equal(t, err3.Fields(), map[string]any{"action": "call", "type": "warning"})
+
+	err4 := err3.WithFields(map[string]any{"type": "error", "debug": true, "line": 15})
+
+	assert.NotSame(t, err3, err4)
+	assert.Equal(t, err3.Fields(), map[string]any{"action": "call", "type": "warning"})
+	assert.Equal(t, err4.Fields(), map[string]any{"action": "call", "type": "error", "debug": true, "line": 15})
+}
+
+func TestWithFieldsDropsFunctions(t *testing.T) {
+	err := New("test").WithFields(map[string]any{"key": "value", "func": func() {}})
+
+	assert.Equal(t, err.Fields(), map[string]any{"key": "value"})
+}
+
+func TestWithCause(t *testing.T) {
+	err1 := New("test")
+	original := errors.New("original error")
+
+	err2 := err1.WithCause(original)
+
+	assert.NotSame(t, err1, err2)
+	assert.Same(t, err2.Unwrap(), original)
 }
 
 func TestErrorsIs(t *testing.T) {
-	tests := []struct {
-		name string
-		err  error
-	}{
-		{
-			name: "*errors.errorString",
-			err:  errors.New("dummy error"),
-		},
-		{
-			name: "*DataError",
-			err:  New("dummy error"),
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			assert.ErrorIs(t, test.err, test.err)
-			assert.ErrorIs(t, Wrap(test.err), test.err)
-			assert.ErrorIs(t, Wrap(test.err).WithField("action", "call"), test.err)
-			assert.ErrorIs(t, Wrap(test.err).WithField("action", "call").WithField("type", "warning"), test.err)
-			assert.ErrorIs(t, Wrap(test.err).WithFields(map[string]any{"type": "warning"}), test.err)
-		})
-	}
+	original := errors.New("original error")
+	err1 := New("original error")
+
+	assert.NotErrorIs(t, err1, original)
+
+	err2 := Wrap(original)
+
+	assert.ErrorIs(t, err2, original)
+
+	err3 := err1.WithCause(original)
+
+	assert.ErrorIs(t, err3, original)
+
+	err4 := err2.WithField("action", "call")
+
+	assert.ErrorIs(t, err4, original)
 }
 
-func TestErrorsIsWithFields(t *testing.T) {
-	assert.ErrorIs(t, New("dummy error").WithField("action", "call"), New("dummy error"))
-	assert.NotErrorIs(t, New("dummy error"), New("dummy error").WithField("action", "call"))
-	assert.NotErrorIs(t, New("dummy error").WithField("action", "call"), New("dummy error").WithField("action", "send"))
+func TestErrorsIsDataError(t *testing.T) {
+	err1 := New("test")
+	err2 := New("test2")
+	err3 := New("test").WithFields(map[string]any{"action": "call", "type": "error"})
+	err4 := New("test").WithFields(map[string]any{"type": "warn"})
+
+	assert.NotErrorIs(t, err1, err2) // different error
+	assert.NotErrorIs(t, err1, err3) // additional fields
+	assert.NotErrorIs(t, err1, err4) // additional fields
+
+	assert.NotErrorIs(t, err2, err1) // different error
+	assert.NotErrorIs(t, err2, err3) // different error
+	assert.NotErrorIs(t, err2, err4) // different error
+
+	assert.ErrorIs(t, err3, err1)    // missing fields
+	assert.NotErrorIs(t, err3, err2) // different error
+	assert.NotErrorIs(t, err3, err4) // different fields
 
-	err := New("dummy error").WithField("module", "http")
-	assert.ErrorIs(t, err.WithField("add", false), err)
-	assert.NotErrorIs(t, err, err.WithField("add", false))
+	assert.ErrorIs(t, err4, err1)    // missing fields
+	assert.NotErrorIs(t, err4, err2) // different error
+	assert.NotErrorIs(t, err4, err3) // different fields
 }

go.mod

@@ -2,4 +2,4 @@ module github.com/gravitton/errors
 
 go 1.26
 
-require github.com/gravitton/assert v1.0.0
+require github.com/gravitton/assert v1.2.0

go.sum

@@ -1,2 +1,2 @@
-github.com/gravitton/assert v1.0.0 h1:VhxPuIN6KNpDXIdMtoKa0tGB/bkxZ0h1mm6jTzBkhnc=
-github.com/gravitton/assert v1.0.0/go.mod h1:wQGHJvwsxQQ7qdX++NLofrI2cJvYIdJJtAo7A15qcAY=
+github.com/gravitton/assert v1.2.0 h1:j9nvHz03FoaxSebCTd0Jr6RoDirs2lb51Eb2eVbV7a4=
+github.com/gravitton/assert v1.2.0/go.mod h1:AdPZ1NXH/Gfj2OkQq+2FvffggEk7lq2PzGPCe30qBe4=