Since this is working as intended and documented as such, and changing the behavior will likely break code (which we can’t do due to the Go 1 compatibility promise), this looks like something to consider for Go2.
As of Go 1, perhaps, WriteTimeout needs a clearer description. Maybe it's just me, but not only I had to read it several times, but I also had to check the source code to ensure that I understood it correctly.
For reference, right now it's like this:
// WriteTimeout is the maximum duration before timing out// writes of the response. It is reset whenever a new// request's header is read. Like ReadTimeout, it does not// let Handlers make decisions on a per-request basis.WriteTimeouttime.Duration
I think this code is working as documented, though no doubt the documentation could be improved. If you set a short timeout, that timeout applies. I can't tell precisely what different behavior you are after; perhaps using a context.Context would help here.
I can't tell precisely what different behavior you are after
Judging from WriteTimeout name alone, one would expect that timeout applies either to each Write separately or to writing response as a whole. However, WriteTimeout in fact imposes combined limit on reading request body, processing it (e.g. executing DB queries), and writing the response.
Moreover, even if WriteTimeout is hit during "request processing", it won't be detected until after request is completely processed (e.g. all DB queries are executed), and attempt to write the response is made. Like context.Context, WriteTimeout effectively "aborts" long requests, howeverer, unlike context.Context, it does so in rather ineffective way, i.e. it only prevents response from being written.
So, in my opinion, there're three related, but separate issues:
Current behaviour is questionable due to issues with requests taking too long (described above).
Variable name is somewhat misleading.
Documentation is somewhat vague.
As of documentation, Mattermost configuration page has this clear and accurate description what WriteTimeout actually does: "this is the maximum time allowed from the end of reading the request headers until the response is written." I like its wording.
(related to #21389)
What version of Go are you using (
go version go1.10 linux/amd64and
go version go1.9.4 linux/amd64, which behave the same.
Does this issue reproduce with the latest release?
What operating system and processor architecture are you using (
What did you do?
WriteTimeoutto some value, which was smaller than the time handler took to handle the request.
What did you expect to see?
Well, the documentation clearly states "It is reset whenever a new request's header is read.", after all. So strictly speaking, everything is working as intended.
However, I expected that
WriteTimeoutwould only timeout write calls that're taking too long, or at least that it would start ticking at the first write. It would make much more sense this way.
What did you see instead?
As soon as server tries to write anything, even it hasn't written anything before, assuming timeout is passed, it will close the connection.
The text was updated successfully, but these errors were encountered: