From 305f68ebf12f1dc5d63275a73236dfa5daee2c38 Mon Sep 17 00:00:00 2001 From: Geoffrey Wossum Date: Tue, 10 Feb 2026 12:34:57 -0600 Subject: [PATCH 1/2] feat: add Capturef Add Capturef to pkg/errors. Capturef allows including context information with the captured error messages. Additionally, tests have been added for Capture. CircleCI configuration has been modified to check format parameters of Capturef during CI pipeline. A .golangci.yml config file has also been added which adds the same check for `golangci-lint`. We do not currently use golangci-lint, but the config file has been added in case we do start using it. --- .circleci/config.yml | 2 +- .golangci.yml | 9 ++ pkg/errors/error_capture.go | 49 ++++++++ pkg/errors/error_capture_test.go | 202 +++++++++++++++++++++++++++++++ 4 files changed, 261 insertions(+), 1 deletion(-) create mode 100644 .golangci.yml create mode 100644 pkg/errors/error_capture_test.go diff --git a/.circleci/config.yml b/.circleci/config.yml index a85f330fa04..096b1609570 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -275,7 +275,7 @@ jobs: - checkout - run: ./checkfmt.sh - run: ./generate.sh - - run: go vet ./... + - run: go vet -printf.funcs=Capturef ./... unit_test: docker: diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000000..f0d95d2a32f --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,9 @@ +version: "2" + +linters: + settings: + govet: + settings: + printf: + funcs: + - Capturef diff --git a/pkg/errors/error_capture.go b/pkg/errors/error_capture.go index 7ecb1054571..7d4c0786e80 100644 --- a/pkg/errors/error_capture.go +++ b/pkg/errors/error_capture.go @@ -1,5 +1,10 @@ package errors +import ( + "errors" + "fmt" +) + // Capture is a wrapper function which can be used to capture errors from closing via a defer. // An example: // @@ -19,3 +24,47 @@ func Capture(rErr *error, fn func() error) func() { } } } + +// Capturef is similar to Capture but allows adding context when an error occurs. +// If an fn returns an error, then a new error is creating by concatenating ": %w" +// to fs and adding the error to the end of a. +// +// An additional difference to Capture is that if fn() returns an error, the +// new error is joined to the existing rErr using errors.Join. +// +// func Example(fn string) (rErr error) { +// f, err := os.Open(fn) +// if err != nil { +// return err +// } +// defer errors.Capturef(&rErr, f.Close, "error closing %q", fn)() +// .... +// } +// +// As an illustration, if Example is called with Example("meta.boltdb") and f.Close +// fails with a disk full error (syscall.ENOSPC), then the error string returned +// would be `error closing "meta.boltdb": no space left on device` and +// errors.Is(Example("meta.boltdb"), syscall.ENOSPC) would return true. +// +// If fs is empty, a is ignored and the error returned by fn() is appended +// to rErr unmodified. +func Capturef(rErr *error, fn func() error, fs string, a ...any) func() { + return func() { + if fn != nil { + if err := fn(); err != nil { + if rErr != nil { + // Eventually, the behavior regarding an empty fs will allow + // Capture to be rewritten as a simple wrapper around Capturef. + // This can happen once we decide Capture should use errors.Join. + if fs != "" { + args := make([]any, 0, len(a)+1) + args = append(args, a...) + args = append(args, err) + err = fmt.Errorf(fs+": %w", args...) + } + *rErr = errors.Join(*rErr, err) + } + } + } + } +} diff --git a/pkg/errors/error_capture_test.go b/pkg/errors/error_capture_test.go new file mode 100644 index 00000000000..5b525e7de4d --- /dev/null +++ b/pkg/errors/error_capture_test.go @@ -0,0 +1,202 @@ +package errors + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestCapture(t *testing.T) { + errClose := errors.New("close error") + errOriginal := errors.New("original error") + + tests := []struct { + name string + origErr error + closeErr error + wantErr error + }{ + { + name: "nil original, nil close", + origErr: nil, + closeErr: nil, + wantErr: nil, + }, + { + name: "nil original, non-nil close", + origErr: nil, + closeErr: errClose, + wantErr: errClose, + }, + { + name: "non-nil original, nil close", + origErr: errOriginal, + closeErr: nil, + wantErr: errOriginal, + }, + { + name: "non-nil original, non-nil close", + origErr: errOriginal, + closeErr: errClose, + wantErr: errOriginal, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.origErr + fn := Capture(&err, func() error { + return tt.closeErr + }) + fn() + + if tt.wantErr == nil { + require.NoError(t, err) + } else { + require.ErrorIs(t, err, tt.wantErr) + } + }) + } +} + +func TestCapture_DeferPattern(t *testing.T) { + errClose := errors.New("close error") + + fn := func() (err error) { + defer Capture(&err, func() error { + return errClose + })() + return nil + } + + require.ErrorIs(t, fn(), errClose) +} + +func TestCapture_DeferPatternPreservesOriginal(t *testing.T) { + errClose := errors.New("close error") + errOriginal := errors.New("original error") + + fn := func() (err error) { + defer Capture(&err, func() error { + return errClose + })() + return errOriginal + } + + err := fn() + require.ErrorIs(t, err, errOriginal) +} + +func TestCapturef(t *testing.T) { + errClose := errors.New("close error") + errOriginal := errors.New("original error") + + t.Run("nil fn is no-op", func(t *testing.T) { + var err error + fn := Capturef(&err, nil, "") + fn() + require.NoError(t, err) + }) + + t.Run("fn returns nil is no-op", func(t *testing.T) { + var err error + fn := Capturef(&err, func() error { return nil }, "") + fn() + require.NoError(t, err) + }) + + t.Run("nil rErr does not panic", func(t *testing.T) { + fn := Capturef(nil, func() error { return errClose }, "") + require.NotPanics(t, fn) + }) + + t.Run("captures close error when original is nil and format is empty", func(t *testing.T) { + var err error + fn := Capturef(&err, func() error { return errClose }, "") + fn() + require.ErrorIs(t, err, errClose) + require.EqualError(t, err, "close error") + }) + + t.Run("joins errors when both present and format is empty", func(t *testing.T) { + err := errOriginal + fn := Capturef(&err, func() error { return errClose }, "") + fn() + require.ErrorIs(t, err, errOriginal) + require.ErrorIs(t, err, errClose) + require.EqualError(t, err, "original error\nclose error") + }) + + t.Run("preserves original when fn returns nil", func(t *testing.T) { + err := errOriginal + fn := Capturef(&err, func() error { return nil }, "") + fn() + require.ErrorIs(t, err, errOriginal) + }) + + t.Run("wraps close error with format string", func(t *testing.T) { + var err error + errDisk := errors.New("no space left on device") + fn := Capturef(&err, func() error { return errDisk }, "error closing %q", "meta.boltdb") + fn() + require.ErrorIs(t, err, errDisk) + require.EqualError(t, err, `error closing "meta.boltdb": no space left on device`) + }) + + t.Run("joins formatted close error with original", func(t *testing.T) { + err := errOriginal + errDisk := errors.New("no space left on device") + fn := Capturef(&err, func() error { return errDisk }, "error closing %q", "meta.boltdb") + fn() + require.ErrorIs(t, err, errOriginal) + require.ErrorIs(t, err, errDisk) + require.EqualError(t, err, "original error\nerror closing \"meta.boltdb\": no space left on device") + }) +} + +func TestCapturef_DeferPattern(t *testing.T) { + errClose := errors.New("close error") + + fn := func() (err error) { + defer Capturef(&err, func() error { + return errClose + }, "closing resource")() + return nil + } + + err := fn() + require.ErrorIs(t, err, errClose) + require.EqualError(t, err, "closing resource: close error") +} + +func TestCapturef_DeferPatternJoinsErrors(t *testing.T) { + errClose := errors.New("close error") + errOriginal := errors.New("original error") + + fn := func() (err error) { + defer Capturef(&err, func() error { + return errClose + }, "closing resource")() + return errOriginal + } + + err := fn() + require.ErrorIs(t, err, errOriginal) + require.ErrorIs(t, err, errClose) + require.EqualError(t, err, "original error\nclosing resource: close error") +} + +func TestCapturef_FormatStringDocExample(t *testing.T) { + errDisk := errors.New("no space left on device") + + fn := func(name string) (rErr error) { + closer := func() error { return errDisk } + defer Capturef(&rErr, closer, "error closing %q", name)() + return nil + } + + err := fn("meta.boltdb") + require.ErrorIs(t, err, errDisk) + require.EqualError(t, err, `error closing "meta.boltdb": no space left on device`) +} From dea11488c0e1eb26472638699ad38a0c19548c56 Mon Sep 17 00:00:00 2001 From: Geoffrey Wossum Date: Wed, 11 Feb 2026 11:06:55 -0600 Subject: [PATCH 2/2] chore: alias stdlib errors import to avoid ambiguity in pkg/errors --- pkg/errors/error_capture.go | 4 ++-- pkg/errors/error_capture_test.go | 28 ++++++++++++++-------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/pkg/errors/error_capture.go b/pkg/errors/error_capture.go index 7d4c0786e80..872f66c02fe 100644 --- a/pkg/errors/error_capture.go +++ b/pkg/errors/error_capture.go @@ -1,7 +1,7 @@ package errors import ( - "errors" + stderrors "errors" "fmt" ) @@ -62,7 +62,7 @@ func Capturef(rErr *error, fn func() error, fs string, a ...any) func() { args = append(args, err) err = fmt.Errorf(fs+": %w", args...) } - *rErr = errors.Join(*rErr, err) + *rErr = stderrors.Join(*rErr, err) } } } diff --git a/pkg/errors/error_capture_test.go b/pkg/errors/error_capture_test.go index 5b525e7de4d..d8fbb94d226 100644 --- a/pkg/errors/error_capture_test.go +++ b/pkg/errors/error_capture_test.go @@ -1,15 +1,15 @@ package errors import ( - "errors" + stderrors "errors" "testing" "github.com/stretchr/testify/require" ) func TestCapture(t *testing.T) { - errClose := errors.New("close error") - errOriginal := errors.New("original error") + errClose := stderrors.New("close error") + errOriginal := stderrors.New("original error") tests := []struct { name string @@ -61,7 +61,7 @@ func TestCapture(t *testing.T) { } func TestCapture_DeferPattern(t *testing.T) { - errClose := errors.New("close error") + errClose := stderrors.New("close error") fn := func() (err error) { defer Capture(&err, func() error { @@ -74,8 +74,8 @@ func TestCapture_DeferPattern(t *testing.T) { } func TestCapture_DeferPatternPreservesOriginal(t *testing.T) { - errClose := errors.New("close error") - errOriginal := errors.New("original error") + errClose := stderrors.New("close error") + errOriginal := stderrors.New("original error") fn := func() (err error) { defer Capture(&err, func() error { @@ -89,8 +89,8 @@ func TestCapture_DeferPatternPreservesOriginal(t *testing.T) { } func TestCapturef(t *testing.T) { - errClose := errors.New("close error") - errOriginal := errors.New("original error") + errClose := stderrors.New("close error") + errOriginal := stderrors.New("original error") t.Run("nil fn is no-op", func(t *testing.T) { var err error @@ -137,7 +137,7 @@ func TestCapturef(t *testing.T) { t.Run("wraps close error with format string", func(t *testing.T) { var err error - errDisk := errors.New("no space left on device") + errDisk := stderrors.New("no space left on device") fn := Capturef(&err, func() error { return errDisk }, "error closing %q", "meta.boltdb") fn() require.ErrorIs(t, err, errDisk) @@ -146,7 +146,7 @@ func TestCapturef(t *testing.T) { t.Run("joins formatted close error with original", func(t *testing.T) { err := errOriginal - errDisk := errors.New("no space left on device") + errDisk := stderrors.New("no space left on device") fn := Capturef(&err, func() error { return errDisk }, "error closing %q", "meta.boltdb") fn() require.ErrorIs(t, err, errOriginal) @@ -156,7 +156,7 @@ func TestCapturef(t *testing.T) { } func TestCapturef_DeferPattern(t *testing.T) { - errClose := errors.New("close error") + errClose := stderrors.New("close error") fn := func() (err error) { defer Capturef(&err, func() error { @@ -171,8 +171,8 @@ func TestCapturef_DeferPattern(t *testing.T) { } func TestCapturef_DeferPatternJoinsErrors(t *testing.T) { - errClose := errors.New("close error") - errOriginal := errors.New("original error") + errClose := stderrors.New("close error") + errOriginal := stderrors.New("original error") fn := func() (err error) { defer Capturef(&err, func() error { @@ -188,7 +188,7 @@ func TestCapturef_DeferPatternJoinsErrors(t *testing.T) { } func TestCapturef_FormatStringDocExample(t *testing.T) { - errDisk := errors.New("no space left on device") + errDisk := stderrors.New("no space left on device") fn := func(name string) (rErr error) { closer := func() error { return errDisk }