Skip to content

Commit f5409ce

Browse files
committed
Return error from Group.Wait
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 #57.
1 parent 9974e9e commit f5409ce

File tree

2 files changed

+37
-2
lines changed

2 files changed

+37
-2
lines changed

group.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import "sync"
66
// coalesced.
77
type Group struct {
88
mutex sync.Mutex
9-
err *Error
9+
err error
1010
wg sync.WaitGroup
1111
}
1212

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

3131
// Wait blocks until all function calls from the Go method have returned, then
3232
// returns the multierror.
33-
func (g *Group) Wait() *Error {
33+
func (g *Group) Wait() error {
3434
g.wg.Wait()
3535
g.mutex.Lock()
3636
defer g.mutex.Unlock()

group_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,38 @@ func TestGroup(t *testing.T) {
4242
}
4343
}
4444
}
45+
46+
func TestGroupWait_ErrorNil(t *testing.T) {
47+
g := new(Group)
48+
g.Go(func() error { return nil })
49+
err := g.Wait()
50+
if err != nil {
51+
t.Fatalf("expected error to be nil, was %v", err)
52+
}
53+
}
54+
55+
func TestGroupWait_ErrorNotNil(t *testing.T) {
56+
g := new(Group)
57+
msg := "test error"
58+
g.Go(func() error { return errors.New(msg) })
59+
err := g.Wait()
60+
if err == nil {
61+
t.Fatalf("expected error to be nil, was %v", err)
62+
}
63+
64+
// err is a *Error, and e is set to the error's value
65+
var e *Error
66+
if !errors.As(err, &e) {
67+
t.Fatalf("expected err to be type *Error, was type %T, value %v", err, err)
68+
}
69+
70+
errs := e.WrappedErrors()
71+
if len(errs) != 1 {
72+
t.Fatalf("expected one wrapped error, found %d", len(errs))
73+
}
74+
75+
wrapped := errs[0]
76+
if wrapped.Error() != "test error" {
77+
t.Fatalf("expected wrap error message to be '%s', was '%s'", msg, wrapped.Error())
78+
}
79+
}

0 commit comments

Comments
 (0)