proposal: context: add WithCancelReason

[OneOfOne]

• Would you consider yourself a novice, intermediate, or experienced Go programmer?
  I'd say experienced

• What other languages do you have experience with?
  C++, Javascript, Python, PHP

• Would this change make Go easier or harder to learn, and why?
  I don't think it'd change how easy or hard it is.

• Has this idea, or one like it, been proposed before?
  I think so? but for the life of me I couldn't find it.

• Who does this proposal help, and why?
  People who use context.WithCancel a lot and need to pass more error-context than just ErrCanceled.

• What is the proposed change?
  It adds a new type and a function to the context package.

// A CancelReasonFunc tells an operation to abandon its work with the specified error
// and it cannot be nil.
// A CancelReasonFunc does not wait for the work to stop.
// A CancelReasonFunc may be called by multiple goroutines simultaneously.
// After the first call, subsequent calls to a CancelFunc do nothing.
// The first error set is the one returned from Err().
type CancelReasonFunc func(reason error)

// WithCancelReason returns a copy of parent with a new Done channel. The returned
// context's Done channel is closed when the returned cancel function is called
// or when the parent context's Done channel is closed, whichever happens first.
//
// Canceling this context releases resources associated with it, so code should
// call cancel as soon as the operations running in this Context complete.
func WithCancelReason(parent Context) (ctx Context, cancel CancelReasonFunc) {}

• Is this change backward compatible?
  Yes.

• Show example code before and after the change.

Before:

func doStuff(ctx context.Context, errCh chan error) {
	select {
	case <-ctx.Done():
	case err := <-err:
		switch err {
		case ErrServerExploded:
			// find another server and retry again.
		}
	}
}

After:

func doStuff(ctx context.Context) {
	<-ctx.Done()
	switch err := ctx.Err() {
		case ErrCanceled:
		// nothing todo
		case ErrServerExploded:
		// find another server and retry again.
	}
}

• What is the cost of this proposal? (Every language change has a cost).
  It doesn't add any overhead, the only cost is one extra function (+ type) in the context package.

• Can you describe a possible implementation?
  https://go-review.googlesource.com/c/go/+/319892

• How would the language spec change? No change

• Orthogonality: how does this change interact or overlap with existing features?
  It would lower the mental overhead of passing errors down to children

• Is the goal of this change a performance improvement?
  Possible slightly less memory allocation for that one extra channel we'd use to pass error details? not really much.

• Does this affect error handling? In a way.

• Is this about generics? No.

I'm not stuck on WithCancelReason, maybe WithError would be better?

[sfllaw]

What would be the canonical way of using this? I’m trying to understand the usage:

func foo(ctx context.Context) error {
	ctx, cancel := context.WithCancelReason(ctx)
	defer cancel(nil)

	// doStuff waits for dial:
	go doStuff(ctx)

	if err := dial(ctx); err != nil {
		cancel(fmt.Errorf("foo: %w", err))
		return err
	}

	return nil
}

The above doc-comment suggests that cancel(nil) is not valid, but almost every context.WithCancel I’ve seen runs defer cancel() to clean up. Shouldn’t context.WithCancelReason use the same mechanism so that the goroutine doesn’t leak?

I’m still struggling to understand why we’d implement doStuff to wait unconditionally for the context to finish. Perhaps you could provide a richer example?

[rsc]

It seems like this can be done as a wrapper around WithCancel, returning a Context wrapper with a custom Err function.

How often does this come up? Why is it important enough to add to the standard package?

[rsc]

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

[OneOfOne]

It's important, at least in our internal code bases.

…

On Wed, Nov 10, 2021 at 5:00 PM Russ Cox ***@***.***> wrote: This proposal has been added to the active column <https://golang.org/s/proposal-status#active> of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#46273 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIHY66TKEADNYWWPHD3Y4DULL2RJANCNFSM45FJAMPQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[ianlancetaylor]

@OneOfOne What @rsc is saying is that you can do this yourself. You don't need a change in the standard library.

The question is: does this come up often enough that it should go into the standard library? This question is mainly for other people.

[ChrisHines]

The documentation for Context.Err() currently says:

// If Done is not yet closed, Err returns nil.
// If Done is closed, Err returns a non-nil error explaining why:
// Canceled if the context was canceled
// or DeadlineExceeded if the context's deadline passed.
// After Err returns a non-nil error, successive calls to Err return the same error.
Err() error

If we add WithCancelReason how do you propose to update the docs for the Err method to reflect the new behavior?

[sfllaw]

The question is: does this come up often enough that it should go into the standard library? This question is mainly for other people.

I can hazard a guess, which is related to #26356. Some of my coworkers have complained that they can’t figure out why something got cancelled, so this is really a request that cancellations explain themselves, in the form of a replacement to WithCancel.

The reason people believe it should belong in the standard library is that they want the standard library to force people to write these explanations. That’s also why #26356 (comment) and others mention stack traces, which is another facet to the same problem.

[rsc]

It still sounds like doing this in an external implementation of Context would still be the place to experiment with a change like this.

[rsc]

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

[egorbunov]

It seems like this can be done as a wrapper around WithCancel, returning a Context wrapper with a custom Err function.

How often does this come up? Why is it important enough to add to the standard package?

Let me show you why this is not true and support from stdlib may help.

Imagine we have WithCancelReason function somehow implemented. My particular case can be described like this:

func doOperation(ctx context.Context) error {
    ctx, cancelReason := WithCancelReason(ctx)

    // there is a background process which periodically checks
    // that we still are allowed to perform an operation
    go func() {
        for {
            if checkAccessLost() {
                cancelReason(ErrAccessLost)
                return
            }
        }
    }()

    return thirdPartyCall(ctx)
}

func thirdPartyCall(ctx context.Context) error {
    ctx, close := context.WithTimeout(ctx, time.Minute)
    defer close()
    
    for {
        if ctx.Err() != nil {
            return ctx.Err()
        }
     }
}

Imagine now that cancelReason(ErrAccessLost) is executed before timeout inside thirdPartyCall occurs.
I would expect in such case that inside thirdPartyCall, ctx.Err() returns ErrAccessLost.

But standard context.WithTimeout returns context.Context which can only return Canceled or DeadlineExceeded from Err() method!

My conclusion here: it is impossible to implement WithCancelReason as an external addition to standard context package in a satisfactory way.