Skip to content
This repository has been archived by the owner on Jul 29, 2023. It is now read-only.

Discussion: why not a custom handler signature? #13

Open
moraes opened this issue Dec 1, 2015 · 4 comments
Open

Discussion: why not a custom handler signature? #13

moraes opened this issue Dec 1, 2015 · 4 comments
Labels

Comments

@moraes
Copy link
Contributor

moraes commented Dec 1, 2015

The title says it all: I'd like to hear your opinions about the pros and cons to stay with the func(http.ResponseWriter, *http.Request) signature for handlers, and why not set one that accepts an extra variables parameter.

@elithrar
Copy link

elithrar commented Dec 4, 2015

If you did this I'd recommend the following:

  1. Continue to accept func(http.ResponseWriter, *http.Request) for backwards compatibility with the large, existing ecosystem
  2. Continue to accept http.Handler
  3. Add support for func(context.Context, http.ResponseWriter, *http.Request) - and pass in routing params/metadata via a context.Background.

Saying that, Go 1.7 is likely to stuff a context.Context into *http.Request as per this golang-dev thread, which would give item no. 3 a lifespan of about ~18 months (1.7 in August + long tail of existing users). Another mux that I use (goji) went down this path and has embraced context.Context with the knowledge of the upcoming changes in 1.7.

Otherwise, a custom function type - i.e. func(mux.Vars, http.ResponseWriter, *http.Request) is awkward to deal with. Ties your handlers to this library more tightly.

@moraes
Copy link
Contributor Author

moraes commented Dec 4, 2015

Good points. I'll add:

  • I have a strong conviction that a router package should not require a custom handler signature. This would be the easy path; tempting and apparently convenient but not The Way To Go™;
  • We don't know how Context will be implemented in the standard library. It was said so far that it will be a Request field (which sounds wrong given its mutability), but we can't exclude the possibility that it will be a new HandlerC interface (which sounds the correct way but fragments the community a little bit). In any case, it's better to wait until this is well defined;
  • Less types and less API are better;

Given all of the above, I'm stepping back from using Context at all, and will use the common signatures from net/http, even if this will cost a little bit in performance (matching twice or using a map/lock setup).

@elithrar
Copy link

elithrar commented Dec 9, 2015

@moraes Can I assume your latest commits and comments in #4 supersede the comment above?

Postscript: totally get what you mean here regarding the ambiguity about where context.Context support will land in net/http. I was doing some thinking on it today, and I'm becoming more convinced that a http.ContextHandler interface (or whatever the name shakes out as) is probably the way to go.

Adding it to *http.Request effectively makes Go 1.7 a strict requirement for any package wanting to use net/context, since new struct fields are a backwards-incompatible change. Perhaps our use of a ServeHTTPC method, along with Goji's (and others, potentially) may even drive the discussion there. 1.7 is still at least 6-7 months away.

@moraes
Copy link
Contributor Author

moraes commented Dec 9, 2015

Yes, you can assume that. I'll fix my statement: "I have a strong conviction that a router package should not require a custom handler signature for the sole purpose of passing route variables". This would be like a database package define their own handler signature to pass a connection link, or an auth package define their own handler signature to pass credentials etc. net/context exists to solve all of these, uh, contextual problems.

In short, using a context parameter is unavoidable at this point, given the range of problems it solves with a unified signature. So we will follow Goji and I hope others will too. We revisit this when net/context is part of the standard library.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Status: No status
Development

No branches or pull requests

3 participants