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.

Sign up for GitHub

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

Jump to bottom

x/sync: Use WithCancelCause for errgroup.WithContext #59355

Closed
jonjohnsonjr opened this issue Mar 31, 2023 · 2 comments
Closed

x/sync: Use WithCancelCause for errgroup.WithContext #59355

jonjohnsonjr opened this issue Mar 31, 2023 · 2 comments
Labels
FrozenDueToAge Proposal Proposal-Accepted
Milestone
Backlog

Comments

@jonjohnsonjr
Copy link
Contributor

jonjohnsonjr commented Mar 31, 2023 •
edited
Loading

When using errgroup.WithContext, the first non-nil error encountered will cancel the context and then be returned by the next call to Wait. This works well when you actually call Wait, but if that canceled context prevents you from calling Wait, you just get an unhelpful context canceled error.

I believe the new context.WithCancelCause added in 1.20 would fix this, so I propose we use that instead of this context.WithCancel so we can propagate the error here, here, and here.
@gopherbot gopherbot added this to the Proposal milestone Mar 31, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/481255 mentions this issue: errgroup: use WithCancelCause to cancel context

peczenyj added a commit to peczenyj/cache that referenced this issue Aug 4, 2023
@peczenyj
update golang.org/x/sync that includes one fix in errgroup golang/go#…
2f71cf4 
…59355 - use WithCancelCause to cancel context
peczenyj added a commit to peczenyj/cache that referenced this issue Aug 4, 2023
@peczenyj
update golang.org/x/sync that includes one fix in errgroup golang/go#…
7d16f0b 
…59355 - use WithCancelCause to cancel context
@rsc
Copy link
Contributor

rsc commented Aug 9, 2023

Seems to have landed without a proposal acceptance but too late to do anything about that now.

@rsc rsc changed the title proposal: x/sync: Use WithCancelCause for errgroup.WithContext x/sync: Use WithCancelCause for errgroup.WithContext Aug 9, 2023
@rsc rsc modified the milestones: Proposal, Backlog Aug 9, 2023
@knz knz mentioned this issue Aug 17, 2023
Merged
@golang golang locked and limited conversation to collaborators Aug 8, 2024
anatoliinzrnk added a commit to anatoliinzrnk/go-sync that referenced this issue Sep 25, 2024
@anatoliinzrnk
errgroup: use WithCancelCause to cancel context
313f66e 
Fixes golang/go#59355

Change-Id: Ib6a88e7e5fefe7b0d5672035af16d109aabcbf1e
Reviewed-on: https://go-review.googlesource.com/c/sync/+/481255
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal Proposal-Accepted
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@rsc @gopherbot @jonjohnsonjr