Repository: go
Issue: net/http/httptrace: WroteHeaders should be called after flush or provide an option to do so 
Labels: NeedsInvestigation,FeatureRequest

### Where we are coming from

We have a setup where an L4 proxy (Envoy) is between the go client and the target server. When the client opens a connection to the server, the proxy first accepts the TCP connection and does a TLS handshake at the frontend before trying to proxy the request forward to the backend.

If the backend is dead, the proxy will immediately close the frontend connection again. This results in a broken pipe at the client, because it was just about to send HTTP data and will now receive an io.EOF error (actually, it is a nothingWrittenError [unwrapped to an EOF](https://github.com/golang/go/blob/master/src/net/http/transport.go#L619) but I digress..)

### The issue we need to solve

Now the issue for us is, we would like to know if we can safely retry the request or not. For GET/HEAD/OPTIONS  etc. that's easy.
However, for POST/PUT requests we need to know if the server has actually received anything (= don't retry) or not (= retry).
Since all we get is an EOF, we can't be sure.

### httptrace
In an attempt to mitigate the situation we turned to httptrace and its `GotConn` and `WroteHeaders` callbacks.
The idea was to use them as indicators if any part of the request has actually made it to the backend server or not. Now, the problem with `WroteHeaders` is that it is called when the request headers have been written to the [bufio.Writer](https://github.com/golang/go/blob/master/src/net/http/request.go#L617) NOT the wire.

For us, this is pretty useless because of course a write to an in-memory buffer always succeeds. It does not tell you anything about whether or not the headers were actually sent to the target server. I question the usefulness of that callback.

I noticed that [further down](https://github.com/golang/go/blob/master/src/net/http/request.go#L702) the headers are actually flushed if the request is a POST with a body and some [other quirky conditions](https://github.com/golang/go/blob/master/src/net/http/transfer.go#L1097) (I'll get to that later).

My suggestion is to move the call to `trace.WroteHeaders()` below the (potential) flush to have some safety that the server has actually seen the headers before we claim that we wrote them.

An alternative could be the addition of a new `FlushedHeaders` callback in httptrace that is specific to this use case and is put [here](https://github.com/golang/go/blob/master/src/net/http/request.go#L705), i.e. called when no error occurred during flushing.

### Now about those 'quirky' flushing conditions...
I've read into [the commit](https://go-review.googlesource.com/c/go/+/114316/2/src/net/http/transfer.go) that introduced this behavior and wondered why it was done this way. Like the comment suggests, we should flush "just in case the server needs the headers early" - which is the case for us. However, we would like to be able to flush on all requests, including e.g. POSTs without a body. So can't we just make `FlushHeaders` a configurable field of `Request` and pass it down to the `transferWriter`? This way the client can decide if it wants to send early headers to the server, instead of having this decision taken away by the SDK.


### tl;dr
- httptrace.WroteHeaders is called already even if nothing was written to the wire
- Either: Move it below `Flush()`
- Or: Add a `httptrace.FlushedHeaders` callback that is called after `Flush()`
- Make `FlushHeaders` an option for requests so that the client can decide if their want to send headers early or not