Does WithContext method need to panic if context is nil?

Question

I want to write a WithContext method for a struct and am taking inspiration from net/http's Request.WithContext.

My question is: why does Request.WithContext panic if the context is nil:

func (r *Request) WithContext(ctx context.Context) *Request {
    if ctx == nil {
        panic("nil context")
    }
    ...
}

And should mine as well?

For more context on why I want to create a WithContext method: I am implementing an interface that does not provide a context parameter in its signature but believe the implementation requires it.

More specifically, I am writing a Redis backend for gorilla/session using the official Redis client for Go, where the Get and Set methods take context.Context.

The idea is that my redis store will be shallow copied with the new context object, when needed, and then used:

type redisStore struct {
    codecs  []securecookie.Codec
    backend Backend // custom interface for Redis client
    options *sessions.Options
    ctx     context.Context
}

func (s *redisStore) WithContext(ctx context.Context) *redisStore {
    if ctx == nil {
        panic("nil context")
    }
    s2 := new(redisStore)
    *s2 = *s
    s2.ctx = ctx
    return s2
}

// Backend

type Backend interface {
    Set(context.Context, string, interface{}) error
    Get(context.Context, string) (string, error)
    Del(context.Context, string) error
}

blackgreen · Answer 1 · 2021-11-14T07:57:41.600

The purpose of panicking is to "fail fast" and reject a nil context without changing the function signature.

If the function does not panic then it must return error in order to reject a bad input:

func (r *Request) WithContext(ctx context.Context) (*Request, error) {
    if ctx == nil {
        return nil, errors.New("nil ctx")
    }
    ...
}

And then who calls this function must handle the error to avoid using an invalid request:

request, err = request.WithContext(nil)
if err != nil {
   
}

By handling the error you are introducing a control flow branch, and you lose method chaining. You also cannot immediately use WithContext return value into a function parameter:

// cannot do, because WithContext returns an error too
data, err := fetchDataWithContext(request.WithContext(ctx), otherParam)

Also it would create an error instance that will be eventually garbage collected. This all is cumbersome, poor usability and unnecessary alloc simply for saying "don't give me a nil context".

About creating a redis store with a context, the context documentation is clear:

Package context defines the Context type, which carries deadlines, cancellation signals, and other request-scoped values across API boundaries and between processes.

The important detail is request-scoped. So setting a context in the redis client itself is contrary to this recommendation. You should pass context values at each get/set call.

Ok, so it seems like the decision behind panicking is to avoid the control flow branch, which seems like a subjective design decision. But I appreciate the answer. RE the important detail being **request-scoped**: I agree and that is why the `WithContext` would be utilized on a per request basis that is then passed in at each get/set call. I'll update the example to show this. — Algebra8, Nov 14 '21 at 21:59

score 0 · Answer 2 · answered Nov 14 '21 at 05:25

0

The context of an HTTP request is canceled if the client closes the connection. When the context is canceled, all its child contexts are also canceled, so a nil context would panic then. Because of this, you cannot pass a nil context to WithContext.

Whether or not your redis store should panic depends on how you are going to use that context. It is usually not a good idea to include a context in a struct. One acceptable way of doing that is if the struct itself is a context. Contexts should be created for each call, should live for the duration of that call, and then thrown away.

answered Nov 14 '21 at 05:25

Burak Serdar

46,455
3
40
59

Regarding your first point, that makes sense and thank you for pointing that out. But the part that is not clear to me is, why panic? Couldn't they return `(*Request, error)` and what implications does that have for what I'm trying to do? – Algebra8 Nov 14 '21 at 05:44
Regarding your second point, that also makes sense and I completely agree but am not too sure what else to do. The idea in this case would be that the store could be re-created with context on a per-request basis in some middleware and thrown away at the end of the middleware chain. – Algebra8 Nov 14 '21 at 05:46
Does the store need the context? You can create a store and put that into a context. – Burak Serdar Nov 14 '21 at 05:51

Does WithContext method need to panic if context is nil?

2 Answers2