The problem with Go’s default HTTP handlers • Hacker News Board

Re: The problem with Go’s default HTTP handlers

#41

Post by mjevans » Tue, Aug 09, 2022, 6:20 PM UTC

jayd16 wrote: post #35

Someone wrote: post #26

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 lib…

Well its a bit of an issue with escape analysis to know for sure that something is or isn't called before the handler returns. Imagine a scenario where multiple threads get involved. That would be a lot to track. I don't think its a great pattern but just speaking hypothetically, if you ensure that only valid Response instances exist (because you organized the constructors to only make valid instances and nulls are i…

... Warning ${function} took too long to check and was skipped; consider refactoring if possible or add UNSAFE.NoEscapeTracking to the function signature.

Maybe something like that to let the humans know and make higher level choices?

Re: The problem with Go’s default HTTP handlers

#42

Post by catlifeonmars » Tue, Aug 09, 2022, 6:30 PM UTC

skrtskrt wrote: post #40

masklinn wrote: post #29

Earlier quoted context omitted.

> the mitigation is relatively straightforward by using custom handlers like mentioned at the end, if needed. Why not implement ServeHTTP on the custom handler though? That's not exactly difficult: type MyHandlerFunc func(w http.ResponseWriter, r *http.Request) error func (f MyHandlerFunc) ServeHTTP(w http.ResponseWriter, r *http.Request) { err := f(w, r) if err != nil { http.Error(w, err.Error(), http.StatusInternal…

Actually a bunch of router/web helper libraries do this in different ways which makes it a huge pain to compose middleware from different libraries. Even Mux's middleware library is not consistent in how it does this for different middlewares so they have to be applied different ways or even have wrapper functions written

This. Middlewares typically expect to wrap an http.Handler so you lose all the middleware composition ergonomics by adding a return value to your custom handler signature

Re: The problem with Go’s default HTTP handlers

#43

Post by randomdata » Tue, Aug 09, 2022, 6:31 PM UTC

adontz wrote: post #33

To me it seems like a problem induced by the way Go handles errors. With exceptions it would be something like 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!")) } and no such problem

There is nothing special about errors, though. This same problem is present for any kind of branching operation.

  age := getUserAge()
  if age "))

Re: The problem with Go’s default HTTP handlers

#44

Post by masklinn » Tue, Aug 09, 2022, 6:32 PM UTC

skrtskrt wrote: post #40

masklinn wrote: post #29

Earlier quoted context omitted.

> the mitigation is relatively straightforward by using custom handlers like mentioned at the end, if needed. Why not implement ServeHTTP on the custom handler though? That's not exactly difficult: type MyHandlerFunc func(w http.ResponseWriter, r *http.Request) error func (f MyHandlerFunc) ServeHTTP(w http.ResponseWriter, r *http.Request) { err := f(w, r) if err != nil { http.Error(w, err.Error(), http.StatusInternal…

Actually a bunch of router/web helper libraries do this in different ways which makes it a huge pain to compose middleware from different libraries. Even Mux's middleware library is not consistent in how it does this for different middlewares so they have to be applied different ways or even have wrapper functions written

Here it just implements the standard library’s interface rather than use some sort of conversion closure adapter, there’s nothing special to it.

Re: The problem with Go’s default HTTP handlers

#45

Post by catlifeonmars » Tue, Aug 09, 2022, 6:33 PM UTC

andrewstuart2 wrote: post #39

masklinn wrote: post #29

Earlier quoted context omitted.

> the mitigation is relatively straightforward by using custom handlers like mentioned at the end, if needed. Why not implement ServeHTTP on the custom handler though? That's not exactly difficult: type MyHandlerFunc func(w http.ResponseWriter, r *http.Request) error func (f MyHandlerFunc) ServeHTTP(w http.ResponseWriter, r *http.Request) { err := f(w, r) if err != nil { http.Error(w, err.Error(), http.StatusInternal…

There are definitely edge cases here (like what if the handler already wrote a body and it was sent on the wire, etc), but this is absolutely the pattern to use, and one of my favorite parts of a type system that allows methods on function types.

Yes, or what if you want to upgrade the connection to a websocket?

Re: The problem with Go’s default HTTP handlers

#46

Post by weltensturm » Tue, Aug 09, 2022, 6:44 PM UTC

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)

Re: The problem with Go’s default HTTP handlers

#47

Post by AtNightWeCode » Tue, Aug 09, 2022, 6:59 PM UTC

Surely one can make the exact same mistake with Echo?

if id == "" { fmt.Errorf("get album: missing id") }

Re: The problem with Go’s default HTTP handlers

#48

Post by merb » Tue, Aug 09, 2022, 6:59 PM UTC

dralley wrote: post #4

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 , ) -> impl IntoResponse { let user = User { id: 1337, username: payload.username, }; // this will be converted into a JSON response // with a status code of `201 Created` (Status…

the problem is that when you use NO_CONTENT that you prolly can violate the http spec. (you can do that with both)

Re: The problem with Go’s default HTTP handlers

#49

Post by n4jm4 » Tue, Aug 09, 2022, 7:02 PM UTC

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.

Re: The problem with Go’s default HTTP handlers

#50

Post by morelisp » Tue, Aug 09, 2022, 7:13 PM UTC

gorjusborg wrote: post #30

mrweasel wrote: post #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 poin…

> In the HTTP handlers it makes sense that you don't have return values, because: What would you do with that value exactly? I think that approach used by clojure's ring shows an elegant way to represent http responses https://github.com/ring-clojure/ring/wiki/Concepts#responses . They are essentially structs with the following fields: status := number headers := map of string->string body := stream | string | seq |…

There are major performance advantages, especially once you're past HTTP/1.1, to passing a mutable response (i.e. in Go a ResponseWriter) to the handler rather than expecting the handler to return something created whole-cloth.