---
title: "Service audit, any cards"
description: "The any context of the service review: 8 cards, each with a FAIL and a PASS example and the organisations that back the rule."
canonical: "https://goodcodereview.com/service-any"
updated: "2026-11-06"
---

# service, any

The stable service cards for the `any` context. The whole level is at https://goodcodereview.com/service.md.

## svc-best-effort-optional-deps

severity: should-fix  
concerns: reliability, observability  
applies_when: The critical path of a request or a job calls a dependency whose data the product can do without, such as enrichment, recommendations, analytics, or a rate-limit control plane.  
Backed by: Netflix (unjudged), Uber (unjudged), Cabify (unjudged), Spotify (supports), Zalando (supports). Backing: mixed.

**Rule.** Give each call to a non-critical dependency a short budget and a default, so its failure degrades the result instead of failing the critical path, and count every fallback.

**Why.** An order page should not fail because the recommendations service is slow. Netflix makes enrichment best-effort, so a slow dependency can degrade its knowledge graph but never block it. Uber's rate-limit clients let traffic through when the control plane is down, because blocking would cause an outage of its own. Cabify drops non-critical details from a driver offer when it cannot collect them in time. It also puts metrics and alerts on every such fallback, because SLO dashboards built for critical flows miss them.

**FAIL**

```python
def trip_offer(trip_id):
    trip = trips.get(trip_id, timeout=0.5)
    eta = traffic.eta(trip.route, timeout=2.0)   # optional detail, but an error here fails the offer
    return Offer(trip, eta)
```

**PASS**

```python
def trip_offer(trip_id):
    trip = trips.get(trip_id, timeout=0.5)
    try:
        eta = traffic.eta(trip.route, timeout=0.1)
    except (TimeoutError, ConnectionError):
        eta = None                                # the offer goes out without the ETA
        degraded.labels(dep="traffic").inc()      # alert on this rate
    return Offer(trip, eta)
```

