From a52b52bbd80dfd33e0439569b8378ca21c82e5eb Mon Sep 17 00:00:00 2001 From: PIERLUIGI VITI Date: Tue, 18 Aug 2026 14:40:11 +0200 Subject: [PATCH 1/2] feat(http): surface Retry-After, and retry a rate limit when asked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gateway throttles two surfaces (public per-IP, authenticated per-session) and answers 429 with a Retry-After. This SDK dropped that header entirely: "rate limited" arrived with no idea for how long, so a caller could only guess — and rail0-go and rail0-ts both surfaced it. That is the gap; the retry is the policy built on top. ApiError#retry_after carries the header as whole seconds, nil on every other error and nil when it is absent, zero or unparseable. Zero is the case worth naming: it is a valid duration, so treating it as one produces a burst of back-to-back requests against the limiter that just asked for a pause. retry_on_429 (default FALSE) makes the SDK wait it out instead. Off by default deliberately — an automatic sleep hides back-pressure from the process that could react to it, and in a request/response app it turns a rate limit into a stalled page — and self-sufficient by design: it does not need max_retries set as well, because that pairing would have made the flag a silent no-op. The waiting lives in Rail0::Backoff, pure and tested, because two of its decisions are easy to get backwards and invisible once wrong: - jitter is ADDITIVE on a server-instructed wait and multiplicative only on a guess. Textbook full jitter scales the delay by rand(), which is right for a backoff we invented and wrong for a Retry-After: scaling the server's own number down means retrying before the window it named has passed. Jitter is still needed, because callers align — rail0-admin proxies every merchant over ONE session, so they share the bucket, are told the same number, and would wake together. - the cap is not paranoia. The gateway sends its WHOLE period as Retry-After rather than the time left in the window, so a limit hit one second in is asked to wait the full 60. A 429 is retried on any method, POST included, and the comment says why: Rack::Attack rejects in middleware, before the request reaches the application, so nothing ran and nothing can run twice. That is not true of a 502 or a timeout on a capture. Co-Authored-By: Claude Opus 5 --- README.md | 39 ++++++++++++++++- lib/rail0.rb | 1 + lib/rail0/api_error.rb | 15 ++++++- lib/rail0/backoff.rb | 59 +++++++++++++++++++++++++ lib/rail0/client.rb | 10 ++++- lib/rail0/http_client.rb | 18 +++++++- lib/rail0/request.rb | 83 ++++++++++++++++++++++++++++------- spec/backoff_spec.rb | 67 +++++++++++++++++++++++++++++ spec/errors_spec.rb | 93 ++++++++++++++++++++++++++++++++++++++++ 9 files changed, 363 insertions(+), 22 deletions(-) create mode 100644 lib/rail0/backoff.rb create mode 100644 spec/backoff_spec.rb diff --git a/README.md b/README.md index 7bb6ddf..d549ec4 100644 --- a/README.md +++ b/README.md @@ -483,11 +483,48 @@ Rail0::Client.new( timeout: 30, # seconds (default 30) max_retries: 0, # network-error retries (default 0) retry_delay: 0.2, # base delay, doubles each attempt + retry_on_429: false, # retry a rate limit (default false) + retry_after_cap: 60, # longest Retry-After to honour, seconds logger: Rail0::DEFAULT_LOGGER # optional ) ``` -Only network errors and timeouts are retried; HTTP error responses are not. +### Rate limits + +The gateway throttles two surfaces independently: the public, unauthenticated one **per +IP** (100 requests / 60s by default — SIWE nonce + verify, `/payment_methods`, the +catalog reads, `/health`) and everything authenticated **per session**, keyed on the +JWT's subject (300 / 60s). Over budget it answers **429** with `code: "rate_limited"` and +a `Retry-After`. + +`Rail0::ApiError#retry_after` carries that header as whole seconds — nil on every other +error, and nil when the header is absent or unusable. Read it rather than guessing: + +```ruby +begin + client.payments.list +rescue Rail0::ApiError => e + raise unless e.error == "rate_limited" + sleep(e.retry_after || 5) # the gateway's own pacing +end +``` + +Note what the number means: the gateway sends **the whole throttle period**, not the time +left in the current window, so it is an upper bound on the wait rather than a measurement. + +`retry_on_429: true` makes the SDK do that waiting for you — Retry-After, clamped to +`retry_after_cap`, plus a little jitter (see `Rail0::Backoff`; callers sharing one session +are told the same number and would otherwise wake in lockstep). It is **off by default** +on purpose: an automatic sleep hides back-pressure from the process that could react to +it, and in a request/response app it turns a rate limit into a stalled page. Turn it on in +a job — and note it sleeps the **calling thread**. It also works on its own: you do not +need to set `max_retries` as well (that pairing would make the flag a silent no-op). + +Only network errors, timeouts and — when opted in — a 429 are retried; no other HTTP +error is. The 429 is safe to retry on **any** method, `POST` included, because the +gateway rejects it in middleware before the request reaches the application: nothing ran, +so nothing can run twice. That is not true of a 502 or a timeout on a capture, where the +broadcast may already be in flight. ## Project structure diff --git a/lib/rail0.rb b/lib/rail0.rb index 66c777a..60a2b03 100644 --- a/lib/rail0.rb +++ b/lib/rail0.rb @@ -11,6 +11,7 @@ require_relative "rail0/version" require_relative "rail0/error_hints" require_relative "rail0/api_error" +require_relative "rail0/backoff" require_relative "rail0/default_logger" require_relative "rail0/request" require_relative "rail0/http_client" diff --git a/lib/rail0/api_error.rb b/lib/rail0/api_error.rb index 0001d93..e0f70e1 100644 --- a/lib/rail0/api_error.rb +++ b/lib/rail0/api_error.rb @@ -15,18 +15,29 @@ class ApiError < StandardError # @!attribute [r] detail # @return [String, nil] One or two sentences fit to show a user verbatim. Also this # exception's message. - attr_reader :status, :error, :title, :detail + # @!attribute [r] retry_after + # @return [Integer, nil] Seconds the gateway asked the caller to wait, from the + # `Retry-After` header — present on a 429 (`error == "rate_limited"`) and nil + # otherwise. Surfaced because the alternative is a caller guessing: the SDK used + # to drop the header, so "rate limited" arrived with no idea of for how long. + # + # Note it is the WHOLE window the gateway throttles over, not the time left in it + # — the limiter sends its period verbatim — so it is an upper bound on the wait, + # not a measurement. Rail0::Backoff clamps it for that reason. + attr_reader :status, :error, :title, :detail, :retry_after # @param status [Integer] # @param error [String] # @param message [String] The detail; kept positional for compatibility. # @param title [String, nil] - def initialize(status, error, message, title: nil) + # @param retry_after [Integer, nil] + def initialize(status, error, message, title: nil, retry_after: nil) super(message) @status = status @error = error @title = title @detail = message + @retry_after = retry_after freeze end diff --git a/lib/rail0/backoff.rb b/lib/rail0/backoff.rb new file mode 100644 index 0000000..f3b02b9 --- /dev/null +++ b/lib/rail0/backoff.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +module Rail0 + # How long to wait before retrying a request the gateway rate-limited. + # + # Pure, and its own module, because the two interesting decisions here are easy to get + # backwards and impossible to notice once they are wrong — a client that waits too + # little walks straight back into the limiter, and one that waits too long looks hung. + # + # 1. JITTER IS ADDITIVE ON A SERVER-INSTRUCTED WAIT, MULTIPLICATIVE ON A GUESS. + # The textbook "full jitter" multiplies the delay by rand(), which is right for a + # backoff we invented — it spreads a thundering herd — and wrong for a Retry-After: + # scaling the server's own number DOWN means retrying before the window it named has + # passed, which is a second 429 by construction. So an instructed wait is honoured in + # full and a small random tail is ADDED; a guessed one is jittered the usual way. + # + # Why any jitter at all when the server told us the time: because callers align on + # it. rail0-admin proxies every merchant over ONE session, so they share the + # per-session bucket and would all be told the same Retry-After, wake together, and + # recreate the burst the limiter just rejected. + # + # 2. THE CAP IS NOT PARANOIA. The gateway sends the WHOLE period as Retry-After + # (rack_attack.rb: `headers["retry-after"] = match_data[:period].to_s`), not the time + # remaining in the window — so hitting the limit one second in is told to wait the + # full 60. Capping bounds both that over-wait and a hostile or misconfigured value + # from anything between the client and the gateway. + module Backoff + module_function + + # @param retry_after [Integer, Float, nil] the server's Retry-After, in seconds. + # Absent, unparseable, zero or negative all mean "no instruction" — and zero is the + # trap: it is a valid duration, so treating it as one produces a burst of + # back-to-back requests against the very limiter that asked for a pause. + # @param attempt [Integer] 1 for the first retry, 2 for the second, … + # @param base [Float] the exponential backoff's first delay, in seconds. + # @param cap [Float] the longest wait to allow, in seconds. + # @param jitter [Float, nil] randomness in [0,1); injected only by tests. Nil draws it. + # @return [Float] seconds to sleep. + def throttle_delay(retry_after:, attempt:, base:, cap:, jitter: nil) + random = jitter || Kernel.rand + instructed = positive_number(retry_after) + + if instructed + # Honour it in full (clamped), plus a fraction of one base delay so aligned + # callers do not wake in lockstep. + [instructed, cap].min + (random * base) + else + # No instruction: exponential from `base`, full jitter, clamped. + [base * (2**(attempt - 1)) * random, cap].min + end + end + + # @return [Float, nil] the value when it is a positive number, else nil. + def positive_number(value) + number = Float(value, exception: false) + number&.positive? ? number : nil + end + end +end diff --git a/lib/rail0/client.rb b/lib/rail0/client.rb index f4d07b1..eb7cee7 100644 --- a/lib/rail0/client.rb +++ b/lib/rail0/client.rb @@ -58,11 +58,17 @@ class Client # @param logger [#call, nil] Optional logger. Pass Rail0::DEFAULT_LOGGER for built-in output. # @param max_retries [Integer] Extra attempts after a network failure. Default: 0. # @param retry_delay [Numeric] Base delay in seconds between retries (exponential backoff). Default: 0.2. + # @param retry_on_429 [Boolean] Retry a rate-limited request, waiting the gateway's + # Retry-After. Default: false — an automatic sleep hides back-pressure from the + # process that could react to it, and stalls a request/response app. Turn it on in a + # job. It works on its own: no need to set +max_retries+ as well. + # @param retry_after_cap [Numeric] Longest Retry-After to honour, in seconds. Default: 60. def initialize(base_url:, headers: {}, token: nil, timeout: 30, logger: nil, - max_retries: 0, retry_delay: 0.2) + max_retries: 0, retry_delay: 0.2, retry_on_429: false, retry_after_cap: 60) http = HttpClient.new( base_url: base_url, headers: headers, token: token, timeout: timeout, - logger: logger, max_retries: max_retries, retry_delay: retry_delay + logger: logger, max_retries: max_retries, retry_delay: retry_delay, + retry_on_429: retry_on_429, retry_after_cap: retry_after_cap ) @auth = Resources::Auth.new(http) @chains = Resources::Chains.new(http) diff --git a/lib/rail0/http_client.rb b/lib/rail0/http_client.rb index 04dc427..a6c7d7e 100644 --- a/lib/rail0/http_client.rb +++ b/lib/rail0/http_client.rb @@ -6,10 +6,22 @@ module Rail0 # @!visibility private class HttpClient - attr_reader :base_url, :timeout, :logger, :max_retries, :retry_delay + attr_reader :base_url, :timeout, :logger, :max_retries, :retry_delay, + :retry_on_429, :retry_after_cap + # @param retry_on_429 [Boolean] retry a rate-limited request, waiting the gateway's + # Retry-After (see Rail0::Backoff). OFF by default, deliberately: an automatic + # sleep hides back-pressure from the one process that could react to it, and in a + # request/response app it turns a 429 into a stalled page. Turn it on for a job. + # + # It does NOT need `max_retries` to be set as well. That pairing is a footgun — + # the flag would silently do nothing — so on its own it allows one retry. + # @param retry_after_cap [Numeric] longest wait to honour, in seconds. The gateway + # sends its whole throttle period as Retry-After rather than the time left in it, + # so this bounds both the over-wait and any hostile value from in between. def initialize(base_url:, headers: {}, token: nil, timeout: 30, logger: nil, - max_retries: 0, retry_delay: 0.2) + max_retries: 0, retry_delay: 0.2, retry_on_429: false, + retry_after_cap: 60) @base_url = base_url.chomp("/") @static_headers = { "Content-Type" => "application/json" }.merge(headers) @token = token @@ -17,6 +29,8 @@ def initialize(base_url:, headers: {}, token: nil, timeout: 30, logger: nil, @logger = logger || NULL_LOGGER @max_retries = max_retries @retry_delay = retry_delay + @retry_on_429 = retry_on_429 + @retry_after_cap = retry_after_cap freeze end diff --git a/lib/rail0/request.rb b/lib/rail0/request.rb index 96793ab..b875928 100644 --- a/lib/rail0/request.rb +++ b/lib/rail0/request.rb @@ -5,6 +5,7 @@ require "uri" require "forwardable" require_relative "api_error" +require_relative "backoff" require_relative "default_logger" module Rail0 @@ -25,7 +26,8 @@ class Request put: Net::HTTP::Put, patch: Net::HTTP::Patch, delete: Net::HTTP::Delete }.freeze - def_delegators :client, :base_url, :headers, :timeout, :logger, :max_retries, :retry_delay + def_delegators :client, :base_url, :headers, :timeout, :logger, :max_retries, :retry_delay, + :retry_on_429, :retry_after_cap attr_reader :client, :method, :path, :body, :paginated, :extra_headers @@ -46,7 +48,8 @@ def call unless response.is_a?(Net::HTTPSuccess) error_body = parse_error_body(response) api_error = ApiError.new(response.code.to_i, error_code(error_body), - error_message(error_body, response), title: error_body[:title]) + error_message(error_body, response), title: error_body[:title], + retry_after: retry_after_seconds(response)) logger.call(LogEntry.new( method: method.to_s.upcase, url: url, duration_ms: duration_ms, attempt: attempt, request_body: body, status: response.code.to_i, response_body: error_body, error: api_error @@ -65,21 +68,71 @@ def call private + # One loop for the two things worth retrying, which fail in different ways: a network + # error raises, a rate limit comes back as a perfectly good 429 response. + # + # A 429 is the one status this SDK retries, and the reason is not that it is common. + # The gateway rejects it in middleware (Rack::Attack), BEFORE the request reaches the + # application — so nothing was executed, and retrying carries no risk of doing the + # work twice. That is not true of a 502 or a timeout on, say, a capture, where the + # broadcast may already be in flight. Which is why the method does not matter here and + # a POST is retried like a GET. + # + # The sleep is on the CALLING thread. There is no thread pool in this SDK and no + # promise to wait on: a job that turns retry_on_429 on is choosing to block. def with_retries(url) attempt = 1 - start = Process.clock_gettime(Process::CLOCK_MONOTONIC) - result = yield - [result, elapsed_ms(start), attempt] - rescue *ERRORS => e - will_retry = attempt <= max_retries - logger.call(LogEntry.new( - method: method.to_s.upcase, url: url, duration_ms: elapsed_ms(start), attempt: attempt, - request_body: body, error: e, will_retry: will_retry - )) - raise unless will_retry - attempt += 1 - sleep(retry_delay * (2**(attempt - 2))) - retry + loop do + start = Process.clock_gettime(Process::CLOCK_MONOTONIC) + begin + response = yield + rescue *ERRORS => e + will_retry = attempt <= max_retries + logger.call(LogEntry.new( + method: method.to_s.upcase, url: url, duration_ms: elapsed_ms(start), + attempt: attempt, request_body: body, error: e, will_retry: will_retry + )) + raise unless will_retry + + attempt += 1 + sleep(retry_delay * (2**(attempt - 2))) + next + end + + return [response, elapsed_ms(start), attempt] unless retry_throttled?(response, attempt) + + delay = Backoff.throttle_delay( + retry_after: response["retry-after"], attempt: attempt, + base: retry_delay, cap: retry_after_cap + ) + logger.call(LogEntry.new( + method: method.to_s.upcase, url: url, duration_ms: elapsed_ms(start), attempt: attempt, + request_body: body, status: response.code.to_i, + response_body: parse_error_body(response), will_retry: true + )) + attempt += 1 + sleep(delay) + end + end + + # Whether this response is a rate limit the client opted into retrying, and whether + # there is budget left. + # + # `max_retries` is what bounds it, except that its default is 0 — so requiring both + # flags would make retry_on_429 a silent no-op. One retry is the floor when the + # caller asked for the behaviour at all. + def retry_throttled?(response, attempt) + return false unless retry_on_429 && response.code.to_i == 429 + + attempt <= [max_retries, 1].max + end + + # @return [Integer, nil] the Retry-After header as whole seconds, when it is a + # positive number. HTTP-date form is not parsed: the gateway never sends one, and + # guessing at a date would be worse than admitting we have no instruction. + def retry_after_seconds(response) + seconds = Backoff.positive_number(response["retry-after"]) + seconds&.round end def parse_body(response) diff --git a/spec/backoff_spec.rb b/spec/backoff_spec.rb new file mode 100644 index 0000000..08bc22f --- /dev/null +++ b/spec/backoff_spec.rb @@ -0,0 +1,67 @@ +RSpec.describe Rail0::Backoff do + # The two decisions that are easy to get backwards, and invisible once they are: + # a client that waits too little walks back into the limiter, one that waits too long + # looks hung. + + describe ".throttle_delay" do + it "honours a server-instructed wait in full, and only ADDS jitter" do + # Scaling the gateway's own number down (textbook full jitter) means retrying + # before the window it named has passed — a second 429 by construction. + delay = described_class.throttle_delay(retry_after: 30, attempt: 1, base: 0.2, cap: 60, + jitter: 0.5) + expect(delay).to be_within(0.001).of(30.1) + expect(delay).to be > 30 + end + + it "caps an instructed wait, because the gateway sends the whole period" do + # rack_attack.rb sends `period`, not the time remaining, so a limit hit one second + # into the window still asks for the full 60. + delay = described_class.throttle_delay(retry_after: 3600, attempt: 1, base: 0.2, cap: 60, + jitter: 0) + expect(delay).to eq(60) + end + + it "falls back to a jittered exponential backoff when there is no instruction" do + %w[nil empty garbage].zip([nil, "", "soon"]).each do |_label, value| + delay = described_class.throttle_delay(retry_after: value, attempt: 3, base: 0.2, + cap: 60, jitter: 1.0) + # base * 2^(attempt-1) = 0.2 * 4 + expect(delay).to be_within(0.001).of(0.8) + end + end + + it "treats a zero or negative Retry-After as no instruction, never as a duration" do + # Zero IS a valid duration, which is the trap: honouring it produces a burst of + # back-to-back requests against the limiter that just asked for a pause. + [0, "0", -5].each do |value| + delay = described_class.throttle_delay(retry_after: value, attempt: 1, base: 0.5, + cap: 60, jitter: 1.0) + expect(delay).to eq(0.5) + end + end + + it "keeps the exponential path under the cap too" do + delay = described_class.throttle_delay(retry_after: nil, attempt: 12, base: 1, cap: 60, + jitter: 1.0) + expect(delay).to eq(60) + end + + it "draws its own jitter when none is injected" do + delays = Array.new(20) do + described_class.throttle_delay(retry_after: 10, attempt: 1, base: 1, cap: 60) + end + expect(delays.uniq.size).to be > 1 # actually random + expect(delays).to all(be_between(10, 11)) # and always at least the instruction + end + end + + describe ".positive_number" do + it "accepts numbers and numeric strings, rejects everything else" do + expect(described_class.positive_number("30")).to eq(30.0) + expect(described_class.positive_number(1.5)).to eq(1.5) + [nil, "", "abc", 0, "0", -1].each do |value| + expect(described_class.positive_number(value)).to be_nil + end + end + end +end diff --git a/spec/errors_spec.rb b/spec/errors_spec.rb index 3c0b15f..4e92e18 100644 --- a/spec/errors_spec.rb +++ b/spec/errors_spec.rb @@ -115,3 +115,96 @@ def stub_error(status, body) end end end + +RSpec.describe "rate limiting" do + BASE = BASE_URL + + def throttled(retry_after: "60") + headers = { "Content-Type" => "application/json" } + headers["Retry-After"] = retry_after if retry_after + { + status: 429, + body: { code: "rate_limited", title: "Too many requests", + detail: "Rate limit reached. Retry in 60 seconds." }.to_json, + headers: headers + } + end + + describe "the error a 429 raises" do + it "carries retry_after, so a caller is not left guessing" do + # The SDK used to drop the header: "rate limited" arrived with no idea for how long. + stub_request(:get, "#{BASE}/health").to_return(throttled) + client = Rail0::Client.new(base_url: BASE) + + expect { client.health.get }.to raise_error(Rail0::ApiError) do |err| + expect(err.status).to eq(429) + expect(err.error).to eq("rate_limited") + expect(err.retry_after).to eq(60) + end + end + + it "leaves retry_after nil when the header is absent or unusable" do + ["0", "later", nil].each do |value| + stub_request(:get, "#{BASE}/health").to_return(throttled(retry_after: value)) + client = Rail0::Client.new(base_url: BASE) + expect { client.health.get }.to raise_error(Rail0::ApiError) { |e| + expect(e.retry_after).to be_nil + } + end + end + + it "is nil on every other error" do + stub_request(:get, "#{BASE}/health") + .to_return(status: 404, body: { code: "not_found" }.to_json, + headers: { "Content-Type" => "application/json" }) + client = Rail0::Client.new(base_url: BASE) + expect { client.health.get }.to raise_error(Rail0::ApiError) { |e| + expect(e.retry_after).to be_nil + } + end + end + + describe "retry_on_429" do + it "does not retry by default — the 429 is the caller's to handle" do + stub = stub_request(:get, "#{BASE}/health").to_return(throttled) + client = Rail0::Client.new(base_url: BASE) + expect { client.health.get }.to raise_error(Rail0::ApiError) + expect(stub).to have_been_requested.once + end + + it "retries once on its own, without max_retries also being set" do + # The pairing would be a footgun: the flag would silently do nothing. + stub = stub_request(:get, "#{BASE}/health") + .to_return(throttled) + .then.to_return(status: 200, body: { status: "ok" }.to_json, + headers: { "Content-Type" => "application/json" }) + client = Rail0::Client.new(base_url: BASE, retry_on_429: true, retry_delay: 0, + retry_after_cap: 0) + expect(client.health.get[:status]).to eq("ok") + expect(stub).to have_been_requested.twice + end + + it "gives up after the budget and raises the last 429 with its retry_after" do + stub_request(:get, "#{BASE}/health").to_return(throttled) + client = Rail0::Client.new(base_url: BASE, retry_on_429: true, max_retries: 2, + retry_delay: 0, retry_after_cap: 0) + expect { client.health.get }.to raise_error(Rail0::ApiError) { |e| + expect(e.retry_after).to eq(60) + } + expect(a_request(:get, "#{BASE}/health")).to have_been_made.times(3) + end + + it "retries a POST as readily as a GET" do + # Safe specifically because Rack::Attack rejects in middleware, before the request + # reaches the application: nothing ran, so nothing can run twice. Not true of a 502. + stub = stub_request(:post, "#{BASE}/auth/nonces") + .to_return(throttled) + .then.to_return(status: 201, body: { nonce: "abc" }.to_json, + headers: { "Content-Type" => "application/json" }) + client = Rail0::Client.new(base_url: BASE, retry_on_429: true, retry_delay: 0, + retry_after_cap: 0) + expect(client.auth.nonce[:nonce]).to eq("abc") + expect(stub).to have_been_requested.twice + end + end +end From 4944c99f1783ac3bdf491ceee327c8c7b1db1355 Mon Sep 17 00:00:00 2001 From: PIERLUIGI VITI Date: Tue, 18 Aug 2026 15:15:55 +0200 Subject: [PATCH 2/2] refactor(backoff): equal jitter on a guessed wait, not full MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Full jitter multiplies the whole delay by rand(), so it can land arbitrarily close to zero — which makes a real pause indistinguishable from the bug where a Retry-After of "0" is honoured as a duration and the retry fires immediately. rail0-go has a test that measures exactly that, and full jitter broke it. Half fixed, half random spreads the herd just as well and leaves "did we actually wait" observable. Applied in all three SDKs so they do not diverge. Co-Authored-By: Claude Opus 5 --- README.md | 4 +++- lib/rail0/backoff.rb | 23 +++++++++++++++-------- spec/backoff_spec.rb | 15 +++++++++++++++ 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index d549ec4..082d6d5 100644 --- a/README.md +++ b/README.md @@ -514,7 +514,9 @@ left in the current window, so it is an upper bound on the wait rather than a me `retry_on_429: true` makes the SDK do that waiting for you — Retry-After, clamped to `retry_after_cap`, plus a little jitter (see `Rail0::Backoff`; callers sharing one session -are told the same number and would otherwise wake in lockstep). It is **off by default** +are told the same number and would otherwise wake in lockstep). The jitter never shortens +a wait below what it is for: additive on the server's own number, and equal jitter — half +fixed, half random — on a guessed one. It is **off by default** on purpose: an automatic sleep hides back-pressure from the process that could react to it, and in a request/response app it turns a rate limit into a stalled page. Turn it on in a job — and note it sleeps the **calling thread**. It also works on its own: you do not diff --git a/lib/rail0/backoff.rb b/lib/rail0/backoff.rb index f3b02b9..6cebbab 100644 --- a/lib/rail0/backoff.rb +++ b/lib/rail0/backoff.rb @@ -7,12 +7,17 @@ module Rail0 # backwards and impossible to notice once they are wrong — a client that waits too # little walks straight back into the limiter, and one that waits too long looks hung. # - # 1. JITTER IS ADDITIVE ON A SERVER-INSTRUCTED WAIT, MULTIPLICATIVE ON A GUESS. - # The textbook "full jitter" multiplies the delay by rand(), which is right for a - # backoff we invented — it spreads a thundering herd — and wrong for a Retry-After: - # scaling the server's own number DOWN means retrying before the window it named has - # passed, which is a second 429 by construction. So an instructed wait is honoured in - # full and a small random tail is ADDED; a guessed one is jittered the usual way. + # 1. JITTER NEVER SHORTENS THE WAIT BELOW WHAT IT IS FOR. + # On a server-instructed wait it is ADDITIVE: scaling a Retry-After DOWN means retrying + # before the window the server named has passed, which is a second 429 by construction. + # So the instruction is honoured in full and a small random tail is added. + # + # On a guessed wait it is EQUAL jitter — half the delay fixed, half random — not the + # textbook "full jitter" that multiplies the whole delay by rand(). Full jitter can + # land arbitrarily close to zero, which makes a real pause indistinguishable from the + # bug where a Retry-After of "0" is honoured as a duration and the retry fires + # immediately. A floor spreads the herd just as well and leaves "did we actually wait" + # observable. # # Why any jitter at all when the server told us the time: because callers align on # it. rail0-admin proxies every merchant over ONE session, so they share the @@ -45,8 +50,10 @@ def throttle_delay(retry_after:, attempt:, base:, cap:, jitter: nil) # callers do not wake in lockstep. [instructed, cap].min + (random * base) else - # No instruction: exponential from `base`, full jitter, clamped. - [base * (2**(attempt - 1)) * random, cap].min + # No instruction: exponential from `base`, EQUAL jitter (half fixed, half random), + # clamped. + full = base * (2**(attempt - 1)) + [(full / 2.0) + ((full / 2.0) * random), cap].min end end diff --git a/spec/backoff_spec.rb b/spec/backoff_spec.rb index 08bc22f..3c5fc9d 100644 --- a/spec/backoff_spec.rb +++ b/spec/backoff_spec.rb @@ -21,6 +21,21 @@ expect(delay).to eq(60) end + it "keeps a floor of half the delay on a guessed wait" do + # EQUAL jitter, not full: a wait that can land near zero is indistinguishable from + # the bug where a "0" Retry-After is honoured as a duration and the retry fires at + # once. + expect(described_class.throttle_delay(retry_after: nil, attempt: 1, base: 0.2, cap: 60, + jitter: 0)).to be_within(0.0001).of(0.1) + expect(described_class.throttle_delay(retry_after: nil, attempt: 1, base: 0.2, cap: 60, + jitter: 1.0)).to be_within(0.0001).of(0.2) + [0, 0.25, 0.5, 0.99].each do |jitter| + delay = described_class.throttle_delay(retry_after: nil, attempt: 3, base: 0.2, + cap: 60, jitter: jitter) + expect(delay).to be >= 0.4 # half of 0.2 * 2^2 + end + end + it "falls back to a jittered exponential backoff when there is no instruction" do %w[nil empty garbage].zip([nil, "", "soon"]).each do |_label, value| delay = described_class.throttle_delay(retry_after: value, attempt: 3, base: 0.2,