x/sync/errgroup: clarify that Go calls the function regardless of prior errors

[YairHalberstadt]

What version of Go are you using (go version)?

go version go1.19-pre4 cl/455575533 +12f49fe0ed linux/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

package test

import (
	"errors"
	"sync/atomic"
	"testing"
	"time"

	"golang.org/x/sync/errgroup"
)

func TestErrGroupDoesNotRunNewFunctionsAfterPreviousError(t *testing.T) {
	group := errgroup.Group{}
	group.SetLimit(1)
	group.Go(func() error {
		return errors.New("")
	})
	time.Sleep(time.Second)

	var ran atomic.Bool
	group.Go(func() error {
		ran.Store(true)
		return nil
	})
        _ = group.Wait()
	if ran.Load() {
		t.Errorf("Expected second func not to run, but it did")
	}
}

What did you expect to see?

The documentation for errgroup states:

Go calls the given function in a new goroutine. It blocks until the new goroutine can be added without the number of active goroutines in the group exceeding the configured limit.

The first call to return a non-nil error cancels the group; its error will be returned by Wait.

I would assume that if the first call to return a non-nil error cancels the group, future funcs passed to go won't run.

Furthermore calls to group.Wait() should return immediately rather than waiting for funcs created after the group was cancelled to complete.

What did you see instead?

All funcs are called, even those created after the group was cancelled. group.Wait() waits for all funcs to complete before returning.

[bcmills]

Since you are using a literal value for the type, check the documentation for the type itself.
https://pkg.go.dev/golang.org/x/sync/errgroup#Group:

A zero Group is valid, has no limit on the number of active goroutines, and does not cancel on error.

Moreover, the documentation for the Go method states:

Go calls the given function in a new goroutine.

It does not state any exception to that behavior if the group already has an error.

We could perhaps clarify the documentation for the Go method, but this is all working as designed and documented.

[YairHalberstadt]

I see that now. I think it would be helpful to clarify that it's specifically the context that's cancelled, and you have to call ctx.Err manually.

Thanks for explaining!

[AndersonQ]

@bcmills I'm happy to submit a PR fixing the docs if this issue is still valid to be fixed

[gopherbot]

Change https://go.dev/cl/424634 mentions this issue: x/sync/errgroup: clarify docs for Go