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

proposal: net/http: ResponseController to manipulate per-request timeouts (and other behaviors) #54136

Open
neild opened this issue Jul 29, 2022 · 2 comments
Assignees
Labels
Milestone

Comments

@neild
Copy link
Contributor

neild commented Jul 29, 2022

This proposal seeks to address #16100 (no way of manipulating timeouts in Handler), and is inspired by #16100 (comment).

HTTP handler timeouts are specified on a per-Server basis: ReadTimeout, WriteTimeout. It would be very useful to permit these timeouts (and possibly other options) to be overridden on a per-handler basis. For example, a handler which serves a long-running streaming response may want to extend the WriteTimeout after each write, and will likely want a different WriteTimeout than a handler serving a short response.

A problem is that we have no good place at the moment to add functions that adjust these timeouts. We might add methods to the ResponseWriter implementation and access them via type assertions (as is done with the existing Flush and Hijack methods), but this proliferation of undiscoverable magic methods scales poorly and does not interact well with middleware which wraps the ResponseWriter type.

Proposal: Add a new concrete ResponseController type which provides additional per-request controls. (A ResponseWriter writes the response, a ResponseController provides additional controls.) This type will supersede the existing Flusher and Hijacker APIs.

// A ResponseController is used by an HTTP handler to control the response.
//
// A ResponseController may not be used after the Handler.ServeHTTP method has returned.
type ResponseController struct{}

// NewResponseController creates a ResponseController for a request.
//
// The Request must be the original value passed to the Handler.ServeHTTP method.
// The ResponseWriter must be the original value passed to the Handler.ServeHTTP method,
// or have an Unwrap() method returning the original ResponseWriter.
func NewResponseController(rw ResponseWriter, req *Request) *ResponseController

// Flush flushes buffered data to the client.
func (rc *ResponseController) Flush() error

// Hijack lets the caller take over the connection.
// See the Hijacker interface for details.
func (rc *ResponseController) Hijack() (net.Conn, *bufio.ReadWriter, error)

We additionally add the ability to set the read and write deadline on a per-request basis, via ResponseController. These functions take a deadline rather than a timeout, for consistency with net.Conn.

// SetReadDeadline sets the deadline for reading the entire request, including the body.
// Reads from the request body after the deadline has been exceeded will return an error.
// A zero value means no deadline.
//
// The read deadline may not be extended after it has been exceeded.
func (rc *ResponseController) SetReadDeadline(deadline time.Time) error

// SetWriteDeadline sets the deadline for writing the response.
// Writes to the response body after the deadline has been exceeded will not block,
// but may succeed if the data has been buffered.
// A zero value means no deadline.
//
// The write deadline may not be extended after it has been exceeded.
func (rc *ResponseController) SetWriteDeadline(deadline time.Time) error

The Handler returned by http.TimeoutHandler currently receives a ResponseWriter which does not implement the Flush or Hijack methods. This will not change under this proposal: The *ResponseController for a TimeoutHandler will return a not-implemented error from Flush, Hijack, SetWriteDeadline, and SetReadDeadline.

@neild neild added the Proposal label Jul 29, 2022
@gopherbot gopherbot added this to the Proposal milestone Jul 29, 2022
@neild neild self-assigned this Jul 29, 2022
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jul 29, 2022
@gopherbot
Copy link

gopherbot commented Jul 29, 2022

Change https://go.dev/cl/420174 mentions this issue: net/http: work in progress per-request timeouts

@mborsz
Copy link

mborsz commented Aug 9, 2022

Thanks for the proposal.

IIUC this will unblock removing timeoutHandler in kubernetes completely which has been a source of many hard to debug races in the past (kubernetes/kubernetes#105884).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

3 participants