The problem with Go’s default HTTP handlers

192 点作者 p5v将近 3 年前

39 条评论

Well yeah, the default stdlib HTTP handlers are very basic. The cool thing is that the standard library is replete enough to build tons on top of that, though. CORS headers using nothing but stdlib for example:<pre><code> origin := http.StripPrefix("/", http.FileServer(http.Dir("www"))) wrapped := http.HandlerFunc(func(writer http.ResponseWriter, req *http.Request) { writer.Header().Set("Access-Control-Allow-Origin", "whatever.com") writer.Header().Set("Access-Control-Allow-Methods", "POST, GET, OPTIONS, PUT, DELETE") writer.Header(). Set("Access-Control-Allow-Headers", "Accept, Content-Type, Content-Length, Accept-Encoding, X-CSRF-Token, Authorization") origin.ServeHTTP(writer, req) }) http.Handle("/", wrapped) </code></pre> You end up writing a ton of boilerplate like this if you're sticking with the standard library, but having a ton of awareness and control over every aspect of what you're doing is really just so easy in this language. If I don't understand anything in the standard library I just go to the definition of it in the golang source code and usually end up figuring out what I need. It's really impressive how much you can get done without using any packages if you're writing anything net-facing that just needs to do one thing and do it well.

评论 #32406535 未加载

dralley将近 3 年前

