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 all commits
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
176 changes: 169 additions & 7 deletions error.go
Expand Up @@ -35,8 +35,53 @@
//
// err = multierr.Append(reader.Close(), writer.Close())
//
// This makes it possible to record resource cleanup failures from deferred
// blocks with the help of named return values.
// The underlying list of errors for a returned error object may be retrieved
// with the Errors function.
//
// errors := multierr.Errors(err)
// if len(errors) > 0 {
// fmt.Println("The following errors occurred:", errors)
// }
//
// Appending from a loop
//
// You sometimes need to append into an error from a loop.
//
// var err error
// for _, item := range items {
// err = multierr.Append(err, process(item))
// }
//
// Cases like this may require knowledge of whether an individual instance
// failed. This usually requires introduction of a new variable.
//
// var err error
// for _, item := range items {
// if perr := process(item); perr != nil {
// log.Warn("skipping item", item)
// err = multierr.Append(err, perr)
// }
// }
//
// multierr includes AppendInto to simplify cases like this.
//
// var err error
// for _, item := range items {
// if multierr.AppendInto(&err, process(item)) {
// log.Warn("skipping item", item)
// }
// }
//
// This will append the error into the err variable, and return true if that
// individual error was non-nil.
//
// See AppendInto for more information.
//
// Deferred Functions
//
// Go makes it possible to modify the return value of a function in a defer
// block if the function was using named returns. This makes it possible to
// record resource cleanup failures from deferred blocks.
//
// func sendRequest(req Request) (err error) {
// conn, err := openConnection()
Expand All @@ -49,14 +94,21 @@
// // ...
// }
//
// The underlying list of errors for a returned error object may be retrieved
// with the Errors function.
// multierr provides the Invoker type and AppendInvoke function to make cases
// like the above simpler and obviate the need for a closure. The following is
// roughly equivalent to the example above.
//
// errors := multierr.Errors(err)
// if len(errors) > 0 {
// fmt.Println("The following errors occurred:", errors)
// func sendRequest(req Request) (err error) {
// conn, err := openConnection()
// if err != nil {
// return err
// }
// defer multierr.AppendInvoke(err, multierr.Close(conn))
// // ...
// }
//
// See AppendInvoke and Invoker for more information.
//
// Advanced Usage
//
// Errors returned by Combine and Append MAY implement the following
Expand Down Expand Up @@ -475,3 +527,113 @@ func AppendInto(into *error, err error) (errored bool) {
*into = Append(*into, err)
return true
}

// Invoker is an operation that may fail with an error. Use it with
// AppendInvoke to append the result of calling the function into an error.
// This allows you to conveniently defer capture of failing operations.
//
// See also, Close and Invoke.
type Invoker interface {
Invoke() error
}

// Invoke wraps a function which may fail with an error to match the Invoker
// interface. Use it to supply functions matching this signature to
// AppendInvoke.
//
// For example,
//
// func processReader(r io.Reader) (err error) {
// scanner := bufio.NewScanner(r)
// defer multierr.AppendInvoke(&err, multierr.Invoke(scanner.Err))
// for scanner.Scan() {
// // ...
// }
// // ...
// }
//
// In this example, the following line will construct the Invoker right away,
// but defer the invocation of scanner.Err() until the function returns.
//
// defer multierr.AppendInvoke(&err, multierr.Invoke(scanner.Err))
type Invoke func() error
prashantv marked this conversation as resolved.
Show resolved Hide resolved

// Invoke calls the supplied function and returns its result.
func (i Invoke) Invoke() error { return i() }

// Close builds an Invoker that closes the provided io.Closer. Use it with
// AppendInvoke to close io.Closers and append their results into an error.
//
// For example,
//
// func processFile(path string) (err error) {
// f, err := os.Open(path)
// if err != nil {
// return err
// }
// defer multierr.AppendInvoke(&err, multierr.Close(f))
// return processReader(f)
// }
//
// In this example, multierr.Close will construct the Invoker right away, but
// defer the invocation of f.Close until the function returns.
//
// defer multierr.AppendInvoke(&err, multierr.Close(f))
func Close(closer io.Closer) Invoker {
return Invoke(closer.Close)
}

// AppendInvoke appends the result of calling the given Invoker into the
// provided error pointer. Use it with named returns to safely defer
// invocation of fallible operations until a function returns, and capture the
// resulting errors.
//
// func doSomething(...) (err error) {
// // ...
// f, err := openFile(..)
// if err != nil {
// return err
// }
//
// // multierr will call f.Close() when this function returns and
// // if the operation fails, its append its error into the
// // returned error.
// defer multierr.AppendInvoke(&err, multierr.Close(f))
//
// scanner := bufio.NewScanner(f)
// // Similarly, this scheduled scanner.Err to be called and
// // inspected when the function returns and append its error
// // into the returned error.
// defer multierr.AppendInvoke(&err, multierr.Invoke(scanner.Err))
//
// // ...
// }
//
// Without defer, AppendInvoke behaves exactly like AppendInto.
//
// err := // ...
// multierr.AppendInvoke(&err, mutltierr.Invoke(foo))
//
// // ...is roughly equivalent to...
//
// err := // ...
// multierr.AppendInto(&err, foo())
//
// The advantage of the indirection introduced by Invoker is to make it easy
// to defer the invocation of a function. Without this indirection, the
// invoked function will be evaluated at the time of the defer block rather
// than when the function returns.
//
// // BAD: This is likely not what the caller intended. This will evaluate
// // foo() right away and append its result into the error when the
// // function returns.
// defer multierr.AppendInto(&err, foo())
//
// // GOOD: This will defer invocation of foo unutil the function returns.
// defer multierr.AppendInvoke(&err, multierr.Invoke(foo))
//
// multierr provides a few Invoker implementations out of the box for
// convenience. See Invoker for more information.
func AppendInvoke(into *error, invoker Invoker) {
AppendInto(into, invoker.Invoke())
}
115 changes: 115 additions & 0 deletions error_test.go
Expand Up @@ -555,6 +555,101 @@ 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)
})
}
}

func TestAppendInvokeClose(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
give error // error returned by Close()
want error
}{
{
desc: "append close nil into empty",
into: new(error),
give: nil,
want: nil,
},
{
desc: "append close into non-empty, non-multierr",
into: errorPtr(errors.New("foo")),
give: errors.New("bar"),
want: Combine(
errors.New("foo"),
errors.New("bar"),
),
},
{
desc: "append close into non-empty multierr",
into: errorPtr(Combine(
errors.New("foo"),
errors.New("bar"),
)),
give: 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) {
closer := newCloserMock(t, tt.give)
AppendInvoke(tt.into, Close(closer))
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 All @@ -580,3 +675,23 @@ func TestAppendIntoNil(t *testing.T) {
func errorPtr(err error) *error {
return &err
}

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
})
}
62 changes: 62 additions & 0 deletions example_test.go
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
}