Superfluous WriteHeader and no response

Question

I'm writing an HTTP API that sends requests to a WebSocket gateway. I'm using go1.14.7 and gorilla/mux v1.8.0. The code will be cross-compiled using GOOS=linux GOARCH=arm GOARM=7.

My problems are the following:

respondBadRequest() always logs this even though I never use WriteHeader:
- http: superfluous response.WriteHeader call from main.(*HttpHandler).respondBadRequest
The API response is always 200 OK even when respondBadRequest() is called
The API response always has an empty body

I'm completely new to Go. Below is my code structure.

type HttpHandler struct {
  gateway Gateway
}

func (h *HttpHandler) respondSuccess(w http.ResponseWriter, text string) {
  w.Write([]byte(text))
}

func (h *HttpHandler) respondBadRequest(w http.ResponseWriter, text string) {
  http.Error(w, text, http.StatusBadRequest)
}

func (h *HttpHandler) respondError(w http.ResponseWriter, text string) {
  http.Error(w, text, http.StatusInternalServerError)
}

func (h *HttpHandler) OnFoobar(w http.ResponseWriter, r *http.Request) {
  f := func(success bool, e error) {
    if e != nil {
      h.respondError(w, e.Error())
    } else if success {
      h.respondSuccess(w, "Foobar Accepted")
    } else {
      h.respondBadRequest(w, "Unknown Foobar")
    }
  }
  //...
  e := h.gateway.Foobar(f)
  if e != nil {
    log.Println(e)
  }
}

//...

httpHandler := &HttpHandler{
  gateway: gateway,
}

r := mux.NewRouter()

r.HandleFunc("/foobar", httpHandler.OnFoobar)

http.ListenAndServe(":8000", r)

If you write any part of a response, `WriteHeader` is already called. You can't send a different header after you've already written a response. — JimB, Mar 16 '21 at 16:00
You're not calling `WriteHeader` directly, but you're calling it indirectly in several places; if you call `Write` and `WriteHeader` hasn't already been called, `Write` will call `WriteHeader(200)` (as stated in the docs). `http.Error` also calls `WriteHeader`. If you want to respond with an error, you have to do so *before* calling `Write` to send any body content (in an HTTP response, the status and headers *must* precede the response body). — Adrian, Mar 16 '21 at 16:05
Thanks for the replies, but I don't really understand what you mean. I'm aware that any `Write` will call `WriteHeader` since I tried to debug my issue before posting here. My issue is that I don't understand where that first `Write` happens that makes mine superfluous. Apparently in `gateway.Foobar()`, but I can't tell why or how to restructure this so that it doesn't interfere with my surrounding API code. — Steffen, Mar 16 '21 at 16:20
`f` looks like a callback, so I guess `Foobar` is asynchronous? Is it doing it's job in a goroutine? If so the handler `OnFoobar` does not wait for `Foobar` to finish and when it exits, if `w` hasn't been written to yet, Go's serve mux will write to it 200ok by default, then later when `Foobar`'s done and it invokes the callback you get the error you get. — mkopriva, Mar 16 '21 at 16:36
@mkopriva Yes, it's asynchronous and thanks for actually understanding the issue... Now that you mention it, it does seem obvious. I guess I could use a channel to delay `OnFinish()` until `f()` is done. Or is there a smarter way around this? — Steffen, Mar 16 '21 at 16:48
@Steffen that seems like unnecessary complexity to me, why don't you remove the callback, make Foobar synchronous, have it return an error that can be inspected to then decide upon the proper status code. Note that in Go the standard router, and I'm pretty sure gorilla/mux as well, already handle each request in a separate goroutine, so you don't have to roll your own asynchronicity. — mkopriva, Mar 16 '21 at 16:52
@mkopriva Unfortunately `Foobar` is from a third-party package I have no control over. — Steffen, Mar 16 '21 at 16:55
@Steffen something like this should do, I believe: https://play.golang.org/p/_2DkCVXmhcS — mkopriva, Mar 16 '21 at 17:06
@mkopriva I tried a channel and it works. Thanks so much. I have a look at `sync.WaitGroup` and answer my (apparently terrible...) question later. — Steffen, Mar 16 '21 at 17:07

score 1 · Answer 1 · edited Mar 16 '21 at 22:07

So as @mkopriva pointed out f is rather obviously called after OnFoobar already finished and wrote an implicit empty response. His fix using sync.WaitGroup:

func (h *HttpHandler) OnFoobar(w http.ResponseWriter, r *http.Request) {
  wg := new(sync.WaitGroup)
  wg.Add(1)
  f := func(success bool, e error) {
    if e != nil {
      h.respondError(w, e.Error())
    } else if success {
      h.respondSuccess(w, "Foobar Accepted")
    } else {
      h.respondBadRequest(w, "Unknown Foobar")
    }
    wg.Done()
  }
  //...
  e := h.gateway.Foobar(f)
  if e != nil {
    log.Println(e)
  }
  wg.Wait()
}

Superfluous WriteHeader and no response

1 Answers1