I like the way Axum for Rust handles this. All response handlers return something that implements IntoResponse. Lots of types implement IntoResponse, thus making the simple cases really streamlined.async fn create_user( Json(payload): Json<CreateUser>, ) -> impl IntoResponse {<pre><code> let user = User { id: 1337, username: payload.username, }; // this will be converted into a JSON response // with a status code of `201 Created` (StatusCode::CREATED, Json(user))</code></pre> }If the handler is complex enough that branches can't all return the same type, you can just add .into_response() to each return site, and now you're just returning a response, which also implements IntoResponse (as a no-op).This avoids the problem discussed in this article while also avoiding much of the boilerplate of manually building responses.

评论 #32399893 未加载

评论 #32403211 未加载

评论 #32407068 未加载

评论 #32400349 未加载

评论 #32402385 未加载

vrnvu将近 3 年前

I dislike the example.The author says that because we have a complex function, and the HTTP handler is designed the way it is, we cannot avoid running into problems. I disagree. This are two separate things.You could take a more functional approach and separate the main logic and the side effect. Which is writing the response.<pre><code> func aMoreAdvancedHandler(w http.ResponseWriter, r *http.Request) { res, err := helper() if err { http.Error(w, err.Error(), err.Status()) } w.write(res) } func helper() ([]byte, err error) { // todo } </code></pre> Where now the helper function has the problem of dealing with multiple operations that can potentially fail. Which is exactly the problem in the example. Not the HTTP handler itself...For example, we could use the defer function call with a named error to check if something fails and guarantee that we always return an appropriate error. Similar to the pattern used to free resources... I don't know how is this commonly called. But again, the problem is the helper function not the handler.I don't want to use the FP terminology, probably most people are familiar with this pseudo-code:<pre><code> func helper() (res, err) { return foo().bar().baz() } </code></pre> Where the chain foo, bar, baz will continue to be executed if all the calls are success, but early terminate if an error occurs.So, where is the problem?

评论 #32418046 未加载

评论 #32405464 未加载

topicseed将近 3 年前

I agree, I wouldn't have been against a required return, but the mitigation is relatively straightforward by using custom handlers like mentioned at the end, if needed.I really like Go's default HTTP handlers because it offers that straightforward flexibility. Nothing rocket-sciencey.

评论 #32401568 未加载

评论 #32401095 未加载

评论 #32400904 未加载

donatj将近 3 年前

So the problem with these "good" examples is that they expose the end user to Errors, messages likely not written by you or your team and moreso not intended for general consumption - you need to be really careful about doing so because an unexpected error could expose lots of troubling things like credentials and general information about your system you'd rather not expose.We've got a general rule of thumb never to expose the message of an Error to the end user.

评论 #32402924 未加载

mrweasel将近 3 年前

It's not something I even thought about, but I guess I see the point, I just don't agree. In the HTTP handlers it makes sense that you don't have return values, because: What would you do with that value exactly?The HTTP handler should always ensure that "something" is returned, at the very least a return code. Once something has been returned to the client, can you really argue that there's an error?There's a point to the risk of missing return value, you just run the risk of complicating the request flow by adding it. I'd argue that most people would view the code path as having ended, once the client receives a response. The request shouldn't really continue on the server end after that. There can be background processing, go routines or something like that, but that's a bit different, as it doesn't involve the original requests-response.

评论 #32399748 未加载

评论 #32401215 未加载

jayd16将近 3 年前

Maybe this is a subtlety of Go I'm not aware of, but at least in most languages, the issue with the explicit return value is that you must first return before you can send the value.By being able to call into the response object, the handler is able to send to the client immediately, and then the handler writer can do other bookkeeping before exiting the handler.With a return value, work would need to be fire off asynchronously before the return value is sent.

badhombres将近 3 年前

To me this reads as a positive to the default handlers, as it shows how easy it was to make it work into a format that works for the programmer's preference. I appreciated the solution to the complaint

silentprog将近 3 年前

I was in the same camp as you a few years ago and also wrote an utility to handle this. Might be of any interest to you.<a href="https://github.com/barthr/web-util" rel="nofollow">https://github.com/barthr/web-util</a>And the blog post explaining it: <a href="https://bartfokker.com/posts/clean-handlers" rel="nofollow">https://bartfokker.com/posts/clean-handlers</a>

alectic将近 3 年前

Streamed-response use cases get weird/complicated if you have to return the response.

评论 #32401051 未加载

评论 #32400298 未加载

binwiederhier将近 3 年前

I agree that they are awkward. I've solved it similarly by defining a custom handleFunc [1] and a custom errHTTP struct [2] which contains the HTTP response code and even more detailed error codes. It is very nice to work with.Like so:<pre><code> if err == util.ErrLimitReached { return errHTTPEntityTooLargeAttachmentTooLarge } </code></pre> Or so:<pre><code> if err != nil { return wrapErrHTTP(errHTTPBadRequestActionsInvalid, err.Error()) } </code></pre> [1] <a href="https://github.com/binwiederhier/ntfy/blob/main/server/server.go#L56" rel="nofollow">https://github.com/binwiederhier/ntfy/blob/main/server/serve...</a>[2] <a href="https://github.com/binwiederhier/ntfy/blob/main/server/errors.go" rel="nofollow">https://github.com/binwiederhier/ntfy/blob/main/server/error...</a>

lhorie将近 3 年前

IMHO this boils down to purity vs side effects. Given a black box function, it's a lot easier to reason about what it did if it behaves in a pure manner returning some data structure that represents some form of state, than passing some mutable complex real world thing into it and then trying to inspect what the thing did after the fact.But the matter of purity vs side effects is completely orthogonal to the default HTTP handler design. The handler provides the inputs and the output sinks that represent the "real world". But it's entirely up to you how you manage the data therein. You can continue to procedurally consume these input/output entities turtles all the way down, or you can work in a functional style until you are actually ready to flush the result of the computation down the writer.

oppositelock将近 3 年前

The low level HTTP library is phenomenal for building more friendly packages on top, think of it as assembly language for HTTP in go. What isn't obvious about that Go http design is that it's very parallel. Your callback gets invoked in a goroutine for each request, so you can write easy to understand code, linearly, without worrying about things like promises or callbacks or message queues.I'm opinionated here, because I also think all API's should be built starting from a spec, such as OpenAPI, and to that end, I've written a library to do just that, generate models and echo handlers from API specs, and other contributors have added gin, chi, etc [1]1: <a href="https://github.com/deepmap/oapi-codegen" rel="nofollow">https://github.com/deepmap/oapi-codegen</a>

halfmatthalfcat将近 3 年前

This isn't so much a critique of the HTTP handlers themselves but relying on (and stuffing a ton of shit into) context as a way of dealing with complex request/response patterns was a major headache.Trying to implement a circuit breaker against multiple upstreams for a single request in Go was a nightmare.

评论 #32402824 未加载

ainar-g将近 3 年前

Another possible extension of that design is using error types for HTTP codes. Something like:<pre><code> type HTTPError struct { Err error Code int } </code></pre> And then, with a couple of wrappers like this:<pre><code> func errorNotFound(err error) (wrapped error) { return &HTTPError{ Err: err, Code: http.StatusNotFound, } } </code></pre> you could do something like this:<pre><code> return errorNotFound(err) </code></pre> or this:<pre><code> return errorInternal(err) // 500 Internal Server Error</code></pre>

评论 #32400238 未加载

评论 #32400703 未加载

carom将近 3 年前

I am writing a large Go web application right now and I disagree with this. I do not want to return an error. I want to write an error page or an error JSON message depending on the handler. By the time you are deep in a web application you are copy and pasting all your error blocks anyway. This [1] is a sample of one of my functions. I honestly love Go for how consistent it is.I wrote a rudimentary static checker [2] to make sure that any function does not call both the jsonError() and errorPage() functions. This was quite a problem given how much I reuse code. I disagree with the author's take because it is incredibly obvious and quite easy to track down when you forget a return. You will have a big block of garbage in the middle of your page and JSON, and your unique error message should lead you directly to the line.My problem is actually in executing the templates at the end of the functions [1], when you finally write out the page. If that has an error, you can't really back track. I've been thinking about changing that to write to a temporary buffer which then writes to the http.ResponseWriter but haven't made the change yet.1. <a href="https://gist.github.com/TACIXAT/24621013248e2d91bebd16c5a02a99f9" rel="nofollow">https://gist.github.com/TACIXAT/24621013248e2d91bebd16c5a02a...</a>2. <a href="https://gist.github.com/TACIXAT/1000e7d873f0a8a738b3e720b4a7684c" rel="nofollow">https://gist.github.com/TACIXAT/1000e7d873f0a8a738b3e720b4a7...</a>

tastysandwich将近 3 年前

I can see the problem, but, I don't know, it's just not something that's ever bothered me. How big are his handlers anyway!? They should be pretty small IMO...I've managed to write some pretty complicated REST services with just the standard HTTP library and Mux for routing.In fact I've never used any of the bigger libraries like Gin or Echo.While all of my Node JS colleagues are drowning in dependabot PRs we're just cruising by in the Golang team with our small handful of dependencies. Heaven!

Matthias247将近 3 年前

The article points out a „pro“ of returning a response in handlers - but fails to miss any cons.However there are a few of them. One particularly important one is that the „return response“ variants lack extremely on the observability front - which then in the end leads to services which are hard to run in production.Eg let’s say I want to measure in my handler what „last byte latency“ is - or at least when the handler pushed the last byte into a tcp socket. With the Go approach it’s not a problem - we can wrap the handler in another handler which does emit metrics.With the „return response object“ it won’t work easily anymore. At the time the handler returns nothing actually has happened.The same thing applies for eg tracking the amount of concurrently executed requests to understand resource utilization (and starvation). No issue with the Go variant - hard to do with „function as a service“.It’s actually rather unfortunate that all Rust frameworks went down the „response object“ route and made it hard to add proper observability.

Someone将近 3 年前

So, the issue is that there’s an implicit contract that says something like “a handler must write to the ResponseWriter or call http.Error before returning”, but the compiler doesn’t enforce that contract.The proposed improvement makes some ways to accidentally not adhere to the contract more obvious to humans, but still doesn’t enforce the contract.I wonder whether there are languages that allow one to write a library that enforces such contracts at compile time.(One way to do that would be to pass the Request and the ResponseWriter together with a state machine that is hard-coded in the API as a single object to the handler, with the equivalents of w.Write and http.Error declaring their state transitions, and having a language functionality that requires such object to be in an end state of that state machine whenever the function returns)

评论 #32401451 未加载

boredumb将近 3 年前

<pre><code> func (api \*App) HandleOnError(w http.ResponseWriter, err error, status int, context string) bool { if err != nil { api.logging.Error(err, context) http.Error(w, err.Error(), http.StatusInternalServerError) return true } return false } </code></pre> and then it's used in the handler funcs<pre><code> if api.HandleOnError(w, err, http.StatusInternalServerError, "Getting stuff for things") { return } </code></pre> Makes the logging verbose and handling always properly taken care of, makes the error handling in handlers verbose enough from the caller but abstract enough to not be a majority of the handlers body.

chrsig将近 3 年前

I dont necessarily agree they should return a value, but I do have some issues with the api:- by default, you can't tell if a handler has already written to the body or emitted a status code.- by default, you can't read the body more than once- r.PostForm vs r.Form ...that they both exist, and that they're only available after a manual call to r.ParseForm- the http server accepts an unbounded number of connections. it'll just keep making goroutines as they come in. there's no way that I know of to apply back pressure.there's probably more that I grump about, but those are the ones at the top of my mind. for the most part it works well, and i can work around my gripes.

评论 #32403012 未加载

skybrian将近 3 年前

There's a subtle problem in the proposed solution: if an error happens after you already started writing the response, you can't change the http status code.So, what should you do with an error that happens while writing the HTTP response itself? Maybe keep statistics on how often it happens, in case happens often enough that it seems relevant for debugging. But there's no good way to report an error to the client because the network connection is likely broken.If you're not going to keep statistics, dropping errors from writing an http response is reasonable and arguably correct.

throwaddzuzxd将近 3 年前

In my team we used Gin and migrated to Echo (because Gin didn't support conflicting routes at the time, like /foo/{variable} and /foo/bar) and we got to the same conclusion. Forgetting to return with Gin (and with net/http) is an issue that actually occurs, and we've been bitten by it more times than we care to admit.Of course it's not worth migrating to Echo just for that but it's good to know that some routers implement it differently, if it's something that bothers you.

adontz将近 3 年前

To me it seems like a problem induced by the way Go handles errors. With exceptions it would be something like<pre><code> try { // Some dangerous code which may throw/raise w.Write([]byte("Hello, World!")) } catch( FileException ) { w.Write([]byte("Screw your file!")) } catch( ... ) { w.Write([]byte("Screw you in general!")) } </code></pre> and no such problem

评论 #32401933 未加载

评论 #32403330 未加载

crabmusket将近 3 年前

For fairness's sake, I'd like to point out that Nodejs has the same behaviour. Helping a new developer create a web server, we fell into the bugs the author is talking about a few times. I was always quick to spot the problem with the benefit of experience, but a newbie debugging a "response was already sent" error was not the simplest thing in the world.

timvisee将近 3 年前

With Rust you could write a function that consumes the HTTP client, not allowing you use it after, resulting in a compiler error for your example.In Go you could write a function taking the HTTP client reference and setting it to null. This would at least produce a runtime error if it ever comes across the path rather than doing something else that is unexpected.

AtNightWeCode将近 3 年前

Surely one can make the exact same mistake with Echo?if id == "" { fmt.Errorf("get album: missing id") }

weltensturm将近 3 年前

Why is it http.Error(w, ...) and not w.Write(http.Error(...))? The latter would be way more clear in showing that nothing magical happens and a return (if wanted) is still necessary (I don't write Go, so I don't know the conventions)

评论 #32402961 未加载

LordHeini将近 3 年前

Returning errors from a http handler is not a good idea imho.Because http errors/responses can be too complex for the limited Golang error objects.You usually want an appropriate error code, a body with a message or set some header.

Gnouc将近 3 年前

You may want to use <a href="https://github.com/orijtech/httperroryzer" rel="nofollow">https://github.com/orijtech/httperroryzer</a> for your CI.

n4jm4将近 3 年前

I've made that same mistake before. And not just in HTTP handlers.There is a case to be made for a linter rule that warns when a conditional evaluates an error value, without presenting a return path.

didip将近 3 年前

This is such a tiny problem to quibble about. Just create a helper function that indeed force you to return values.The default leaves you room to do harder things like custom chunk streaming.

评论 #32400723 未加载

tomohawk将近 3 年前

Errors important to the server should not be conflated with http responses to the client. There is sometimes a correlation between the two, but often not.

m00x将近 3 年前

Editorialized title. The real title is: "I Don’t Like Go’s Default HTTP Handlers".Please edit the HN title to reflect the blog post.

alingse将近 3 年前

we have already do this in prod.type HandlerFuncErr func(w http.ResponseWriter, r http.Request) errorfunc HandleErr(h HandlerFuncErr) http.HandlerFunc { return func(w http.ResponseWriter, r http.Request) { ... } }

blablabla123将近 3 年前

Seems a rather theoretical problem to me because usually an external library is used. Also in the underlying http response first the return code is written before the body. This matters a lot when for instance copying a buffer into the response

asp_hornet将近 3 年前

I thought a “naked return” was when the return types were named?

zxcvbn4038将近 3 年前

Who cares? Rando with a blog, maybe he'll like nodejs better.

JustSomeNobody将近 3 年前

They're not functions, they're procedures. Procedures don't return values. Write your code accordingly.