A retry helper in a pull request is short, passes its tests and looks responsible, so it tends to get approved without much reading. This post is for whoever is reviewing one. You get five checks to run against the diff, a runnable Python example where one timed-out request charges a customer twice, and a fixed version that charges once.
The code uses Python 3.13, requests 2.34 and urllib3 2.8. The "payments API" is a local stub, so nothing else is needed.
The helper as it shows up in review
This shape turns up constantly, whether a person or a coding agent wrote it:
# charge_broken.py: the retry helper as it often shows up in a pull request
import sys, time
import requests
API = "http://127.0.0.1:8099/charges"
def post_with_retry(url, payload, attempts=5):
for attempt in range(1, attempts + 1):
try:
r = requests.post(url, json=payload, timeout=1)
r.raise_for_status()
return r.json()
except Exception as e:
print(f"attempt {attempt} failed: {type(e).__name__}")
time.sleep(0.5)
raise RuntimeError("gave up")
order, amount = sys.argv[1], int(sys.argv[2])
try:
print(post_with_retry(API, {"order_id": order, "amount": amount}))
except RuntimeError as e:
print(e)
print(requests.get(f"{API}/{order}", timeout=2).json())
It looks careful. The unit tests for helpers like this usually mock requests.post to fail once and then succeed, so they pass, and none of the problems below show up.
Reproduce it
The stub behaves like a payments API on a bad day. It commits the charge, then takes 1.5 seconds to answer the first request for each order. It honours an Idempotency-Key header the way Stripe's API does, by returning the saved result when it sees the same key again.
# fake_payments.py: a tiny payments API that commits the charge, then answers slowly
import json, time, uuid
from http.server import ThreadingHTTPServer, BaseHTTPRequestHandler
charges = [] # every charge ever created
by_key = {} # Idempotency-Key -> saved response body
seen_orders = set()
class Handler(BaseHTTPRequestHandler):
def _send(self, code, body):
data = json.dumps(body).encode()
self.send_response(code)
self.send_header("Content-Type", "application/json")
self.send_header("Content-Length", str(len(data)))
self.end_headers()
try:
self.wfile.write(data)
except BrokenPipeError:
pass # client already gave up
def do_POST(self):
body = json.loads(self.rfile.read(int(self.headers["Content-Length"])))
if body.get("amount", 0) <= 0:
return self._send(400, {"error": "amount must be positive"})
key = self.headers.get("Idempotency-Key")
if key in by_key:
return self._send(201, by_key[key])
charge = {"id": "ch_" + uuid.uuid4().hex[:8], "order_id": body["order_id"]}
charges.append(charge) # the write is committed here
if key:
by_key[key] = charge
if body["order_id"] not in seen_orders:
seen_orders.add(body["order_id"])
time.sleep(1.5) # first call per order is slow
self._send(201, charge)
def do_GET(self):
order = self.path.rsplit("/", 1)[-1]
self._send(200, {"order_id": order,
"charges": sum(c["order_id"] == order for c in charges)})
def log_message(self, *args):
pass
if __name__ == "__main__":
print("listening on http://127.0.0.1:8099")
ThreadingHTTPServer(("127.0.0.1", 8099), Handler).serve_forever()
Start the stub in one terminal and run the client twice in another, once with a valid amount and once with an invalid one:
pip install "requests>=2.32"
python fake_payments.py
# second terminal
python charge_broken.py ord_1001 4200
python charge_broken.py ord_1002 0
attempt 1 failed: ReadTimeout
{'id': 'ch_8c1bb8f3', 'order_id': 'ord_1001'}
{'order_id': 'ord_1001', 'charges': 2}
attempt 1 failed: HTTPError
attempt 2 failed: HTTPError
attempt 3 failed: HTTPError
attempt 4 failed: HTTPError
attempt 5 failed: HTTPError
gave up
{'order_id': 'ord_1002', 'charges': 0}
Two charges for one order. The client stopped waiting after one second, but the server had already written the charge, and the retry wrote a second one. The other run sent the same invalid request five times and took more than two seconds to report a 400 it already had after the first attempt.
Check 1: is the retried call safe to repeat?
A timeout tells you the client stopped waiting. It says nothing about whether the server finished. For a GET that's fine. For a POST that moves money, sends an email or enqueues a job, a retry without an idempotency key can run the side effect twice.
AWS says it directly in its Builders' Library article Timeouts, retries, and backoff with jitter: "APIs with side effects aren't safe to retry unless they provide idempotency."
So ask what this endpoint does when it receives the same request twice. If it creates two of something and the PR sends no idempotency key, the retry shouldn't merge as written.
The key has to be created once per logical operation, outside the loop. A uuid4() inside the loop gives every attempt a new key, which is the same as sending none. Stripe's idempotent requests docs suggest V4 UUIDs and say v1 keys can be pruned once they are at least 24 hours old, so a job that replays a failed charge the next day can create a fresh one.
Check 2: which errors get retried?
except Exception retries everything. A 400 for bad input, a 401 for an expired token, a KeyError while parsing the response and a plain bug in the caller all get five attempts, and none of them improve on attempt two.
Retry what is transient: connection errors, read timeouts, and the statuses that mean try later (429, 502, 503, 504). Everything else should raise on the first attempt so the caller sees the real error quickly.
If the PR uses urllib3's Retry through an HTTPAdapter, read the allowed_methods argument. The default set leaves out POST. In urllib3 2.8 it is DELETE, GET, HEAD, OPTIONS, PUT and TRACE. A diff that sets allowed_methods=None or adds "POST" to get a flaky test passing has switched on the check 1 problem for every POST through that session.
Check 3: how long between attempts, and does it read Retry-After?
A fixed time.sleep(0.5) means every client that failed at the same moment retries at the same moment. If the dependency is struggling because of load, that is a synchronized second wave. Exponential backoff with jitter spreads them out. The full jitter form picks a random delay between zero and the exponential cap.
When a 429 or 503 comes with a Retry-After header, use it. It's the only information you have about when the server expects to recover.
Check 4: what's the worst-case wall time?
Look for a hard cap on attempts, then add up the timeouts and delays. The fixed helper below allows four attempts with a 3.05 second connect timeout, a 1 second read timeout and delays capped at 5 seconds. That's about 7 seconds if every read times out and about 15 if every connect does. Inside a request handler behind a 10 second gateway timeout, the user gets a 504 while the retries carry on behind it.
Check 5: does something above this already retry?
Retries multiply across layers. The same AWS article walks through a five-layer stack where each layer retries three times and says the database load "will increase 243x". An SDK, a service mesh, a queue consumer's redelivery and this new helper can all be retrying one call. Ask where else retries happen and keep them in one place.
The fixed helper
# charge_fixed.py: same call, with the review fixes applied
import random, sys, time, uuid
import requests
API = "http://127.0.0.1:8099/charges"
RETRYABLE_STATUS = {429, 502, 503, 504}
def post_with_retry(url, payload, attempts=4, base=0.2, cap=5.0):
key = str(uuid.uuid4()) # one key for every attempt of this charge
for attempt in range(1, attempts + 1):
try:
r = requests.post(url, json=payload, timeout=(3.05, 1),
headers={"Idempotency-Key": key})
except (requests.ConnectionError, requests.Timeout) as e:
print(f"attempt {attempt} failed: {type(e).__name__}")
if attempt == attempts:
raise
delay = random.uniform(0, min(cap, base * 2 ** attempt))
else:
if r.status_code < 400:
return r.json()
if r.status_code not in RETRYABLE_STATUS or attempt == attempts:
r.raise_for_status() # a 400 will not succeed on attempt 2
retry_after = r.headers.get("Retry-After", "")
# Retry-After can also be an HTTP date; this sketch only reads seconds
delay = (float(retry_after) if retry_after.isdigit()
else random.uniform(0, min(cap, base * 2 ** attempt)))
time.sleep(delay)
order, amount = sys.argv[1], int(sys.argv[2])
try:
print(post_with_retry(API, {"order_id": order, "amount": amount}))
except requests.HTTPError as e:
print(f"not retried: {e.response.status_code} {e.response.json()['error']}")
print(requests.get(f"{API}/{order}", timeout=2).json())
python charge_fixed.py ord_1003 4200
python charge_fixed.py ord_1004 0
attempt 1 failed: ReadTimeout
{'id': 'ch_6a565768', 'order_id': 'ord_1003'}
{'order_id': 'ord_1003', 'charges': 1}
not retried: 400 amount must be positive
{'order_id': 'ord_1004', 'charges': 0}
The first attempt still timed out. The second carried the same key, so the stub returned the saved charge instead of writing another, and the 400 came back on the first try.
This version has limits worth raising in the same review. It reads Retry-After only as seconds, not the HTTP date form. It assumes the server implements idempotency keys, and plenty of internal APIs don't, in which case the fix belongs on the server first. It also emits no metric, so a dependency that now needs two attempts per call looks healthy from the outside until the retries run out.
For what stacked retries do across services, there's a longer write-up on retry storms and cascading failure before merge.
Top comments (0)