Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add AppendInvoke convenience wrapper #47

Merged
merged 8 commits into from
May 6, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
56 changes: 56 additions & 0 deletions error.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,3 +475,59 @@ func AppendInto(into *error, err error) (errored bool) {
*into = Append(*into, err)
return true
}

// Invoker wraps a function that might return an error.
// It is used in combination with AppendInvoke to conveniently capture errors e.g. with defer
// statements without having to deal with anonymous inline functions.
// See also Invoke for wrapping any `func() error` without having to implement the
// Invoker interface directly.
// Close is a special Invoker wrapper for resources implementing the io.Closer interface.
type Invoker interface {
Invoke() error
}

// Invoke is an Invoker wrapper for arbitrary functions returning solely an error.
// It might be used e.g. for bufio.Scanner.Err() or similar functions in combination with
// AppendInvoke to capture cleanup errors without losing original error information
type Invoke func() error

func (cf Invoke) Invoke() error {
prashantv marked this conversation as resolved.
Show resolved Hide resolved
return cf()
}

// Close is a special wrapper for the common io.Closer interface.
// This wrapper is a specialization of Invoke for the common case where
// an IO resource needs to be closed after everything else is done.
//
// The following sample illustrates how to record the failure of the deferred Close() call
// without losing information about the original error
//
// func doSomething(..) (err error) {
// f := acquireResource()
// defer multierr.AppendInvoke(&err, multierr.Close(f))
// // ...
// }
func Close(closer io.Closer) Invoker {
return Invoke(closer.Close)
}

// AppendInvoke appends the result of the given Invoker into the given error
// It is a specialization of AppendInto that takes an Invoker to allow inlined defer statements without
// an anonymous function that e.g. invokes io.Closer.Close() or bufio.Scanner.Err() when the defer statement
// is executed.
// The following snippet illustrates the anti-pattern that leads to undesired behavior:
//
// defer multierr.AppendInto(&err, scanner.Err())
//
// declaring the defer statement evaluates the Err() call immediately and not at the execution time of the
// AppendInto call as possibly assumed.
//
// The following pattern achieves the desired behavior:
//
// defer multierr.AppendInvoke(&err, multierr.Invoke(scanner.Err))
//
// multierr.Invoke() casts any function returning solely an error to an Invoker and executes the captured
// function when multierr.AppendInvoke() is called i.e. after the enclosing function returned.
func AppendInvoke(into *error, call Invoker) {
AppendInto(into, call.Invoke())
prashantv marked this conversation as resolved.
Show resolved Hide resolved
}
120 changes: 120 additions & 0 deletions error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,126 @@ func TestAppendInto(t *testing.T) {
}
}

func TestAppendInvoke(t *testing.T) {
tests := []struct {
desc string
into *error
give Invoker
want error
}{
{
desc: "append into empty",
into: new(error),
give: Invoke(func() error {
return errors.New("foo")
}),
want: errors.New("foo"),
},
{
desc: "append into non-empty, non-multierr",
into: errorPtr(errors.New("foo")),
give: Invoke(func() error {
return errors.New("bar")
}),
want: Combine(
errors.New("foo"),
errors.New("bar"),
),
},
{
desc: "append into non-empty multierr",
into: errorPtr(Combine(
errors.New("foo"),
errors.New("bar"),
)),
give: Invoke(func() error {
return errors.New("baz")
}),
want: Combine(
errors.New("foo"),
errors.New("bar"),
errors.New("baz"),
),
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
AppendInvoke(tt.into, tt.give)
assert.Equal(t, tt.want, *tt.into)
})
}
}

type closerMock func() error

func (c closerMock) Close() error {
return c()
}

func newCloserMock(tb testing.TB, err error) io.Closer {
var closed bool
tb.Cleanup(func() {
if !closed {
tb.Error("closerMock wasn't closed before test end")
}
})

return closerMock(func() error {
closed = true
return err
})
}

func TestClose(t *testing.T) {
tests := []struct {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like these cases are exactly the same as AppendInvoke, should we instead share the cases? Then we could have the same cases be used across all variances:

  • an Invoke function that returns the specified error
  • an object implementing Invoker that returns the specified error
  • Closer
  • possible also with AppendInvokeFn if we add that method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting idea, unfortunately this is not directly possible.
You would need to recreate the inputs for every case that would 'reuse' the input because the into modifies the original input and I'm not aware of any good solution how to handle this without introducing either weird side effects or rather complex test input generation.

desc string
into *error
giveSetup func(tb testing.TB) Invoker
want error
}{
{
desc: "append invoker nil into empty",
into: new(error),
giveSetup: func(tb testing.TB) Invoker {
return Close(newCloserMock(tb, nil))
},
want: nil,
},
{
desc: "append invoker into non-empty, non-multierr",
into: errorPtr(errors.New("foo")),
giveSetup: func(tb testing.TB) Invoker {
return Close(newCloserMock(tb, errors.New("bar")))
},
want: Combine(
errors.New("foo"),
errors.New("bar"),
),
},
{
desc: "append invoker into non-empty multierr",
into: errorPtr(Combine(
errors.New("foo"),
errors.New("bar"),
)),
giveSetup: func(tb testing.TB) Invoker {
return Close(newCloserMock(tb, errors.New("baz")))
},
want: Combine(
errors.New("foo"),
errors.New("bar"),
errors.New("baz"),
),
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
AppendInvoke(tt.into, tt.giveSetup(t))
assert.Equal(t, tt.want, *tt.into)
})
}
}

func TestAppendIntoNil(t *testing.T) {
t.Run("nil pointer panics", func(t *testing.T) {
assert.Panics(t, func() {
Expand Down
62 changes: 62 additions & 0 deletions example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package multierr_test
import (
"errors"
"fmt"
"io"

"go.uber.org/multierr"
)
Expand Down Expand Up @@ -92,3 +93,64 @@ func ExampleAppendInto() {
// call 3 failed
// foo; baz
}

func ExampleAppendInvoke() {
var err error

var errFunc1 multierr.Invoker = multierr.Invoke(func() error {
fmt.Println("call 1 failed")
return errors.New("foo")
})

var errFunc2 multierr.Invoker = multierr.Invoke(func() error {
fmt.Println("call 2 did not fail")
return nil
})

var errFunc3 multierr.Invoker = multierr.Invoke(func() error {
fmt.Println("call 3 failed")
return errors.New("baz")
})

defer func() {
fmt.Println(err)
}()
Comment on lines +115 to +117
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional: I wonder if we should do a full file example instead, as it may be easier to see the print from a separate function that calls a function using AppendInvoke. It's a little easier to think of this as the result the caller sees IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean moving this to a new file only containing this very example and refactoring the anonymous function to normal functions?

defer multierr.AppendInvoke(&err, errFunc3)
defer multierr.AppendInvoke(&err, errFunc2)
defer multierr.AppendInvoke(&err, errFunc1)

// Output:
// call 1 failed
// call 2 did not fail
// call 3 failed
// foo; baz
}

type fakeCloser func() error

func (f fakeCloser) Close() error {
return f()
}

func FakeCloser(err error) io.Closer {
return fakeCloser(func() error {
return err
})
}

func ExampleClose() {
var err error

closer := FakeCloser(errors.New("foo"))

defer func() {
fmt.Println(err)
}()
defer multierr.AppendInvoke(&err, multierr.Close(closer))

fmt.Println("Hello, World")

// Output:
// Hello, World
// foo
}