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.

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

context: add APIs for setting a cancelation cause when deadline or timer expires #56661

Closed
Sajmani opened this issue Nov 8, 2022 · 11 comments
Closed

Comments

@Sajmani
Copy link
Contributor

Sajmani commented Nov 8, 2022

This is a follow on to proposal #51365 to add WithDeadlineCause and WithTimeoutCause to the context package. These functions enable the user to arrange a cancelation cause to be set when a deadline or timer expires. This is not possible to do efficiently with the APIs introduced in https://go-review.git.corp.google.com/c/go/+/375977.

The proposed API additions are:

// WithDeadlineCause behaves like WithDeadline but also sets the cause of the
// returned Context when the deadline is exceeded. The returned CancelFunc does
// not set the cause.
func WithDeadlineCause(parent Context, d time.Time, cause error) (Context, CancelFunc)

// WithTimeoutCause behaves like WithTimeout but also sets the cause of the
// returned Context when the timout expires. The returned CancelFunc does
// not set the cause.
func WithTimeoutCause(parent Context, timeout time.Duration, cause error) (Context, CancelFunc)

Example:

tooSlow := fmt.Errorf("too slow!")
ctx, cancel := context.WithTimeoutCause(context.Background(), 1*time.Second, tooSlow)
time.Sleep(2*time.Second) // timer fires, setting the cause
cancel() // no effect as ctx has already been canceled
// ctx.Err() == context.DeadlineExceeded && Cause(ctx) == tooSlow

The cause passed to these functions is set only when the deadline or timer expires. The common case is that the context is canceled before that happens via the returned CancelFunc. In this case, the cause will be context.Canceled:

tooSlow := fmt.Errorf("too slow!")
ctx, cancel := context.WithTimeoutCause(context.Background(), 1*time.Second, tooSlow)
time.Sleep(500*time.Millisecond) // timer hasn't expired yet
cancel() // cancels the timer and sets ctx.Err()
// ctx.Err() == Cause(ctx) == context.Canceled

If users need to set the cause on both the timer and cancel() paths, they can stack contexts:

Example where the timer fires first:

finishedEarly := fmt.Errorf("finished early")
tooSlow := fmt.Errorf("too slow!")
ctx, cancel := context.WithCancelCause(context.Background())
ctx, _ = context.WithTimeoutCause(ctx, 1*time.Second, tooSlow)
time.Sleep(2*time.Second) // timer fires, setting the cause
cancel(finishedEarly) // no effect as ctx has already been canceled
// ctx.Err() == context.DeadlineExceeded && Cause(ctx) == tooSlow

Example where cancel happens first:

finishedEarly := fmt.Errorf("finished early")
tooSlow := fmt.Errorf("too slow!")
ctx, cancel := context.WithCancelCause(context.Background())
ctx, _ = context.WithTimeoutCause(ctx, 1*time.Second, tooSlow)
time.Sleep(500*time.Millisecond) // timer hasn't expired yet
cancel(finishedEarly) // cancels the timer and sets ctx.Err()
// ctx.Err() == context.Canceled && Cause(ctx) == finishedEarly
@gopherbot gopherbot added this to the Proposal milestone Nov 8, 2022
@Sajmani
Copy link
Contributor Author

Sajmani commented Nov 8, 2022

@ChrisHines

@rsc
Copy link
Contributor

rsc commented Nov 9, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@gopherbot
Copy link

Change https://go.dev/cl/449318 mentions this issue: context: add APIs for setting a cancelation cause when deadline or timer expires

@Sajmani
Copy link
Contributor Author

Sajmani commented Nov 10, 2022

https://go.dev/cl/449318 implements this proposal. The test exercises all the behaviors documented above.

@rsc
Copy link
Contributor

rsc commented Nov 16, 2022

@ChrisHines, you had been vocal about these on #51365 (comment). Do you still think we should do them?

I think these are probably right but I'm less sure than about #51365, so I think it would make sense to wait for Go 1.21 rather than push them through in the last days of this release. We'll have more time to fine-tune that way.

@ChrisHines
Copy link
Contributor

To clarify, I can see the use of these, but personally I think there is a lot of clumsiness to both these and #51365. Much of that clumsiness is due to the need for backwards compatibility, so we are in a tight spot here.

I was vocal about these in #51365 mostly because I wanted to understand how to use them properly and because it seemed we didn't properly follow through with the discussion of their ergonomics and I didn't want to see that slip through the cracks. It was said that we could omit them from the stdlib because it would be easy to implement them on top of the base WithCancelCause feature. That seemed wrong to me and it seems @Sajmani agrees with based on implementation experience.

I have encountered the problem of seeing a deadline exceeded message in logs and wished there was a way to back track to the specific deadline in real code, though, so from that point of view some way to solve that problem would be nice. I'm not 100% sure this is the best way to do it, but I haven't invested time to explore alternatives either.

@rsc
Copy link
Contributor

rsc commented Nov 30, 2022

Does anyone object to this proposal?

@rsc
Copy link
Contributor

rsc commented Dec 7, 2022

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Dec 14, 2022

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: context: add APIs for setting a cancelation cause when deadline or timer expires context: add APIs for setting a cancelation cause when deadline or timer expires Dec 14, 2022
@rsc rsc modified the milestones: Proposal, Backlog Dec 14, 2022
@kortschak

This comment was marked as off-topic.

@dmitshur dmitshur modified the milestones: Backlog, Go1.21 Jan 19, 2023
@gopherbot
Copy link

Change https://go.dev/cl/486535 mentions this issue: doc: add release notes for new context functions

gopherbot pushed a commit that referenced this issue Apr 20, 2023
For #40221
For #56661
For #57928

Change-Id: Iaf7425bb26eeb9c23235d13c786d5bb572159481
Reviewed-on: https://go-review.googlesource.com/c/go/+/486535
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Sameer Ajmani <sameer@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
eric pushed a commit to fancybits/go that referenced this issue Aug 23, 2023
…mer expires

Fixes golang#56661

Change-Id: I1c23ebc52e6b7ae6ee956614e1a0a45d6ecbd5b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/449318
Run-TryBot: Sameer Ajmani <sameer@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Apr 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants