Skip to content

Commit

Permalink
Return error from Group.Wait
Browse files Browse the repository at this point in the history
Changes the return value of Group.Wait to error instead of *Error. The *Error concrete type can lead to unintuitive, subtle bugs around nil checks (see: https://golang.org/doc/faq#nil_error). Returning the error interface instead eliminates these. Addresses hashicorp#57.
  • Loading branch information
ccampo133 committed Nov 12, 2021
1 parent 9974e9e commit 7d7a61a
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 2 deletions.
4 changes: 2 additions & 2 deletions group.go
Expand Up @@ -6,7 +6,7 @@ import "sync"
// coalesced.
type Group struct {
mutex sync.Mutex
err *Error
err error
wg sync.WaitGroup
}

Expand All @@ -30,7 +30,7 @@ func (g *Group) Go(f func() error) {

// Wait blocks until all function calls from the Go method have returned, then
// returns the multierror.
func (g *Group) Wait() *Error {
func (g *Group) Wait() error {
g.wg.Wait()
g.mutex.Lock()
defer g.mutex.Unlock()
Expand Down
35 changes: 35 additions & 0 deletions group_test.go
Expand Up @@ -42,3 +42,38 @@ func TestGroup(t *testing.T) {
}
}
}

func TestGroupWait_ErrorNil(t *testing.T) {
g := new(Group)
g.Go(func() error { return nil })
err := g.Wait()
if err != nil {
t.Fatalf("expected error to be nil, but was %v", err)
}
}

func TestGroupWait_ErrorNotNil(t *testing.T) {
g := new(Group)
msg := "test error"
g.Go(func() error { return errors.New(msg) })
err := g.Wait()
if err == nil {
t.Fatalf("expected error to be nil, but was %v", err)
}

// err is a *Error, and e is set to the error's value
var e *Error
if !errors.As(err, &e) {
t.Fatalf("expected err to be type *Error, but was type %T, value %v", err, err)
}

errs := e.WrappedErrors()
if len(errs) != 1 {
t.Fatalf("expected one wrapped error, but found %d", len(errs))
}

wrapped := errs[0]
if wrapped.Error() != "test error" {
t.Fatalf("expected wrap error message to be '%s', but was '%s'", msg, wrapped.Error())
}
}

0 comments on commit 7d7a61a

Please sign in to comment.