Go: net/http: make default configs have better timeouts

Created on 26 Feb 2018  路  11Comments  路  Source: golang/go

See https://github.com/golang/go/issues/23459.

Client, Server, and Transport may all have timeout fields in which zero = infinity. Instead it should be a reasonable default.

NeedsInvestigation

Most helpful comment

This issue is close to my heart (1, 2, #16100) but while I agree there should be reasonable defaults, I think changing them now would be a full violation of Go 1 compatibility.

A default Server or Client can serve streaming requests today, with any timeout they couldn't.

All 11 comments

This issue is close to my heart (1, 2, #16100) but while I agree there should be reasonable defaults, I think changing them now would be a full violation of Go 1 compatibility.

A default Server or Client can serve streaming requests today, with any timeout they couldn't.

Perhaps 1.11 should have high, non-infinity default timeouts, letting people adapt with less of a shock before setting lower, more sensible defaults in 1.12.

I don't think the way we transition matters, IMHO it's a Go 1 compatibility promise violation: there's a specific real documented use case (streaming) that would stop working from one version to the other.

If we want to consider this a security exception, I'm not sure where I stand, but I think that's the frame in which we should evaluate it.

Perhaps the less intrusive change wrt backwards compatibility would be to only set non-zero timeouts on package-level http.ListenAndServe{,TLS} calls which create implicit Server instance, but keep them unchanged on explicitly created Servers?

/cc @bradfitz

At least we should mention the risk in the docs. See #22085

In #22982, an instance of this meta bug, I said in https://github.com/golang/go/issues/22982#issuecomment-394828083 :

I sent CL 116356 to add a 5 minute timeout, but right after I mailed it I realized it would break people wanting to do long downloads.

I think the only conservative thing we could do by default is to add some sort of InactivityTimeout, but it's too late for that, so bumping to Go 1.12.

Recording here an idea that someone at GopherCon suggested. (I don't remember who, apologies.)

Even if we can't change the DefaultClient behavior, we could add a DefaultClientWithTimeout, which although ugly and a mouthful, would prominently draw attention to the importance of client timeouts ("officially" endorsing it as a default, and giving us a place in the docs to elaborate), and provide a way to use them that's easy, maintained, and efficient (globally reusing connections).

Not sure how to map this to the server side, though.

Honestly, I think the DefaultClient is pretty much broken. If you try to traffic switch by changing DNS you can wait forever (or does it recently changed?).
@FiloSottile don鈥檛 you think my example is more a bug?

I am also fine with DefaultClientWithTimeout, but something should be changed in go1.

@szuecs If I understand correctly, the issue is that DefaultClient does not respond to changes in the DNS? I can see that being intended behavior due to connection reuse, or maybe due to hostname lookup caching, but in any case that's unrelated to request/response timeouts. Can you open a separate issue and expand on what you expected to happen vs. what actually happened? Thank you!

@FiloSottile a separate issue is already available https://github.com/golang/go/issues/23427

Was this page helpful?
0 / 5 - 0 ratings

Related issues

longzhizhi picture longzhizhi  路  3Comments

OneOfOne picture OneOfOne  路  3Comments

mingrammer picture mingrammer  路  3Comments

enoodle picture enoodle  路  3Comments

ajstarks picture ajstarks  路  3Comments