**Counterpoint.** "A silent default hides an outage." It does when nobody counts it, so the PASS needs the metric and an alert on it. Call a dependency non-critical only when the product works without its data. Authentication, payment, and authorization checks are critical, and they fail closed. Failing open on a rate limiter gives up protection to keep availability, so write that choice into the design. For the timeout itself, see [svc-timeouts](service.md#svc-timeouts). To check that the fallback really runs, see [prod-kill-switch-fallback](production.md#prod-kill-switch-fallback).

## svc-bounded-background-tasks

severity: block  
concerns: reliability, correctness  
applies_when: The code starts a task, goroutine, or thread per item, request, or message.  
Backed by: Google (unjudged), Python Software Foundation (unjudged), Tokio project (unjudged), Cabify (unjudged), AWS (unjudged). Backing: unjudged.

**Rule.** Cap how many background tasks run at once, and never spawn one per input with no limit.

**Why.** Input size is not under your control. One task per item works for ten items and exhausts memory, connections, or the downstream service at ten thousand, and the failure lands on everything else in the process. Go's `errgroup.SetLimit`, asyncio's `Semaphore` and Tokio's `Semaphore` exist for exactly this.

**FAIL**

```python
for msg in batch:                       # batch size comes from the queue
    asyncio.create_task(handle(msg))    # 50k messages -> 50k concurrent handlers
```

**PASS**

```python
async def worker(queue):
    while (msg := await queue.get()) is not None:
        await handle(msg)

queue = asyncio.Queue(maxsize=100)
async with asyncio.TaskGroup() as tg:
    for _ in range(32):                 # at most 32 handlers at once
        tg.create_task(worker(queue))
    for msg in batch:
        await queue.put(msg)
    for _ in range(32):
        await queue.put(None)           # one stop signal per worker
```

**Counterpoint.** "Tasks are cheap." Creating them is, but what each one holds (a socket, a buffer, a database connection) is not. A semaphore around each handler also caps concurrency, though it still creates one task object per input. This card covers work that a service starts for itself. [sys-load-shedding-bounded-queues](system.md#sys-load-shedding-bounded-queues) covers refusing incoming requests under load.

## svc-circuit-breaker-bulkhead

severity: should-fix  
concerns: reliability  
applies_when: A service calls several dependencies from shared pools, or retries a dependency that may be down.  
Backed by: Microsoft (unjudged), AWS (supports), Netflix (supports). Backing: mixed.

**Rule.** Give each critical dependency its own concurrency limit.

**Why.** One shared pool lets a slow dependency use every thread and starve healthy paths. Retrying a dead dependency only adds load, so also fail fast through a breaker with limited half-open probing where retries are demonstrably harmful.

**FAIL**

```text
pool = ThreadPool(50)               # used by billing, search, email alike
breaker = None                      # retries a dead host every request
```

**PASS**

```text
pools = {"billing": asyncio.Semaphore(10), "search": asyncio.Semaphore(20)}
# pseudocode: breaker logs and counts every state change; half-open allows 1 probe
```

**Counterpoint.** "More moving parts than a timeout." True. Start with the timeout and the per-dependency limit, and add the breaker where retries are demonstrably harmful. Retry and breaker have opposite aims, so do not present one as a substitute for the other.

## svc-retry-policy

severity: block  
concerns: reliability  
applies_when: The code retries a failed call, or wraps a client that retries.  
Backed by: Microsoft (unjudged), AWS (unjudged), Google (unjudged), Stripe (unjudged), GitHub (unjudged). Backing: unjudged.

**Rule.** Retry only transient errors, with capped attempts and jittered exponential backoff.

**Why.** Immediate, unbounded, or synchronized retries turn a brief fault into a load spike on the struggling dependency. Retry at one layer only: three layers of three attempts is nine calls per user request. Make retried writes safe with [svc-idempotency-key](service.md#svc-idempotency-key).

**FAIL**

```text
while True:
    try: return client.call()
    except Exception: sleep(1)        # no cap, no jitter, retries 4xx too
```

**PASS**

```python
import random, time
for i in range(3):
    try:
        return client.call()
    except TransientError:
        if i == 2:
            raise
        time.sleep(random.uniform(0, min(CAP_S, BASE_S * 2**i)))
# retry at one layer only: the client's own retries are off
```

**Counterpoint.** "Retries make us more reliable." Only for transient faults and only within a budget. Past that they hurt, so a non-transient error should fail at once. A retry budget (retries capped as a share of traffic) helps further. This card leaves its size to the team.

## svc-stream-dont-materialise

severity: should-fix  
concerns: performance  
applies_when: A handler reads, transforms, or returns a result whose size depends on the data (files, query results, response bodies).  
tool gate: Memory limit in tests with a large fixture.  
Backed by: OpenJS Foundation (unjudged), PostgreSQL (supports), Reactive Streams (supports). Backing: mixed.

**Rule.** Process large results as a stream with backpressure; do not load them whole into memory.

**Why.** A fast source piles data into memory faster than a slow consumer drains it. In the Node docs' test, memory grew from about 88 MB to about 1.5 GB. Memory then scales with data size, not with the code.

**FAIL**

```python
rows = db.execute("SELECT * FROM events").fetchall()   # 50M rows
return json.dumps(rows)
```

**PASS**

```python
def generate():
    for row in db.execute("SELECT * FROM events"):     # server-side cursor
        yield encode(row)
return StreamingResponse(generate())
```

**Counterpoint.** "Streaming complicates errors and retries." Yes: a failure mid-stream cannot change the status code. For small, bounded results, loading the whole result is simpler and fine; state the bound. [sys-load-shedding-bounded-queues](system.md#sys-load-shedding-bounded-queues) covers queue limits.

## svc-timeouts

severity: block  
concerns: performance, reliability  
applies_when: The code calls a remote service over a network.  
Backed by: Python Software Foundation (unjudged), Microsoft (unjudged), Google (unjudged), AWS (unjudged). Backing: unjudged.

**Rule.** Set an explicit timeout on every remote call.

**Why.** A call without a bound holds a thread, a connection, and a caller indefinitely, so one slow dependency becomes an outage. Many clients, gRPC and `requests` among them, set no timeout by default. Where the caller has a deadline of its own, pass the remaining time down: a per-hop timeout that ignores it keeps working for a client that already gave up, and a retry loop shares that one deadline across its attempts.

**FAIL**

```python
resp = requests.get(url)                   # no timeout: can wait forever
reply = stub.GetUser(request)              # gRPC sets no deadline by default
```

**PASS**

```python
client = httpx.Client(timeout=httpx.Timeout(10.0, connect=2.0))   # set once on the client
resp = client.get(url)

def GetProfile(self, request, context):
    # bounded, and no longer than what the caller has left
    return stub.GetUser(request, timeout=min(2.0, context.time_remaining()))
```

**Counterpoint.** "Defaults are fine; the library will time out eventually." Many defaults are infinite, and an eventual timeout longer than the caller's own deadline is wasted work. Configure the client's timeout once at construction, connect and read separately, then shrink it per call to what the caller has left. Pick the numbers from measured latency, such as the downstream p99.9, not folklore.

## svc-update-rollback-on-failure

severity: should-fix  
concerns: reliability  
applies_when: The code replaces its own binary or an installed program with a new version.  
tool gate: Test that makes the final step fail and checks the old version still runs.  
Backed by: Microsoft (unjudged), Google (unjudged), Mender (unjudged). Backing: unjudged. People cited: go-update (Alan Shreve) (unjudged).

**Rule.** When an update fails, restore the previous version instead of leaving a missing or half-written one.

**Why.** Deleting the old binary before the new one is in place leaves nothing to run if the copy fails, and the tool cannot update itself back. On Windows a running executable cannot be deleted but can be renamed, so move it aside first and restore it on failure.

**FAIL**

```go
exe, _ := os.Executable()
os.Remove(exe) // fails on Windows for a running exe; if the next line fails, no binary is left
return os.Rename(newBin, exe)
```

**PASS**

```go
exe, err := os.Executable()
if err != nil {
	return err
}
old := exe + ".old"
if err := os.Rename(exe, old); err != nil {
	return err
}
if err := os.Rename(newBin, exe); err != nil {
	if rerr := os.Rename(old, exe); rerr != nil {
		return fmt.Errorf("update failed (%v) and rollback failed: %w", err, rerr)
	}
	return err
}
os.Remove(old) // best effort; retry at next start if the OS still holds it
return nil
```

**Counterpoint.** "Keeping a second copy costs disk space and clutter." One binary-sized file, removed after success, is cheap next to a tool that cannot start.

## svc-update-verify-provenance

severity: should-fix  
concerns: security  
applies_when: The code downloads and installs or runs a new version of itself or of another program, such as a self-updater or an install script.  
Backed by: Sparkle (unjudged), Sigstore (unjudged), The Update Framework (unjudged), Debian (unjudged), GitHub (unjudged), Google (unjudged). Backing: unjudged. People cited: go-update (Alan Shreve) (unjudged).

**Rule.** Verify a downloaded update against a signature, an attestation or a pinned key, or against a checksum that reached you by another route than the file; a checksum fetched from the same host as the file does not count.

**Why.** An attacker who controls the download host replaces the archive and its checksum file together, so the check passes and the new binary runs with the user's rights. A signature made with a key that is not on that host, or a hash from a different channel, still fails for the attacker.

**FAIL**

```go
archive, _ := get(base + "/tool.tar.gz")
want, _ := get(base + "/tool.tar.gz.sha256") // same host as the archive
got := sha256.Sum256(archive)
if hex.EncodeToString(got[:]) != strings.TrimSpace(string(want)) {
	return errors.New("checksum mismatch")
}
install(archive)
```

**PASS**

```go
var releaseKey = mustDecodeKey("<base64 public key>") // compiled into the binary, never downloaded

archive, _ := get(base + "/tool.tar.gz")
sig, _ := get(base + "/tool.tar.gz.sig")
if !ed25519.Verify(releaseKey, archive, sig) {
	return errors.New("bad signature")
}
install(archive)
```

**Counterpoint.** "HTTPS already protects the download, and signing needs key management." TLS protects the connection to the host, not the files on it once the host or its account is breached. Keyless signing and build attestations remove most of the key handling.