Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http: clarify that TimeoutHandler buffers all writes into memory #47899

Open
jsign opened this issue Aug 22, 2021 · 2 comments · May be fixed by #48038
Open

net/http: clarify that TimeoutHandler buffers all writes into memory #47899

jsign opened this issue Aug 22, 2021 · 2 comments · May be fixed by #48038
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@jsign
Copy link
Contributor

jsign commented Aug 22, 2021

While reading http.TimeoutHandler implementation I noticed that in current docs there's no mention that all writes in the response are buffered in memory before being flushed to the client.

Digging a bit in the git history, this was a clarification that was present on the original implementation.

The last change that touched the docs has a good summary of some back and forth in implementing Flusher, which docs changes made this original note be lost.

It feels to me that clarifying that all writes are buffered to memory is something worth adding again since that should be considered by users when wrapping handlers that might produce big writes.

The proposed new docs for that paragraph could be:

// TimeoutHandler buffers all Handler writes to memory. 
// It supports the Pusher interface but does not support the Hijacker 
// or Flusher interfaces.

If that sounds like a good idea, I can create a CL with this change.

@seankhliao
Copy link
Member

cc @neild

@seankhliao seankhliao added Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 23, 2021
@gopherbot
Copy link

Change https://golang.org/cl/345795 mentions this issue: net/http: clarify TimeoutHandler buffer writes to memory

@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants