diff --git a/pkg/client/octopusdeploy.go b/pkg/client/octopusdeploy.go index 629c7275..e74faa7b 100644 --- a/pkg/client/octopusdeploy.go +++ b/pkg/client/octopusdeploy.go @@ -49,6 +49,7 @@ import ( "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/projects" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/projectvariables" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/proxies" + "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/ratelimit" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/releases" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/reporting" "github.com/OctopusDeploy/go-octopusdeploy/v2/pkg/runbooks" @@ -208,6 +209,22 @@ func NewClientForTool(httpClient *http.Client, apiURL *url.URL, apiKey string, s // NewClientWithCredentials returns a new Octopus API client with the specified credentials and a tool reference in the useragent string. // If a nil client is provided, a new http.Client will be used. func NewClientWithCredentials(httpClient *http.Client, apiURL *url.URL, apiCredentials ICredential, spaceID string, requestingTool string) (*Client, error) { + return NewClientWithOptions(httpClient, apiURL, apiCredentials, spaceID, requestingTool, ClientOptions{}) +} + +// ClientOptions configures optional client behaviour. The zero value gives the defaults. +type ClientOptions struct { + // RateLimit configures how the client deals with the server's rate limiting. By default, requests the server + // rejects with HTTP 429 are retried, and requests are not paced against the server's advisory rate limit headers. + RateLimit ratelimit.Options +} + +// NewClientWithOptions returns a new Octopus API client with the specified credentials, a tool reference in the +// useragent string, and the given options. If a nil client is provided, a new http.Client will be used. +// +// The client makes its requests through a copy of httpClient whose Transport is wrapped to handle the server's rate +// limiting (see the ratelimit package); httpClient itself is not modified. +func NewClientWithOptions(httpClient *http.Client, apiURL *url.URL, apiCredentials ICredential, spaceID string, requestingTool string, options ClientOptions) (*Client, error) { if apiURL == nil { return nil, internal.CreateInvalidParameterError("NewClient", "apiURL") } @@ -227,6 +244,12 @@ func NewClientWithCredentials(httpClient *http.Client, apiURL *url.URL, apiCrede httpClient = &http.Client{} } + // Every request this client makes, through sling or the httpSession, goes out on this one http.Client, so they all + // share one view of the rate limit allowance they are spending. + rateLimitedClient := *httpClient + rateLimitedClient.Transport = ratelimit.NewTransport(httpClient.Transport, options.RateLimit) + httpClient = &rateLimitedClient + // fetch root resource and process paths base, root, err := getRoot(httpClient, baseURLWithAPI, apiCredentials, requestingTool) if err != nil { diff --git a/pkg/newclient/httpsession.go b/pkg/newclient/httpsession.go index b1e42819..be2a42a2 100644 --- a/pkg/newclient/httpsession.go +++ b/pkg/newclient/httpsession.go @@ -73,6 +73,11 @@ func (h *HttpSession) DoRawJsonRequest(req *http.Request, requestBody any, outpu req.Header.Set("Content-Type", "application/json") } req.Body = io.NopCloser(bytes.NewReader(body)) + req.ContentLength = int64(len(body)) + // GetBody lets the request be replayed, e.g. when it's retried after the server rate limits it. + req.GetBody = func() (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader(body)), nil + } } if req.Header.Get("Accept") == "" { diff --git a/pkg/ratelimit/options.go b/pkg/ratelimit/options.go new file mode 100644 index 00000000..7bb2d8ad --- /dev/null +++ b/pkg/ratelimit/options.go @@ -0,0 +1,65 @@ +// Package ratelimit keeps the client's requests inside the rate limit the Octopus Server enforces. +// +// It does two things, both of which mirror the .NET client and the web portal: +// - Retrying: requests the server rejects with HTTP 429 (Too Many Requests) are retried after the delay in the +// response's Retry-After header, backing off with each attempt. On by default. +// - Pacing: the client spaces its requests out using the advisory Octopus-RateLimit-Policy and Octopus-RateLimit +// headers, so that it stays inside the limit rather than running at it and being rejected. Off by default; see +// Options.UseRateLimitHeaders. +package ratelimit + +import "time" + +const ( + defaultRetryCount = 5 + defaultRetryDefaultDelay = 2 * time.Second + defaultRetryMaxDelay = 30 * time.Second +) + +// Options configures how the client deals with the server's rate limiting. The zero value gives the defaults: 429s +// are retried, and requests are not paced. +type Options struct { + // RetryCount is the maximum number of times a request will be retried after the server rejects it with HTTP 429 + // (Too Many Requests) because it hit a rate limit. Zero means the default of 5. Set it to a negative number to + // turn off retrying, in which case the HTTP 429 surfaces to the caller as an error. + RetryCount int + + // RetryDefaultDelay is how long to wait before retrying an HTTP 429 response that has no Retry-After header, + // before backing off. Zero means the default of 2 seconds. + RetryDefaultDelay time.Duration + + // RetryMaxDelay is the longest we are willing to wait between retries. If the server's Retry-After asks us to wait + // longer than this, we stop retrying and the HTTP 429 surfaces to the caller as an error. Zero means the default of + // 30 seconds. + RetryMaxDelay time.Duration + + // UseRateLimitHeaders makes the client pace itself against the advisory Octopus-RateLimit-Policy and + // Octopus-RateLimit headers the server sends, spacing requests out so that it stays inside the limit instead of + // running at it and being rejected with HTTP 429. Off by default, because it makes the client wait on its own + // initiative, which a caller that would rather see the HTTP 429 doesn't want. + UseRateLimitHeaders bool +} + +func (o Options) retryCount() int { + if o.RetryCount < 0 { + return 0 + } + if o.RetryCount == 0 { + return defaultRetryCount + } + return o.RetryCount +} + +func (o Options) retryDefaultDelay() time.Duration { + if o.RetryDefaultDelay <= 0 { + return defaultRetryDefaultDelay + } + return o.RetryDefaultDelay +} + +func (o Options) retryMaxDelay() time.Duration { + if o.RetryMaxDelay <= 0 { + return defaultRetryMaxDelay + } + return o.RetryMaxDelay +} diff --git a/pkg/ratelimit/pacer.go b/pkg/ratelimit/pacer.go new file mode 100644 index 00000000..1ee9cb8c --- /dev/null +++ b/pkg/ratelimit/pacer.go @@ -0,0 +1,203 @@ +package ratelimit + +import ( + "math" + "regexp" + "strconv" + "strings" + "sync" + "time" +) + +const ( + // PolicyHeaderName advertises the policy in force for us, e.g. `l=200;rpm=600`: l is the burst limit and rpm + // how fast it refills. Static per policy, so it rarely changes. + PolicyHeaderName = "Octopus-RateLimit-Policy" + + // RateLimitHeaderName advertises where we currently stand against that policy, e.g. `r=7;t=18`: r is what we + // have left and t how long until the whole allowance is back. Sent on every response. + RateLimitHeaderName = "Octopus-RateLimit" +) + +// reservedRequests is how much of the advertised allowance we hold back rather than spend. The server's remaining +// count is whole requests and its bucket refills on a timer rather than continuously, so its arithmetic and ours +// disagree slightly; stopping a request short of the line means that disagreement costs us a small wait instead of +// a rejection. +const reservedRequests = 1 + +// pacer paces outgoing requests against the rate limit the server advertises, using GCRA (the generic cell rate +// algorithm) - the same shape of algorithm the server's token bucket uses, run in reverse. It is a port of the .NET +// client's RateLimitPacer and the web portal's rateLimitPacer.ts, so all three behave the same way. +// +// The state is a single "theoretical arrival time" (TAT): the point at which the requests we have already sent will +// have been paid for at the policy's steady rate. A request may go as soon as the TAT is within burstTolerance of +// now, and each request pushes the TAT out by one emissionInterval. That gives us the same "burst then settle to the +// steady rate" shape as the server's bucket, without needing a timer to refill anything. +// +// Every response resyncs the TAT to what the server says is left, so we self-correct: if something else is spending +// the same user's allowance, or the server's replenishment doesn't line up exactly with our arithmetic, the next +// response pulls us back into line. The resync only ever moves the TAT later, never earlier, so a response that +// overtook a newer one can't talk us into speeding up. +// +// A pacer with no policy is inert. Nothing paces until the server tells us what the limit is, so an older server or +// an unlimited policy behaves exactly as it did before. +type pacer struct { + mu sync.Mutex + + // now returns elapsed time on a monotonic clock rather than wall clock time: we only ever compare our own + // timestamps, and this can't jump. Replaceable in tests. + now func() time.Duration + + havePolicy bool + burstLimit float64 + requestsPerMinute float64 + // emissionInterval is how long a single request costs at the policy's steady rate. + emissionInterval time.Duration + // burstTolerance is how far ahead of the steady rate we are allowed to run. + burstTolerance time.Duration + theoreticalArrivalTime time.Duration +} + +func newPacer() *pacer { + start := time.Now() + return &pacer{now: func() time.Duration { return time.Since(start) }} +} + +// reserveSlot claims the next slot in the schedule and returns how long the caller should wait before sending - zero +// when we have allowance to spare, which is the normal case. +// +// The slot is claimed whether or not the caller ends up waiting, so concurrent callers get consecutive slots and +// space themselves out, rather than each being told it can go now. +func (p *pacer) reserveSlot() time.Duration { + p.mu.Lock() + defer p.mu.Unlock() + + // Until the server has told us what the limit is, there's nothing to pace against. + if !p.havePolicy { + return 0 + } + + now := p.now() + earliestSendTime := p.theoreticalArrivalTime - p.burstTolerance + + p.theoreticalArrivalTime = max(p.theoreticalArrivalTime, now) + p.emissionInterval + + if earliestSendTime > now { + return earliestSendTime - now + } + return 0 +} + +// observeResponse takes in what the server told us about the limit on a response, and resynchronises our schedule +// with it. Either header value may be empty (an older server, or an endpoint the server doesn't rate limit); we keep +// the policy we already knew, and give back the slot the request claimed - see refundSlot. +// +// Call this for every response, including error responses: the server writes these headers before the endpoint +// runs, so a 429 or a 500 from a rate limited endpoint still tells us where we stand. +func (p *pacer) observeResponse(policyHeaderValue string, rateLimitHeaderValue string) { + p.mu.Lock() + defer p.mu.Unlock() + + limit, haveLimit := getParameter(policyHeaderValue, "l") + rate, haveRate := getParameter(policyHeaderValue, "rpm") + + // Only take a policy that makes sense, and only do this work when it's actually new: the policy header is static + // for as long as we stay on the same policy, so this is the same values on nearly every response, and reapplying + // them would throw away the schedule we're keeping. + if haveLimit && haveRate && limit >= 1 && rate > 0 && (!p.havePolicy || limit != p.burstLimit || rate != p.requestsPerMinute) { + p.burstLimit = math.Floor(limit) + p.requestsPerMinute = rate + p.emissionInterval = time.Duration(float64(time.Minute) / rate) + + // One for the request being sent, one held back; the rest of the bucket is ours to burst through. + allowedBurst := p.burstLimit - 1 - reservedRequests + if allowedBurst > 0 { + p.burstTolerance = time.Duration(float64(p.emissionInterval) * allowedBurst) + } else { + p.burstTolerance = 0 + } + + // The schedule we were keeping is denominated in the old policy's units, so it means nothing now. The + // remaining count below, read from this same response, puts us back on track immediately. + p.theoreticalArrivalTime = p.now() + + p.havePolicy = true + } + + if !p.havePolicy { + return + } + + remaining, haveRemaining := getParameter(rateLimitHeaderValue, "r") + if !haveRemaining { + p.refundSlot() + return + } + + remaining = min(max(remaining, 0), p.burstLimit) + + // What the server says we've spent, expressed as a point in time the way we keep it. We only ever move the + // schedule later, never earlier: responses can arrive out of order, and an older one arriving last reports a + // fuller bucket than the truth, which would otherwise talk us into speeding up. + spent := time.Duration(float64(p.emissionInterval) * (p.burstLimit - remaining)) + p.theoreticalArrivalTime = max(p.theoreticalArrivalTime, p.now()+spent) +} + +// refundSlot gives back the slot a request claimed, because the response told us the request never cost the limiter +// anything: no remaining count means either the endpoint isn't rate limited (the server opts a handful of them out, +// and puts others on limiters that send no advisory headers) or the request never reached the limiter at all. +// +// Without this, reserveSlot spends schedule on requests the server didn't charge us for, and a caller that mixes +// limited and unlimited endpoints paces its whole request volume against a limit that only governs part of it. +// +// Refunding too much is safe in a way spending too much isn't: the resync in observeResponse only ever raises the +// schedule towards what the server says is left, so the next response that does carry a count puts the floor +// straight back under us. Spending too much has no such correction, because nothing lowers the schedule except time +// passing. +// +// The caller must hold p.mu. +func (p *pacer) refundSlot() { + // Only ever give back time we're actually holding. A schedule that has already fallen behind the clock isn't + // pacing anything, and pulling it further into the past would bank credit for a future burst. + now := p.now() + if p.theoreticalArrivalTime > now { + p.theoreticalArrivalTime = max(p.theoreticalArrivalTime-p.emissionInterval, now) + } +} + +// numberPattern is the plain decimal numbers we accept as a parameter value. strconv.ParseFloat on its own is more +// forgiving than we want here: it reads "Inf", "NaN" and hex floats such as "0x1p4". +var numberPattern = regexp.MustCompile(`^[-+]?(\d+(\.\d*)?|\.\d+)([eE][-+]?\d+)?$`) + +// getParameter reads one parameter out of a `name=value;name=value` header, e.g. `l` from `l=200;rpm=600`. Returns +// false if the header is empty or has no such parameter, and tolerates parameters we don't know about so the server +// can add more without breaking us. +func getParameter(headerValue string, name string) (float64, bool) { + if headerValue == "" { + return 0, false + } + + for _, parameter := range strings.Split(headerValue, ";") { + key, rawValue, found := strings.Cut(parameter, "=") + if !found { + continue + } + + if !strings.EqualFold(strings.TrimSpace(key), name) { + continue + } + + rawValue = strings.TrimSpace(rawValue) + if !numberPattern.MatchString(rawValue) { + return 0, false + } + + value, err := strconv.ParseFloat(rawValue, 64) + if err != nil { + return 0, false + } + return value, true + } + + return 0, false +} diff --git a/pkg/ratelimit/pacer_test.go b/pkg/ratelimit/pacer_test.go new file mode 100644 index 00000000..acd4789a --- /dev/null +++ b/pkg/ratelimit/pacer_test.go @@ -0,0 +1,266 @@ +package ratelimit + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// A burst of 10 refilling at one request a second. The pacer holds one request back, so it will let 9 of those 10 go +// at once and then settle to a request a second. +const testPolicy = "l=10;rpm=60" + +const ( + emissionInterval = time.Second + twoEmissionIntervals = 2 * time.Second +) + +// newTestPacer returns a pacer on a clock that only moves when the test moves it, so the delays are exact. +func newTestPacer() (*pacer, *time.Duration) { + now := new(time.Duration) + *now = time.Hour // well clear of zero, so nothing is accidentally relying on the clock starting there + return &pacer{now: func() time.Duration { return *now }}, now +} + +func TestPacer_BeforeTheServerHasToldUsAnything_DoesNotWait(t *testing.T) { + p, _ := newTestPacer() + + require.Zero(t, p.reserveSlot()) +} + +func TestPacer_WhenTheServerSendsNoRateLimitingHeaders_DoesNotWait(t *testing.T) { + p, _ := newTestPacer() + + // A server with no policy for this caller says nothing at all, and we shouldn't invent a limit for it. + p.observeResponse("", "") + + require.Zero(t, p.reserveSlot()) +} + +func TestPacer_WithQuotaToSpare_DoesNotWait(t *testing.T) { + p, _ := newTestPacer() + p.observeResponse(testPolicy, "r=5;t=5") + + require.Zero(t, p.reserveSlot()) +} + +func TestPacer_WhenOnlyTheReservedRequestIsLeft_WaitsForOneRequestToComeBack(t *testing.T) { + p, _ := newTestPacer() + + // One request left means the server would let us through, but the request after that would be rejected, and the + // two sides' arithmetic doesn't agree to the last fraction of a request. So we stop here. + p.observeResponse(testPolicy, "r=1;t=9") + + require.Equal(t, emissionInterval, p.reserveSlot()) +} + +func TestPacer_WhenTheQuotaIsSpent_WaitsForTheReservedRequestToComeBackToo(t *testing.T) { + p, _ := newTestPacer() + p.observeResponse(testPolicy, "r=0;t=10") + + require.Equal(t, twoEmissionIntervals, p.reserveSlot()) +} + +func TestPacer_WithNoFurtherResponses_SpendsTheBurstAndThenPacesItself(t *testing.T) { + p, _ := newTestPacer() + p.observeResponse(testPolicy, "r=10;t=0") + + // A burst of 10, less the one we hold in reserve. Each slot is taken as it's handed out, so consecutive callers + // work their way through the burst rather than all being waved through. + for i := 0; i < 9; i++ { + require.Zero(t, p.reserveSlot(), "request %d is still within the burst", i+1) + } + + require.Equal(t, emissionInterval, p.reserveSlot()) + require.Equal(t, twoEmissionIntervals, p.reserveSlot()) +} + +func TestPacer_AsTimePasses_TheAllowanceComesBack(t *testing.T) { + p, now := newTestPacer() + p.observeResponse(testPolicy, "r=0;t=10") + require.Equal(t, twoEmissionIntervals, p.reserveSlot()) + + // The slot we just took is ours; waiting it out (and one more interval) puts us back at the front of the queue. + *now += 3 * emissionInterval + + require.Zero(t, p.reserveSlot()) +} + +func TestPacer_WhenAnOlderResponseArrivesLast_DoesNotTalkUsIntoSpeedingUp(t *testing.T) { + p, _ := newTestPacer() + p.observeResponse(testPolicy, "r=1;t=9") + + // Responses can overtake each other, and this one is reporting a bucket that was fuller than the one we already + // know about. Believing it would have us spend quota that has already gone. + p.observeResponse(testPolicy, "r=9;t=1") + + require.Equal(t, emissionInterval, p.reserveSlot()) +} + +func TestPacer_WhenTheServerReportsLessQuotaThanWeExpected_SlowsUsDown(t *testing.T) { + p, _ := newTestPacer() + p.observeResponse(testPolicy, "r=9;t=1") + require.Zero(t, p.reserveSlot()) + + // Something else has been spending the same allowance, so what's left isn't what our own arithmetic said. + p.observeResponse(testPolicy, "r=0;t=10") + + require.Equal(t, twoEmissionIntervals, p.reserveSlot()) +} + +func TestPacer_WithABurstOfOne_LeavesNoRoomToBurstRatherThanGoingBackwards(t *testing.T) { + p, _ := newTestPacer() + + // There is nothing to hold in reserve here, so the burst tolerance would come out negative if we let it. + p.observeResponse("l=1;rpm=60", "r=1;t=0") + require.Zero(t, p.reserveSlot()) + + p.observeResponse("l=1;rpm=60", "r=0;t=1") + require.Equal(t, emissionInterval, p.reserveSlot()) +} + +func TestPacer_WhenThePolicyChanges_StartsAgainOnTheNewOne(t *testing.T) { + p, _ := newTestPacer() + + // Held back by a policy that only allows a request every 10 seconds... + p.observeResponse("l=10;rpm=6", "r=1;t=90") + require.Equal(t, 10*time.Second, p.reserveSlot()) + + // ...which an administrator then loosens. The schedule we'd worked out is in the old policy's units, so holding on + // to it would keep us slow long after the limit that justified it had gone. + p.observeResponse(testPolicy, "r=10;t=0") + + require.Zero(t, p.reserveSlot()) +} + +func TestPacer_WithAPolicyItCannotMakeSenseOf_KeepsOutOfTheWay(t *testing.T) { + p, _ := newTestPacer() + p.observeResponse("l=nonsense;rpm=0", "r=0;t=10") + + require.Zero(t, p.reserveSlot()) +} + +func TestPacer_WhenAResponseCarriesNoRemainingCount_GivesTheSlotBack(t *testing.T) { + p, _ := newTestPacer() + p.observeResponse(testPolicy, "r=10;t=0") + + // Spend the burst, so the next request has to wait for the schedule we've built up. + for i := 0; i < 10; i++ { + p.reserveSlot() + } + + require.Equal(t, twoEmissionIntervals, p.reserveSlot()) + + // Two of those requests turn out to have gone somewhere the server doesn't rate limit, so they cost us nothing and + // the schedule they claimed comes back. Otherwise we'd pace our whole request volume against a limit that only + // governs part of it. + p.observeResponse("", "") + p.observeResponse(testPolicy, "") + + require.Equal(t, emissionInterval, p.reserveSlot()) +} + +func TestPacer_WhenRefundsOutnumberTheSlotsTaken_DoesNotBankCreditForAFutureBurst(t *testing.T) { + p, _ := newTestPacer() + p.observeResponse(testPolicy, "r=10;t=0") + p.reserveSlot() + + // Once the schedule is back level with the clock there's nothing left to give back, however many unlimited + // responses arrive. Otherwise a quiet spell of unlimited requests would buy a burst we never earned. + for i := 0; i < 20; i++ { + p.observeResponse("", "") + } + + // Still the burst the policy allows, and no longer. + for i := 0; i < 9; i++ { + require.Zero(t, p.reserveSlot(), "request %d is still within the burst", i+1) + } + + require.Equal(t, emissionInterval, p.reserveSlot()) +} + +func TestPacer_AfterRefundingTooFar_PutsTheFloorBackUnderTheSchedule(t *testing.T) { + p, _ := newTestPacer() + p.observeResponse(testPolicy, "r=0;t=10") + + // A request we refund may in fact have reached the limiter - a cancelled one, say. Refunding is the safe direction + // to be wrong in, because the next response that does report a count resyncs us. Had these refunds stood, the + // request below would have gone straight out. + p.observeResponse("", "") + p.observeResponse("", "") + p.observeResponse("", "") + p.observeResponse(testPolicy, "r=0;t=10") + + require.Equal(t, twoEmissionIntervals, p.reserveSlot()) +} + +func TestPacer_WhenTheServerStopsRateLimitingUs_StopsPacing(t *testing.T) { + p, _ := newTestPacer() + p.observeResponse(testPolicy, "r=0;t=10") + require.Equal(t, twoEmissionIntervals, p.reserveSlot()) + + // An administrator turned the policy off mid-session, so responses stop carrying the headers. The refunds unwind + // the schedule we'd built, rather than leaving us pacing for ever against a limit that has gone. + for i := 0; i < 11; i++ { + p.observeResponse("", "") + } + + require.Zero(t, p.reserveSlot()) +} + +func TestGetParameter_ReadsTheValue(t *testing.T) { + testCases := []struct { + header string + name string + expected float64 + }{ + {"l=200;rpm=600", "l", 200}, + {"l=200;rpm=600", "rpm", 600}, + {"r=7;t=18", "t", 18}, + {"l=200; rpm=600", "rpm", 600}, + {"rpm=0.5", "rpm", 0.5}, + {"L=200", "l", 200}, + {"l=2e2", "l", 200}, + } + + for _, tc := range testCases { + t.Run(tc.header+"/"+tc.name, func(t *testing.T) { + value, ok := getParameter(tc.header, tc.name) + require.True(t, ok) + require.Equal(t, tc.expected, value) + }) + } +} + +func TestGetParameter_WhenItIsNotThere_SaysSo(t *testing.T) { + testCases := []struct { + header string + name string + }{ + {"", "l"}, + {"l=200", "rpm"}, + {"l=200", ""}, + {"nonsense", "l"}, + {"l=", "l"}, + {"l=abc", "l"}, + // strconv.ParseFloat would read all of these; the server never sends them, so we don't either. + {"l=Inf", "l"}, + {"l=NaN", "l"}, + {"l=0x10", "l"}, + } + + for _, tc := range testCases { + t.Run(tc.header+"/"+tc.name, func(t *testing.T) { + _, ok := getParameter(tc.header, tc.name) + require.False(t, ok) + }) + } +} + +func TestGetParameter_IgnoresParametersItDoesNotKnowAbout(t *testing.T) { + // So the server can add to these headers without older clients falling over. + value, ok := getParameter("l=200;something=new;rpm=600", "rpm") + require.True(t, ok) + require.Equal(t, float64(600), value) +} diff --git a/pkg/ratelimit/retry.go b/pkg/ratelimit/retry.go new file mode 100644 index 00000000..10b1fe25 --- /dev/null +++ b/pkg/ratelimit/retry.go @@ -0,0 +1,87 @@ +package ratelimit + +import ( + "errors" + "math" + "regexp" + "strconv" + "strings" + "time" +) + +// retryAfterHeaderName is the standard header the server sends with an HTTP 429, saying how many seconds to wait. +const retryAfterHeaderName = "Retry-After" + +// jitterFraction is how much random jitter we add on top of a wait, as a fraction of that wait. Everything the limiter +// rejects in a burst is told to wait the same number of seconds, so without this they all come back at the same +// instant, and the ones that are rejected again go on moving in lockstep for the rest of their retries. +const jitterFraction = 0.25 + +// retryDelay works out how long we should wait before retrying, based on the Retry-After value the server sent us and +// how many times we have already retried. Returns false if we shouldn't retry because the server asked us to wait +// longer than we're prepared to. +// +// attempt is 0 for the first retry and counts up from there. The wait doubles with each attempt: the server's +// Retry-After tells us how long until it has capacity for one more request, not how long until it has capacity for +// everything it just rejected, so when a whole batch of requests is rate limited at once, retrying them all at that +// same interval only lets a handful through per round. Backing off gives the limiter time to refill for the whole +// queue instead of just the front of it. +// +// random is a value in [0, 1) that decides how much of the jitter allowance we use. +func retryDelay(retryAfterHeaderValue string, options Options, attempt int, random float64) (time.Duration, bool) { + baseDelay, ok := parseRetryAfterHeader(retryAfterHeaderValue) + if !ok { + baseDelay = options.retryDefaultDelay() + } + + maxDelay := options.retryMaxDelay() + + // A Retry-After of zero means we can go again straight away. + if baseDelay < 0 { + baseDelay = 0 + } + + // The server asked for longer than we are willing to wait, so give up now rather than backing off into it. + if baseDelay > maxDelay { + return 0, false + } + + delay := baseDelay + for i := 0; i < attempt && delay < maxDelay; i++ { + delay *= 2 + } + delay = min(delay, maxDelay) + + // Jitter always lands after the delay, never before it: the server told us the earliest it wants us back. + jitter := min(time.Duration(float64(delay)*jitterFraction), maxDelay-delay) + return delay + time.Duration(random*float64(jitter)), true +} + +var retryAfterPattern = regexp.MustCompile(`^-?\d+$`) + +// parseRetryAfterHeader parses a raw Retry-After header value into a delay, or returns false when the header is +// missing or we can't make sense of it, in which case the caller falls back to the default delay. +// +// Retry-After may also be an HTTP date, but we deliberately don't read it as one, the same as the .NET client and the +// web portal. Converting a date into a delay leans on the local clock, which we don't control and which can be minutes +// out: skew one way would have us retry with no delay at all, exactly the hammering the rate limiter exists to stop, +// and skew the other way would push us past the max delay and silently stop us retrying. The Octopus Server always +// sends a number of seconds, so this only comes up if something in between rewrites it. +func parseRetryAfterHeader(headerValue string) (time.Duration, bool) { + headerValue = strings.TrimSpace(headerValue) + if !retryAfterPattern.MatchString(headerValue) { + return 0, false + } + + // On overflow ParseInt saturates and reports ErrRange; the saturated value is what we want, as either way it's far + // outside anything we would wait for. + seconds, err := strconv.ParseInt(headerValue, 10, 64) + if err != nil && !errors.Is(err, strconv.ErrRange) { + return 0, false + } + + // Clamp before converting, so a huge Retry-After can't overflow time.Duration and wrap around to a short wait. + const maxSeconds = int64(math.MaxInt64 / time.Second) + seconds = min(max(seconds, -maxSeconds), maxSeconds) + return time.Duration(seconds) * time.Second, true +} diff --git a/pkg/ratelimit/retry_test.go b/pkg/ratelimit/retry_test.go new file mode 100644 index 00000000..5da3aca0 --- /dev/null +++ b/pkg/ratelimit/retry_test.go @@ -0,0 +1,125 @@ +package ratelimit + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// Jitter is exercised on its own below; everywhere else we pin it out so the arithmetic is readable. +const noJitter = 0 + +func requireRetryDelay(t *testing.T, expected time.Duration, retryAfter string, options Options, attempt int) { + t.Helper() + delay, ok := retryDelay(retryAfter, options, attempt, noJitter) + require.True(t, ok, "expected to retry after Retry-After %q", retryAfter) + require.Equal(t, expected, delay) +} + +func TestRetryDelay_WaitsForTheNumberOfSecondsInRetryAfter(t *testing.T) { + requireRetryDelay(t, 5*time.Second, "5", Options{}, 0) + requireRetryDelay(t, 5*time.Second, " 5 ", Options{}, 0) +} + +func TestRetryDelay_WaitsTheDefaultDelayWhenTheServerDidNotSendARetryAfter(t *testing.T) { + requireRetryDelay(t, 2*time.Second, "", Options{}, 0) + requireRetryDelay(t, 3*time.Second, "", Options{RetryDefaultDelay: 3 * time.Second}, 0) +} + +func TestRetryDelay_WaitsTheDefaultDelayWhenRetryAfterMakesNoSense(t *testing.T) { + requireRetryDelay(t, 2*time.Second, "soon please", Options{}, 0) +} + +func TestRetryDelay_WaitsTheDefaultDelayForAMalformedDelayRatherThanRetryingImmediately(t *testing.T) { + requireRetryDelay(t, 2*time.Second, "5.5", Options{}, 0) + requireRetryDelay(t, 2*time.Second, "+5", Options{}, 0) + requireRetryDelay(t, 2*time.Second, ".5", Options{}, 0) +} + +func TestRetryDelay_RetriesImmediatelyWhenRetryAfterIsAZeroOrNegativeDelay(t *testing.T) { + requireRetryDelay(t, 0, "0", Options{}, 0) + requireRetryDelay(t, 0, "-3", Options{}, 0) +} + +func TestRetryDelay_WaitsTheDefaultDelayForADateFormRetryAfterRatherThanTrustingTheLocalClock(t *testing.T) { + // We don't read Retry-After as a date. A machine running a few minutes out either way would compute a delay of zero + // and hammer the server, or one past the max delay that silently stops us retrying. + requireRetryDelay(t, 2*time.Second, "Sun, 30 Aug 2026 10:00:07 GMT", Options{}, 0) + requireRetryDelay(t, 2*time.Second, "Sun, 30 Aug 2026 09:59:50 GMT", Options{}, 0) +} + +func TestRetryDelay_GivesUpWhenTheServerAsksUsToWaitLongerThanWeAreWillingTo(t *testing.T) { + _, ok := retryDelay("31", Options{}, 0, noJitter) + require.False(t, ok) + + requireRetryDelay(t, 30*time.Second, "30", Options{}, 0) + + _, ok = retryDelay("11", Options{RetryMaxDelay: 10 * time.Second}, 0, noJitter) + require.False(t, ok) +} + +func TestRetryDelay_GivesUpWhenRetryAfterIsTooBigToRepresent(t *testing.T) { + // Rather than overflowing into a short (or negative) wait. + _, ok := retryDelay("9223372036854775807", Options{}, 0, noJitter) + require.False(t, ok) + + _, ok = retryDelay("99999999999999999999999", Options{}, 0, noJitter) + require.False(t, ok) +} + +func TestRetryDelay_DoublesTheWaitForEachRetryWeHaveAlreadyMade(t *testing.T) { + // A burst that is rate limited all at once is told to wait the same number of seconds by every rejection. Retrying + // on that interval only lets through what the limiter refills in one interval, so we back off to give it time to + // refill for everything it rejected. + requireRetryDelay(t, 2*time.Second, "2", Options{}, 0) + requireRetryDelay(t, 4*time.Second, "2", Options{}, 1) + requireRetryDelay(t, 8*time.Second, "2", Options{}, 2) + requireRetryDelay(t, 16*time.Second, "2", Options{}, 3) +} + +func TestRetryDelay_NeverBacksOffPastTheLongestWaitWeAreWillingToMake(t *testing.T) { + requireRetryDelay(t, 30*time.Second, "2", Options{}, 10) + requireRetryDelay(t, 30*time.Second, "2", Options{}, 1000) +} + +func TestRetryDelay_BacksOffFromTheDefaultDelayWhenTheServerDidNotSendARetryAfter(t *testing.T) { + requireRetryDelay(t, 8*time.Second, "", Options{}, 2) +} + +func TestRetryDelay_StillRetriesImmediatelyAtEveryAttemptWhenRetryAfterIsZero(t *testing.T) { + requireRetryDelay(t, 0, "0", Options{}, 3) +} + +func TestRetryDelay_AddsJitterAfterTheWaitNeverBeforeIt(t *testing.T) { + // The server told us the earliest it wants us back, so jitter only ever makes us later. + delay, ok := retryDelay("2", Options{}, 0, 0.999) + require.True(t, ok) + require.Greater(t, delay, 2*time.Second) + require.LessOrEqual(t, delay, 2500*time.Millisecond) +} + +func TestRetryDelay_KeepsTheJitterInsideTheLongestWaitWeAreWillingToMake(t *testing.T) { + delay, ok := retryDelay("2", Options{}, 10, 0.999) + require.True(t, ok) + require.Equal(t, 30*time.Second, delay) + + delay, ok = retryDelay("28", Options{}, 0, 0.999) + require.True(t, ok) + require.LessOrEqual(t, delay, 30*time.Second) +} + +func TestOptions_ZeroValueGivesTheDefaults(t *testing.T) { + // These are what ships: five retries, two seconds when the server didn't say, and never more than thirty. The same + // as the web portal. + options := Options{} + require.Equal(t, 5, options.retryCount()) + require.Equal(t, 2*time.Second, options.retryDefaultDelay()) + require.Equal(t, 30*time.Second, options.retryMaxDelay()) + require.False(t, options.UseRateLimitHeaders) +} + +func TestOptions_ANegativeRetryCountTurnsRetryingOff(t *testing.T) { + require.Equal(t, 0, Options{RetryCount: -1}.retryCount()) + require.Equal(t, 2, Options{RetryCount: 2}.retryCount()) +} diff --git a/pkg/ratelimit/transport.go b/pkg/ratelimit/transport.go new file mode 100644 index 00000000..440151e2 --- /dev/null +++ b/pkg/ratelimit/transport.go @@ -0,0 +1,156 @@ +package ratelimit + +import ( + "context" + "io" + "math/rand/v2" + "net/http" + "time" +) + +// NewTransport wraps base (or http.DefaultTransport, if base is nil) so that requests going through it are retried +// when the server rejects them with HTTP 429, and optionally paced against the server's advisory rate limit headers. +// +// The Octopus client constructors in pkg/client already do this for you; only use this directly if you're making +// requests with your own http.Client. Don't wrap a transport twice, or retries will multiply. +// +// Every request made through the returned transport shares one view of the rate limit allowance, because they are +// all spending the same one. Requests whose body can't be replayed (a body with no http.Request.GetBody, such as a +// streamed package upload) are still paced, but are sent only once, and an HTTP 429 is passed back to the caller. +func NewTransport(base http.RoundTripper, options Options) http.RoundTripper { + if base == nil { + base = http.DefaultTransport + } + + t := &transport{ + base: base, + options: options, + sleep: sleepContext, + random: rand.Float64, + } + if options.UseRateLimitHeaders { + t.pacer = newPacer() + } + return t +} + +type transport struct { + base http.RoundTripper + options Options + + // pacer is nil when we aren't pacing. + pacer *pacer + + // Replaceable in tests. + sleep func(ctx context.Context, d time.Duration) error + random func() float64 +} + +func (t *transport) RoundTrip(req *http.Request) (*http.Response, error) { + retryCount := t.options.retryCount() + if !canReplay(req) { + retryCount = 0 + } + + attemptRequest := req + for attempt := 0; ; attempt++ { + // Wait our turn if we're going faster than the server's rate limit allows. Retries are paced too: a request + // the limiter has already rejected is the last one that should go straight back at it. + if err := t.waitForPacing(attemptRequest); err != nil { + // A RoundTripper must close the request body, even on error. On a retry, the base transport already + // closed the previous attempt's body and we haven't made this attempt's one yet. + if attempt == 0 && req.Body != nil { + _ = req.Body.Close() + } + return nil, err + } + + response, err := t.base.RoundTrip(attemptRequest) + if err != nil { + return nil, err + } + + if t.pacer != nil { + t.pacer.observeResponse(response.Header.Get(PolicyHeaderName), response.Header.Get(RateLimitHeaderName)) + } + + if response.StatusCode != http.StatusTooManyRequests || attempt >= retryCount { + return response, nil + } + + delay, ok := retryDelay(response.Header.Get(retryAfterHeaderName), t.options, attempt, t.random()) + if !ok { + return response, nil + } + + nextRequest, err := cloneForRetry(req) + if err != nil { + // We can't rebuild the body, so let the caller see the 429 rather than an error of our own making. + return response, nil + } + + discard(response) + + if delay > 0 { + if err := t.sleep(req.Context(), delay); err != nil { + if nextRequest.Body != nil { + _ = nextRequest.Body.Close() + } + return nil, err + } + } + + attemptRequest = nextRequest + } +} + +func (t *transport) waitForPacing(req *http.Request) error { + if t.pacer == nil { + return nil + } + + delay := t.pacer.reserveSlot() + if delay <= 0 { + return nil + } + return t.sleep(req.Context(), delay) +} + +// canReplay reports whether we can send the request again: it either has no body, or it has a GetBody to make a fresh +// copy of it. http.NewRequest sets GetBody for in-memory bodies; anything else is potentially a large or unrewindable +// stream. +func canReplay(req *http.Request) bool { + return req.Body == nil || req.Body == http.NoBody || req.GetBody != nil +} + +func cloneForRetry(req *http.Request) (*http.Request, error) { + clone := req.Clone(req.Context()) + if req.Body != nil && req.Body != http.NoBody { + body, err := req.GetBody() + if err != nil { + return nil, err + } + clone.Body = body + } + return clone, nil +} + +// discard drains and closes a response we're not passing on, so its connection can be reused. +func discard(response *http.Response) { + _, _ = io.Copy(io.Discard, response.Body) + _ = response.Body.Close() +} + +// sleepContext waits for d, or until ctx is done, in which case it returns ctx's error so the caller can give up +// rather than sending a request nobody is waiting for any more. +func sleepContext(ctx context.Context, d time.Duration) error { + timer := time.NewTimer(d) + defer timer.Stop() + + select { + case <-timer.C: + return nil + case <-ctx.Done(): + return ctx.Err() + } +} diff --git a/pkg/ratelimit/transport_test.go b/pkg/ratelimit/transport_test.go new file mode 100644 index 00000000..48485095 --- /dev/null +++ b/pkg/ratelimit/transport_test.go @@ -0,0 +1,290 @@ +package ratelimit + +import ( + "bytes" + "context" + "errors" + "io" + "net/http" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// fakeServer answers requests with canned responses, in order, and records what it was sent. +type fakeServer struct { + responses []*http.Response + requests []*http.Request + bodies []string +} + +func (s *fakeServer) RoundTrip(req *http.Request) (*http.Response, error) { + s.requests = append(s.requests, req) + + body := "" + if req.Body != nil { + b, err := io.ReadAll(req.Body) + if err != nil { + return nil, err + } + _ = req.Body.Close() + body = string(b) + } + s.bodies = append(s.bodies, body) + + if len(s.responses) == 0 { + return nil, errors.New("fakeServer has run out of responses") + } + response := s.responses[0] + s.responses = s.responses[1:] + return response, nil +} + +// trackedBody lets a test see whether a response body was closed. +type trackedBody struct { + io.Reader + closed bool +} + +func (b *trackedBody) Close() error { + b.closed = true + return nil +} + +func response(statusCode int, headers ...string) *http.Response { + r := &http.Response{ + StatusCode: statusCode, + Header: http.Header{}, + Body: &trackedBody{Reader: strings.NewReader("{}")}, + } + for i := 0; i+1 < len(headers); i += 2 { + r.Header.Set(headers[i], headers[i+1]) + } + return r +} + +func tooManyRequests(retryAfter string) *http.Response { + return response(http.StatusTooManyRequests, retryAfterHeaderName, retryAfter) +} + +// newTestTransport returns a transport over server that records how long it was asked to wait, rather than waiting. +func newTestTransport(server *fakeServer, options Options) (*transport, *[]time.Duration) { + sleeps := &[]time.Duration{} + t := NewTransport(server, options).(*transport) + t.random = func() float64 { return 0 } + t.sleep = func(ctx context.Context, d time.Duration) error { + *sleeps = append(*sleeps, d) + return ctx.Err() + } + if t.pacer != nil { + // A clock that never moves, so the pacing delays are exact. + t.pacer.now = func() time.Duration { return time.Hour } + } + return t, sleeps +} + +func get(t *testing.T) *http.Request { + req, err := http.NewRequest(http.MethodGet, "http://octopus.example/api/projects", nil) + require.NoError(t, err) + return req +} + +func TestTransport_RetriesARateLimitedRequestAfterTheRetryAfterDelay(t *testing.T) { + server := &fakeServer{responses: []*http.Response{tooManyRequests("3"), response(http.StatusOK)}} + transport, sleeps := newTestTransport(server, Options{}) + + resp, err := transport.RoundTrip(get(t)) + + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Len(t, server.requests, 2) + require.Equal(t, []time.Duration{3 * time.Second}, *sleeps) +} + +func TestTransport_BacksOffWithEachRetry(t *testing.T) { + server := &fakeServer{responses: []*http.Response{tooManyRequests("1"), tooManyRequests("1"), tooManyRequests("1"), response(http.StatusOK)}} + transport, sleeps := newTestTransport(server, Options{}) + + resp, err := transport.RoundTrip(get(t)) + + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Equal(t, []time.Duration{time.Second, 2 * time.Second, 4 * time.Second}, *sleeps) +} + +func TestTransport_ClosesTheRateLimitedResponsesItDoesNotPassOn(t *testing.T) { + rejected := tooManyRequests("1") + server := &fakeServer{responses: []*http.Response{rejected, response(http.StatusOK)}} + transport, _ := newTestTransport(server, Options{}) + + _, err := transport.RoundTrip(get(t)) + + require.NoError(t, err) + require.True(t, rejected.Body.(*trackedBody).closed) +} + +func TestTransport_PassesTheRateLimitedResponseOnOnceItRunsOutOfRetries(t *testing.T) { + server := &fakeServer{responses: []*http.Response{tooManyRequests("0"), tooManyRequests("0"), tooManyRequests("0")}} + transport, _ := newTestTransport(server, Options{RetryCount: 2}) + + resp, err := transport.RoundTrip(get(t)) + + require.NoError(t, err) + require.Equal(t, http.StatusTooManyRequests, resp.StatusCode) + require.Len(t, server.requests, 3) + require.False(t, resp.Body.(*trackedBody).closed, "the caller gets the response to read") +} + +func TestTransport_DoesNotRetryWhenRetryingIsTurnedOff(t *testing.T) { + server := &fakeServer{responses: []*http.Response{tooManyRequests("0")}} + transport, _ := newTestTransport(server, Options{RetryCount: -1}) + + resp, err := transport.RoundTrip(get(t)) + + require.NoError(t, err) + require.Equal(t, http.StatusTooManyRequests, resp.StatusCode) + require.Len(t, server.requests, 1) +} + +func TestTransport_DoesNotRetryWhenTheServerAsksUsToWaitLongerThanWeAreWillingTo(t *testing.T) { + server := &fakeServer{responses: []*http.Response{tooManyRequests("60")}} + transport, sleeps := newTestTransport(server, Options{}) + + resp, err := transport.RoundTrip(get(t)) + + require.NoError(t, err) + require.Equal(t, http.StatusTooManyRequests, resp.StatusCode) + require.Len(t, server.requests, 1) + require.Empty(t, *sleeps) +} + +func TestTransport_DoesNotRetryOtherErrors(t *testing.T) { + server := &fakeServer{responses: []*http.Response{response(http.StatusServiceUnavailable, retryAfterHeaderName, "1")}} + transport, _ := newTestTransport(server, Options{}) + + resp, err := transport.RoundTrip(get(t)) + + require.NoError(t, err) + require.Equal(t, http.StatusServiceUnavailable, resp.StatusCode) + require.Len(t, server.requests, 1) +} + +func TestTransport_ReplaysTheRequestBodyOnARetry(t *testing.T) { + server := &fakeServer{responses: []*http.Response{tooManyRequests("0"), response(http.StatusCreated)}} + transport, _ := newTestTransport(server, Options{}) + + req, err := http.NewRequest(http.MethodPost, "http://octopus.example/api/projects", bytes.NewBufferString(`{"Name":"x"}`)) + require.NoError(t, err) + req.Header.Set("X-Custom", "kept") + + resp, err := transport.RoundTrip(req) + + require.NoError(t, err) + require.Equal(t, http.StatusCreated, resp.StatusCode) + require.Equal(t, []string{`{"Name":"x"}`, `{"Name":"x"}`}, server.bodies) + require.Equal(t, http.MethodPost, server.requests[1].Method) + require.Equal(t, "kept", server.requests[1].Header.Get("X-Custom")) +} + +func TestTransport_DoesNotRetryARequestWhoseBodyCannotBeReplayed(t *testing.T) { + // A streamed body, like a package upload, can't be rewound, so the request goes once and the caller sees the 429. + server := &fakeServer{responses: []*http.Response{tooManyRequests("0")}} + transport, _ := newTestTransport(server, Options{}) + + req, err := http.NewRequest(http.MethodPost, "http://octopus.example/api/packages/raw", io.NopCloser(strings.NewReader("package"))) + require.NoError(t, err) + require.Nil(t, req.GetBody) + + resp, err := transport.RoundTrip(req) + + require.NoError(t, err) + require.Equal(t, http.StatusTooManyRequests, resp.StatusCode) + require.Len(t, server.requests, 1) +} + +func TestTransport_GivesUpWhenTheRequestIsCancelledWhileWaitingToRetry(t *testing.T) { + server := &fakeServer{responses: []*http.Response{tooManyRequests("10")}} + transport, _ := newTestTransport(server, Options{}) + transport.sleep = sleepContext + + ctx, cancel := context.WithCancel(context.Background()) + req := get(t).WithContext(ctx) + go cancel() + + _, err := transport.RoundTrip(req) + + require.ErrorIs(t, err, context.Canceled) + require.Len(t, server.requests, 1) +} + +func TestTransport_DoesNotPaceByDefault(t *testing.T) { + server := &fakeServer{responses: []*http.Response{ + response(http.StatusOK, PolicyHeaderName, testPolicy, RateLimitHeaderName, "r=0;t=10"), + response(http.StatusOK), + }} + transport, sleeps := newTestTransport(server, Options{}) + + _, err := transport.RoundTrip(get(t)) + require.NoError(t, err) + _, err = transport.RoundTrip(get(t)) + require.NoError(t, err) + + require.Nil(t, transport.pacer) + require.Empty(t, *sleeps) +} + +func TestTransport_PacesRequestsAgainstTheAdvisoryHeadersWhenAskedTo(t *testing.T) { + server := &fakeServer{responses: []*http.Response{ + response(http.StatusOK, PolicyHeaderName, testPolicy, RateLimitHeaderName, "r=0;t=10"), + response(http.StatusOK), + }} + transport, sleeps := newTestTransport(server, Options{UseRateLimitHeaders: true}) + + // The first request goes straight out; nothing has told us about a limit yet. + _, err := transport.RoundTrip(get(t)) + require.NoError(t, err) + require.Empty(t, *sleeps) + + // The server said the allowance is spent, so the next request waits for it to come back. + _, err = transport.RoundTrip(get(t)) + require.NoError(t, err) + require.Equal(t, []time.Duration{twoEmissionIntervals}, *sleeps) +} + +func TestTransport_PacesRetriesTooAndLearnsFromRateLimitedResponses(t *testing.T) { + // The 429 carries the advisory headers as well, so the retry is paced as well as waiting out Retry-After. + server := &fakeServer{responses: []*http.Response{ + response(http.StatusTooManyRequests, retryAfterHeaderName, "1", PolicyHeaderName, testPolicy, RateLimitHeaderName, "r=0;t=10"), + response(http.StatusOK), + }} + transport, sleeps := newTestTransport(server, Options{UseRateLimitHeaders: true}) + + resp, err := transport.RoundTrip(get(t)) + + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Equal(t, []time.Duration{time.Second, twoEmissionIntervals}, *sleeps) +} + +func TestTransport_ClosesTheRequestBodyWhenCancelledBeforeSending(t *testing.T) { + server := &fakeServer{responses: []*http.Response{ + response(http.StatusOK, PolicyHeaderName, testPolicy, RateLimitHeaderName, "r=0;t=10"), + }} + transport, _ := newTestTransport(server, Options{UseRateLimitHeaders: true}) + _, err := transport.RoundTrip(get(t)) + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + body := &trackedBody{Reader: strings.NewReader("x")} + req, err := http.NewRequestWithContext(ctx, http.MethodPost, "http://octopus.example/api/projects", body) + require.NoError(t, err) + + _, err = transport.RoundTrip(req) + + require.ErrorIs(t, err, context.Canceled) + require.True(t, body.closed) + require.Len(t, server.requests, 1, "the cancelled request was never sent") +}