proposal: context: context.Expired

[sivchari]

Proposal Details

Proposal Details

Proposed addition:

func Expired(err error) bool

Motivation

Currently, many codebases should check whether the error is context.Canceled or context.DeadlineExceeded like this.

x, err := doFunc(ctx) // call 
if err != nil {
    if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded)
        // handle
    }
    return err
}

As the above case, it's right if developer want to detect any context cancellation, but this handling is redundant. This syntax search shows:

• 6.5k errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded): https://github.com/search?q=errors.Is%28err%2C+context.Canceled%29+%7C%7C+errors.Is%28err%2C+context.DeadlineExceeded%29&type=code&p=1
• 508 !errors.Is(err, context.Canceled) || !errors.Is(err, context.DeadlineExceeded): https://github.com/search?q=%21errors.Is%28err%2C+context.Canceled%29+%7C%7C+%21errors.Is%28err%2C+context.DeadlineExceeded%29&type=code&p=1

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.

https://github.com/trinodb/trino-go-client/pull/140 https://github.com/argoproj/argo-cd/pull/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.

[gabyhelp]

Related Issues

• context: add APIs for setting a cancelation cause when deadline or timer expires #56661 (closed)
• context: Cannot tell which deadline or timeout cause expiration #35791 (closed)
• proposal: context: add WithCancelReason #46273 (closed)
• context: add APIs for writing and reading cancelation cause #51365 (closed)
• context: the docs should better clarify the context timeout #70945 (closed)
• proposal: context: add method like WithoutCancel but preserve context deadline #67135
• proposal: context: context.Cause should return a wrapped error #63759

Related Code Changes

• context: allow cancelling a context with custom error
• context: don't return unnecessary CancelFunc if ctx is expired
• net/http: use context for Client timeout

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[mateusz834]

I think that if we go with this this should be a:

var Expired error = expiredError{}

And then we can define an Is method.

So the use would look like this:

if errors.Is(err, context.Expired) {
    // ....
}

[seankhliao]

Is this sufficient though? Now that there's WithCancelCause / WithDeadlineCause / WithTimeoutCause, a context may be canceled without matching either error.

[mateusz834]

Now that there's WithCancelCause / WithDeadlineCause / WithTimeoutCause, a context may be canceled without matching either error.

Cause errors are only returned through context.Cause(err), and i don't think that we can match the error in any way, as such errors are not wrapped. So that would only work for code using ctx.Err().

[sivchari]

Concerning WithCancelCause / WithDeadlineCause / WithTimeoutCause, like opinion @mateusz834 , I expect to use through ctx.Error or the error wrapped ctx.Error. Cause is for knowing detailed reason why the context is canceled, it's not scope about usage of this proposal.

[peczenyj]

For deadline errors, the error deadlineExceededError implements this interface: Timeout() bool , perhaps this kind of error interface must be named in the stdlib errors package?

var terr interface{ Timeout() bool }
if errors.As(err, &terr){
  ...
}

could be

var terr errors.ErrTimeout
if errors.As(err, &terr) && terr.Timeout(){
  ...
}

or perhaps

if errors.IsTimeout(err) { // to simplify
  ...
}

[neild]

If we were to add anything, I think it would be:

package context

// Error is an error value which matches Canceled and DeadlineExceeded.
//
// To test to see if an error results from a canceled or expired context:
//    if errors.Is(err, context.Error) { ... }
var Error error

I'll note, however, that you can write the motivating example as:

x, err := doFunc(ctx) // call 
if err != nil && ctx.Err() != nil {
  // Possibly err doesn't result from ctx being canceled,
  // but ctx is definitely canceled.
}

[sivchari]

I liked @neild idea.

I'll note, however, that you can write the motivating example as:

As you mentioned, if context and error are handled within the same function, this approach should work. However, as shown in the code search results, there are many cases where context is not held within the same scope (e.g., extracted as isContextError), and since comparison using errors.Is is standard as an API for error checking in Go, I believe this proposal is valuable.

[sivchari]

Are there any progress ?