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..872f66c02fe 100644 --- a/pkg/errors/error_capture.go +++ b/pkg/errors/error_capture.go @@ -1,5 +1,10 @@ package errors +import ( + stderrors "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 = stderrors.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..d8fbb94d226 --- /dev/null +++ b/pkg/errors/error_capture_test.go @@ -0,0 +1,202 @@ +package errors + +import ( + stderrors "errors" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestCapture(t *testing.T) { + errClose := stderrors.New("close error") + errOriginal := stderrors.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 := stderrors.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 := stderrors.New("close error") + errOriginal := stderrors.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 := stderrors.New("close error") + errOriginal := stderrors.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 := 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) + 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 := 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) + 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 := stderrors.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 := stderrors.New("close error") + errOriginal := stderrors.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 := stderrors.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`) +}