DevOps NetworkDevOpsNetwork
Join Free
Practical infra coding

Debug a retry loop that never backs off

mediumDebug and review code

Problem statement

During an outage of the artifact store, every CI runner hammered it with retries at a fixed one-second rhythm, and some jobs then "succeeded" with an empty artifact. The retry helper below is the suspect. Review it, list every bug, and write a fixed version.

retry.py (broken)

Python
import time
def fetch_with_retry(fetch, retries=3, delay=1):
for attempt in range(retries):
try:
return fetch()
except Exception as e:
print(f" attempt {attempt} failed: {e}")
time.sleep(delay)

To test it without waiting, a harness replaces time.sleep with a function that records each delay, and calls the helper with a fake fetch that replays a scripted list of outcomes:

  • always down: four ConnectionErrors, retries=4
  • bad request: four ValueError("400 bad token")s, retries=4 (a permanent error)
  • recovers: one TimeoutError, then "OK"

Symptom (actual output of the broken version):

TEXT
attempt 0 failed: refused
attempt 1 failed: refused
attempt 2 failed: refused
attempt 3 failed: refused
always down: returned None, slept [1, 1, 1, 1]
attempt 0 failed: 400 bad token
attempt 1 failed: 400 bad token
attempt 2 failed: 400 bad token
attempt 3 failed: 400 bad token
bad request: returned None, slept [1, 1, 1, 1]
attempt 0 failed: slow
recovers: returned 'OK', slept [1]

Examples

Example 1

Input: The same harness with the fixed `fetch_with_retry`

Output: attempt 1/4 failed: refused attempt 2/4 failed: refused attempt 3/4 failed: refused attempt 4/4 failed: refused always down: raised RetryError(gave up after 4 attempts: refused), slept [1, 2, 4] bad request: raised ValueError(400 bad token), slept [] attempt 1/4 failed: slow recovers: returned 'OK', slept [1]

Explanation: Delays double, there is no sleep after the final attempt, the permanent error is raised at once, and giving up is an exception instead of a silent None.

Hints

Approach

Optimal

The broken helper has five problems:

  1. No backoff. delay is never changed, so every runner retries every second: slept [1, 1, 1, 1]. During an outage this keeps load on the failing service constant instead of backing off. Fix: delay *= 2 after each sleep, capped by max_delay.
  2. Sleeps after the last attempt. The loop sleeps even when there is no next attempt, wasting time before failing. Fix: break when attempt == retries.
  3. Silent failure. When all attempts fail, the loop ends and the function falls off the end, returning None. The caller treats None as a result, which is how jobs "succeeded" with an empty artifact. Fix: raise a RetryError that names the attempt count and chains the last exception with from last, so the traceback shows the real cause.
  4. Retries everything. except Exception retries a ValueError for a bad token four times, and would also retry programming errors like KeyError. Fix: catch only transient errors (ConnectionError, TimeoutError); anything else propagates immediately, as bad request now shows with slept [].
  5. Misleading log. Attempts are numbered from 0 and there is no total, so attempt 3 failed does not say whether more tries are coming. Fix: count from 1 and print 3/4.

The harness is part of the answer: replacing time.sleep with slept.append lets the test assert on the exact delays without waiting for them, and the scripted flaky function makes every scenario reproducible.

Adding random jitter to each delay is the usual next step; it is left out here so the delays in the output are exact.

ComplexityTime O(retries)Space O(1)
Python
import time
class RetryError(Exception):
pass
TRANSIENT = (ConnectionError, TimeoutError)
def fetch_with_retry(fetch, retries=3, delay=1, max_delay=30):
last = None
for attempt in range(1, retries + 1):
try:
return fetch()
except TRANSIENT as e: # only retry errors that can go away
last = e
print(f" attempt {attempt}/{retries} failed: {e}")
if attempt == retries:
break # no pointless sleep after the last try
time.sleep(min(delay, max_delay))
delay *= 2 # the missing backoff
raise RetryError(f"gave up after {retries} attempts: {last}") from last
# ---- test harness: fake sleep and a scripted flaky call ----
import time
slept = []
time.sleep = slept.append # record delays instead of waiting
def flaky(outcomes):
outcomes = list(outcomes)
def call():
o = outcomes.pop(0)
if isinstance(o, Exception):
raise o
return o
return call
def run(name, fn):
slept.clear()
try:
result = fn()
print(f"{name}: returned {result!r}, slept {slept}")
except Exception as e:
print(f"{name}: raised {type(e).__name__}({e}), slept {slept}")
run("always down", lambda: fetch_with_retry(flaky([ConnectionError("refused")] * 4), retries=4))
run("bad request", lambda: fetch_with_retry(flaky([ValueError("400 bad token")] * 4), retries=4))
run("recovers", lambda: fetch_with_retry(flaky([TimeoutError("slow"), "OK"]), retries=4))

Follow-up questions

  • Add full jitter and explain why it helps when hundreds of runners fail at once.
  • Make the set of retryable exceptions a parameter, and retry on HTTP 503 but not 404.
  • Add an overall deadline so the helper never runs longer than 60 seconds in total.

Frequently asked questions

Retry helpers are copy-pasted all over automation code, and bad ones make outages worse: synchronized fixed-interval retries from many clients keep a recovering service overloaded. Reviewers are expected to spot the missing backoff, the swallowed failure and the over-broad except.

The caller cannot tell "gave up" from "got an empty result". Code like data = fetch_with_retry(get_artifact) then writes None or an empty file and reports success. Failing loudly is always better than returning a value that looks valid.

It keeps the original exception as __cause__, so the traceback shows both the RetryError and the underlying ConnectionError with its own stack. Without it, you lose the details of what actually failed.