The problem with Go’s default HTTP handlers (preslav.me)

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.

    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.StatusInternalServerError)
     }
    }

    http.Handle("/hello", MyHandlerFunc(someHandler))

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.

    // current
    func responseHandler(rw http.ResponseWriter, req *http.Request) {
        fi, err := os.OpenFile("/tmp/file")
        if err != nil {
            rw.WriteHeader(500)
            rw.Write([]byte("error opening file"))
        }
        io.Copy(rw, fi)
        fi.Close()
    }

    // returning a streaming Response object
    func responseHandler(req *http.Request) *http.HandlerResponse {
        fi, err := os.OpenFile("/tmp/file")
        if err != nil {
            return &http.HandlerResponse{Status: 500, Body: ioutil.NopCloser(bytes.NewReader([]byte("error opening file"))}
        }
        return &http.HandlerResponse{Status: 200, Body: fi}
    }

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 {

    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))

}

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.

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.

    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
    }

and then it's used in the handler funcs

    if api.HandleOnError(w, err, http.StatusInternalServerError, "Getting stuff for things") {
         return
    }

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.

Another possible extension of that design is using error types for HTTP codes. Something like:

  type HTTPError struct {
          Err  error
          Code int
  }

And then, with a couple of wrappers like this:

  func errorNotFound(err error) (wrapped error) {
          return &HTTPError{
                  Err:  err,
                  Code: http.StatusNotFound,
          }
  }

you could do something like this:

  return errorNotFound(err)

or this:

  return errorInternal(err) // 500 Internal Server Error

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

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

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

we have already do this in prod.

type HandlerFuncErr func(w http.ResponseWriter, r http.Request) error

func HandleErr(h HandlerFuncErr) http.HandlerFunc { return func(w http.ResponseWriter, r http.Request) { ... } }

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.

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.

https://github.com/barthr/web-util

And the blog post explaining it: https://bartfokker.com/posts/clean-handlers

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.

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)

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.

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:

  if err == util.ErrLimitReached {
      return errHTTPEntityTooLargeAttachmentTooLarge
  } 

Or so:

  if err != nil {
    return wrapErrHTTP(errHTTPBadRequestActionsInvalid, err.Error())
  }

[1] https://github.com/binwiederhier/ntfy/blob/main/server/serve...

[2] https://github.com/binwiederhier/ntfy/blob/main/server/error...

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

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

  age := getUserAge()
  if age < 18 {
     w.Write([]byte("You are too young"))
     // Forgot to return here...
  }
  w.Write([]byte("<insert adult content>"))

  var err error
  defer func() {
    if err != nil {
      w.Write([]byte(err.Error())
    }
  }()

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.

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.

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)

Surely one can make the exact same mistake with Echo?

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

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.

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:

  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)

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.