Does WithContext method need to panic if context is nil?-CodePudding

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
}

CodePudding user response：

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.

CodePudding user response：

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.