Since API changes are something that is now possible to do with module versions, I thought it would be worth mentioning one that gnaws at me pretty frequently.
We use contexts heavily in my code-base, and it comes up moderately often that we want a cancellable or deadline-respecting sync.WaitGroup, and for this purpose authors tend to turn to x/sync/errgroup. This often works out fine as long as the scope of the errgroup.Group is confined to a single function, but it also regularly will be used with something somewhat stateful. These frequently follow a certain pattern, the core elements of which are exemplified by this contrived example:
type ServerRunner struct {
group *errgroup.Group // *Group as a field
groupCtx context.Context // context as a field (heavily discouraged)
}
func (sr *ServiceRunner) Run(server *servers.Server) {
sr.Group.Go(func() error { return server.Run(sr.groupCtx) }) // wrapper that calls Go
}
func (sr *ServiceRunner) Wait(ctx context.Context) {
select {
case <-ctx.Done():
return ctx.Err()
case <-sr.groupCtx.Done():
sr.groupCtx.Wait() // wrapper that calls wait and/or ctx.Done
return sr.groupCtx.Err()
}
}I assert (without evidence) that this boilerplate is pretty common among context-respecting code that interacts with errgroup.Group, and in fact the above could be almost directly converted into a utility package, but then we would have ContextGroup wrapping errgroup wrapping WaitGroup... which feels excessive.
I have also observed that the context returned by WithContext is occasionally misused for code other than the goroutines spawned by Go, in some cases by directly shadowing the ctx variable, which often results in spooky action at a distance where one failure causes code in another part of the application to have its context cancelled.
So, I propose that the API for errgroup split out the context- and non-context APIs:
type Group struct { /* ... */ }
func (Group) Go(func() error) { /* ... */ }
func (Group) Wait() error { /* ... */ }
type ContextGroup struct { /* ... */ }
func WithContext(ctx context.Context) *ContextGroup { /* ... */ }
func (ContextGroup) Go(func(context.Context) error) { /* ... */ }
func (ContextGroup) Wait(context.Context) error { /* ... */ }Unfortunately, this is definitely a backward-incompatible change, and one for which there is probably little chance for a mechanical rewrite unless ContextGroup had a mechanism for retrieving its context.
Since API changes are something that is now possible to do with module versions, I thought it would be worth mentioning one that gnaws at me pretty frequently.
We use contexts heavily in my code-base, and it comes up moderately often that we want a cancellable or deadline-respecting
sync.WaitGroup, and for this purpose authors tend to turn tox/sync/errgroup. This often works out fine as long as the scope of theerrgroup.Groupis confined to a single function, but it also regularly will be used with something somewhat stateful. These frequently follow a certain pattern, the core elements of which are exemplified by this contrived example:I assert (without evidence) that this boilerplate is pretty common among context-respecting code that interacts with
errgroup.Group, and in fact the above could be almost directly converted into a utility package, but then we would have ContextGroup wrapping errgroup wrapping WaitGroup... which feels excessive.I have also observed that the context returned by
WithContextis occasionally misused for code other than the goroutines spawned byGo, in some cases by directly shadowing thectxvariable, which often results in spooky action at a distance where one failure causes code in another part of the application to have its context cancelled.So, I propose that the API for errgroup split out the context- and non-context APIs:
Unfortunately, this is definitely a backward-incompatible change, and one for which there is probably little chance for a mechanical rewrite unless ContextGroup had a mechanism for retrieving its context.