net/http: no Client API to close server connection based on Response

[mborsz]

What version of Go are you using (go version)?

1.8

What operating system and processor architecture are you using (go env)?

linux/amd64

What did you do?

I'm using http.Client that connects to HTTP/2 server. I would like to prevent reusing the connection if last response had error 500.

I tried to implement something like this, but I failed:

• Transport.CancelRequest doesn't work for HTTP/2,
• Request.Cancel works only on a single stream in HTTP/2 connection,
• CloseIdleConnections won't work if the connection is in use.
• I also tried to use httptrace.GetConn to steal net.Conn used by the request and close it, but this is against httptrace protocol.

I did small research and I can't find any way to implement such a thing.
This is a feature request to add it.

Background:
I have an environment where I have tcp load balancer with few replicas behind.
While new connections are directed only to healthy replicas, the existing ones are always attached to the same replica (even if it becomes unhealthy).

If http.Client is attached to replica which became unhealthy, reusing the same connection will cause directing request to the same unhealthy instance.
A more reasonable approach in this case is to reconnect so that load balancer can attach a healthy replica.

[ianlancetaylor]

CC @bradfitz @tombergan

[bradfitz]

I'm not sure this is even HTTP/2 specific. The net/http API doesn't really expose much or any details of the underlying connections. It's mostly a request/response-based API only.

We do have CloseIdleConnections, but that should work the same way between HTTP/1 and HTTP/2. If not, that's a bug and we should fix that, rather than add new API surface.

That said, even with CloseIdleConnections, there's no nice way to do what you're wanting with either HTTP/1 or HTTP/2, as you can't close the connection until it's idle, and by the time it's "idle", another request might've arrived on it.

@rs sent out a document about expanding the HTTP/2 connection pool interface while I was on vacation. See #17776 (comment) and that bug and linked CLs. Maybe this is another use case that design could address. @rs, thoughts?

I suppose one path could be taking advantage of the fact that net/http.(*Response).Body is an interface, and add support for some optional CloseConnection method on it that acts like Close but also nukes the connection.

[rs]

If you implement your own connection pool (with the patches I proposed here), you could just remove the connection from the pool when you detect a 500 error. To close the connection, you then call CloseIfIdle in MarkComplete if the connection is not part the pool. This way you will gracefully close the discarded connections once all in-progress streams are completed.

[rsc]

Duplicate of #20977.

[rs]

It is not. #20977 is about server closing client connection. Here we are taking about client closing server connection.

[bradfitz]

Oh, okay. We were triaging too quickly and I got confused about which bugs were which.

[jim-minter]

Bumping this thread -- it's old but still an issue.

As an HTTP client, I'm hitting a case where some HTTP server instance behind a load balancer breaks and starts returning 500s (FWIW with no body) and without the "Connection: close" header. I retry, but I end up reusing the same TCP connection to the same broken HTTP instance, so I never hit a different backend server and my retry policy is basically useless.

Obviously I need to get the server owner to fix its behavior, but it would be great if, as a client, there were a way to get net/http not to reuse the connection further, in order to be less beholden to the server's behavior.

This happens with both HTTP/1.1 and HTTP/2.

If appropriate, I could live with the request to close the connection racing with other new requests to the same endpoint. Getting to the point where 2 or 3 requests fail and then the connection is closed is way better than having requests fail ad infinitum.

http.Transport.CloseIdleConnections() doesn't solve the problem well (a) because it's a big hammer, and (b) because there's no guarantee that the connection is idle when CloseIdleConnections() is called.

FWIW I can see in func (pc *persistConn) readLoop() there's the following test:

if resp.Close || rc.req.Close || resp.StatusCode <= 199 || bodyWritable {
	// Don't do keep-alive on error if either party requested a close
	// or we get an unexpected informational (1xx) response.
	// StatusCode 100 is already handled above.
	alive = false
}

I imagine that extending that to if resp.Close || rc.req.Close || resp.StatusCode <= 199 || bodyWritable || resp.StatusCode >= 500 { might probably help this specific case, but I imagine that's an unacceptably large behavior change for the rest of the world.

I'm not sure how else this could neatly be done.

[jim-minter]

I created a thread on go-nuts about this ("net/http: no Client API to close server connection based on Response #21978") and got some useful advice.

For reference, https://go.dev/play/p/ijSN9CsQFbc is the sort of thing we're going to try out. I think it might help in our somewhat exceptional circumstance (500s like this are rare, haven't yet been able to diagnose and fix the server-side root cause, transport is bound to specific API client, etc.) but others' mileage may vary. A potentially useful hack but not a general purpose kind of thing.