golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.98k stars 17.67k forks source link

net/http: http.Client Timeout incompatible with writable bodies #31391

Open drigz opened 5 years ago

drigz commented 5 years ago

As part of #26937, Transport started returning writable bodies for 101 Switching Protocol responses. This is great, thank you!

However, if you have a non-zero Timeout on your http.Client, you can't use this, because of the following lines from client.go:

    if !deadline.IsZero() {
        resp.Body = &cancelTimerBody{
            stop:          stopTimer,
            rc:            resp.Body,
            reqDidTimeout: didTimeout,
        }
    }

This wraps the writable body in a non-writable body.

Go version: 1.12 Observed behavior: res.Body.(io.ReadWriteCloser) fails when client.Timeout > 0. Expected behavior: timeouts and writeable bodies are compatible.

Although we have some streaming requests, we would like to set a long (eg one hour) timeout on our requests to prevent resource exhaustion. #30876 suggests that we can't even do this manually with contexts.

agnivade commented 5 years ago

@bradfitz

nhooyr commented 5 years ago

How about we change the context cancellation to also apply to the request body. Then, when the body is first written to, both the context cancellation and Timeout are reset to give the application full control over the protocol being spoken now?

jim-minter commented 4 years ago

I think I've been able to hack around this issue outside of the standard library as follows:

// cancelBody is a workaround for the fact that http timeouts are incompatible
// with hijacked connections (https://github.com/golang/go/issues/31391):
// net/http.cancelTimerBody does not implement Writer.
type cancelBody struct {
    io.ReadWriteCloser
    t *time.Timer
    c chan struct{}
}

func (b *cancelBody) wait() {
    select {
    case <-b.t.C:
        b.ReadWriteCloser.Close()
    case <-b.c:
        b.t.Stop()
    }
}

func (b *cancelBody) Close() error {
    select {
    case b.c <- struct{}{}:
    default:
    }

    return b.ReadWriteCloser.Close()
}

func newCancelBody(rwc io.ReadWriteCloser, d time.Duration) io.ReadWriteCloser {
    b := &cancelBody{
        ReadWriteCloser: rwc,
        t:               time.NewTimer(d),
        c:               make(chan struct{}),
    }

    go b.wait()

    return b
}

Where appropriate, add resp.Body = newCancelBody(resp.Body.(io.ReadWriteCloser), time.Hour) to wrap the old resp.Body in one that will close itself after an hour but which still allows 101 Switching Protocol responses.