Skip to content

Commit

Permalink
reverseproxy: Revert #4952 - don't ignore context cancellation in str…
Browse files Browse the repository at this point in the history
…eam mode

i.e. Revert commit f5dce84

Two years ago, the patch in #4952 was a seemingly necessary way to fix an issue (sort of an edge case), but it broke other more common use cases (see #6666).

Now, as of #6669, it seems like the original issue can no longer be replicated, so we are reverting that patch, because it was incorrect anyway.

If it turns out the original issue returns, a more proper patch may be in #6669 (even if used as a baseline for a future fix). A potential future fix could be an opt-in setting.
  • Loading branch information
mholt committed Nov 12, 2024
1 parent b183aec commit 238f110
Showing 1 changed file with 0 additions and 14 deletions.
14 changes: 0 additions & 14 deletions modules/caddyhttp/reverseproxy/reverseproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,6 @@ type Handler struct {
// response is recognized as a streaming response, or if its
// content length is -1; for such responses, writes are flushed
// to the client immediately.
//
// Normally, a request will be canceled if the client disconnects
// before the response is received from the backend. If explicitly
// set to -1, client disconnection will be ignored and the request
// will be completed to help facilitate low-latency streaming.
FlushInterval caddy.Duration `json:"flush_interval,omitempty"`

// A list of IP ranges (supports CIDR notation) from which
Expand Down Expand Up @@ -833,15 +828,6 @@ func (h *Handler) reverseProxy(rw http.ResponseWriter, req *http.Request, origRe
}
req = req.WithContext(httptrace.WithClientTrace(req.Context(), trace))

// if FlushInterval is explicitly configured to -1 (i.e. flush continuously to achieve
// low-latency streaming), don't let the transport cancel the request if the client
// disconnects: user probably wants us to finish sending the data to the upstream
// regardless, and we should expect client disconnection in low-latency streaming
// scenarios (see issue #4922)
if h.FlushInterval == -1 {
req = req.WithContext(context.WithoutCancel(req.Context()))
}

// do the round-trip
start := time.Now()
res, err := h.Transport.RoundTrip(req)
Expand Down

0 comments on commit 238f110

Please sign in to comment.