-
Notifications
You must be signed in to change notification settings - Fork 18k
proposal: context: context.Expired #73533
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
Comments
I think that if we go with this this should be a: var Expired error = expiredError{} And then we can define an So the use would look like this: if errors.Is(err, context.Expired) {
// ....
} |
Is this sufficient though? Now that there's WithCancelCause / WithDeadlineCause / WithTimeoutCause, a context may be canceled without matching either error. |
Cause errors are only returned through |
Concerning WithCancelCause / WithDeadlineCause / WithTimeoutCause, like opinion @mateusz834 , I expect to use through ctx.Error or the error wrapped ctx.Error. |
Proposal Details
Proposal Details
Proposed addition:
Motivation
Currently, many codebases should check whether the error is context.Canceled or context.DeadlineExceeded like this.
As the above case, it's right if developer want to detect any context cancellation, but this handling is redundant.
This syntax search shows:
Of course it's fine that's only redundant, unfortunately, some bugs can be happened since the returning error is different by Cancel and Timeout.
These are example that happens bugs.
trinodb/trino-go-client#140
argoproj/argo-cd#22824
These cases tried to consider cancellation correctly, but the difference between the explicit cancellation and timeouted cancellation couldn't be considered. To reduce these subtle incorrect handling, I propose above new API.
Of course, we can use Canceled and DeadlineExceeded separately to distinguish these error.
The text was updated successfully, but these errors were encountered: