AI Code Review for Error Handling: Swallowed Exceptions, Retry Storms and Logs That Leak

Error handling is the part of a pull request that nobody reads. The happy path gets the attention, the tests cover the happy path, and the catch block at the bottom gets a glance and a nod. Then, months later, a dependency starts returning a new error shape, the catch block swallows it, the retry loop hammers a struggling service, and the one log line that would have explained the outage contains a customer’s session token. Every one of those failures was visible in the diff on the day it was merged.

This post is a checklist of the error-handling mistakes an AI code reviewer should be configured to catch, with the pattern for each, why human reviewers skim past them, and how to phrase them as review instructions. It follows the shape of our posts on migrations and dependency updates, in that most of it applies to any reviewer, and where PURA does something specific we say so.

Why error handling slips through review

Three reasons, and the first is attention. A reviewer has a finite budget of care per PR, and spends it on the logic that does the work. The error path is read as boilerplate because it usually is boilerplate, which is precisely when a subtle change to it goes unnoticed. The second is that error handling is correct by omission as often as by presence: the bug is the catch that is not there, or the error that is not rethrown, and absence does not show up in a diff. The third is that the consequences are delayed. A wrong calculation fails a test today; a swallowed exception fails a customer in March.

A model-based reviewer has none of the attention problem. It reads the catch block with exactly the same care as the function above it, and if told what to look for, it looks every time. That consistency is the whole value, so the checklist below is written to be turned into instructions rather than read once.

The checklist

1. Swallowed exceptions

An empty catch, a catch that only logs at debug level, a .catch(() => {}) on a promise, a try whose except clause is pass. The reviewer should flag any handler that neither rethrows, nor returns a value the caller checks, nor records the failure somewhere a human will see it. The question to ask is “if this throws in production, who finds out?”, and “nobody” is a finding. The common false positive is a deliberate best-effort operation such as a cache warm; the fix is a comment saying so, which the reviewer can be told to accept.

2. Catching too much

catch (e) around a block that can fail in five ways, with one recovery that is only right for one of them. The classic shape is a network call wrapped with the parse of its response: a timeout and a malformed body get the same handling, so a bug in the parser is reported as the upstream being down. The reviewer should ask whether the catch distinguishes the errors it can recover from (retry on a timeout) from the ones it cannot (a programming error in the parser), and whether the second kind is allowed to propagate.

3. Retry storms

A retry loop with no backoff, no jitter, no cap, or a cap that is multiplied by another retry loop one layer up. Three retries at the HTTP client, three at the service wrapper and three in the job runner is twenty-seven requests per failure, from every instance at once, against a service that is already struggling. The reviewer should find every retry in the diff, look for exponential backoff with jitter and an upper bound, and then search the call chain for a second retry wrapping the first. This is where repository-wide context earns its keep: the outer loop is rarely in the same file.

4. Retrying the unretryable

A retry on a non-idempotent operation (a payment, a send, an insert without a key) turns one failure into duplicate side effects. A retry on a 4xx, a validation error or an authentication failure wastes time and hides a bug. The reviewer should check that retried operations are idempotent or carry an idempotency key, and that the retry predicate is a list of transient errors rather than “anything”. The post on false positives covers how to stop the reviewer flagging the retries that are fine.

5. Error messages that leak

Logging the whole request object, the whole exception including its context, or the upstream response body, any of which can contain tokens, passwords, personal data or internal hostnames. Returning a raw exception message to an API caller, which turns a stack trace or a database error into public information. The reviewer should flag any log or response that includes an unstructured dump of a request, a header set, a config object or an exception that wraps one, and ask for explicit fields instead. This overlaps with the security checklist, and it is the item most often caught by a model and most often missed by a person, because the leak is in what the variable contains, not in what the line says.

6. Lost context

Catching an error and throwing a new one without the original attached: throw new Error('failed') with no cause, a Python raise inside except that drops from e, a Go error returned without wrapping. The stack trace that would have located the bug is gone. The reviewer should ask that every rethrow carries the original error and that the new message adds information (what was being attempted, with which identifier) rather than replacing it.

7. Cleanup that only runs on success

A file handle, a lock, a database transaction or a temporary resource released at the end of the happy path and nowhere else. The reviewer should look for every acquire in the diff and check that the release is in a finally, a defer, a context manager or a using declaration, not after the last statement that can throw. Transactions deserve a specific question: on failure, is there an explicit rollback, or does the connection go back to the pool with the transaction open?

8. Partial failure treated as success

A batch operation that returns 200 when three of the fifty items failed; a loop that catches per item and continues, then reports completion. The reviewer should check that partial results are surfaced to the caller with enough detail to act on, and that “done” means all of it. This is also one of the shapes covered in the review for coding agents post, because agent-written code is unusually fond of catching per item and moving on.

9. Timeouts missing or infinite

An HTTP call, a queue read, a lock wait or a subprocess with no timeout at all. Under load the missing timeout turns a slow dependency into a pile of stuck workers. The reviewer should flag any outbound call that does not set a timeout explicitly, and any timeout that is set to a value that would exceed the caller’s own deadline. Where the project has a standard client with defaults, the instruction is to use it rather than the raw library.

10. Error handling that changes behaviour silently

A diff that widens a catch, lowers a log level, changes a rethrow into a return, or removes an error branch, with no mention in the description. Each of those is a behaviour change for every caller, and none of them look like one. The reviewer should call out any change to error-handling semantics in its summary, even when the change looks intentional, so the human approver decides with eyes open.

Turning the checklist into review instructions

The items above work as a block of project instructions for any reviewer that accepts them. A version that has worked for teams using PURA, which reads the repository’s review guidance alongside the diff, looks like this:

• For every catch, except, rescue or error return in the diff, state what happens to the error: rethrown with cause, handled with a named recovery, or dropped. Flag dropped errors unless a comment explains why.
• For every retry, confirm backoff, jitter and a cap, and search for an enclosing retry.
• Flag any log line or response body that includes a request, headers, config, exception context or upstream body as an unstructured value.
• Flag outbound calls without an explicit timeout.
• Flag any change to existing error-handling semantics and describe it in one sentence.

Keep the instructions short and concrete. The custom rules post covers why a reviewer given a page of principles produces worse findings than one given ten checks it can tick, and how to iterate on the wording when the false positives arrive.

What the reviewer still cannot tell you

A model reading the repository can tell you that a retry has no backoff. It cannot tell you that the service being retried falls over at forty requests per second, or that the operations team has an alert on exactly the log line you are about to lower to debug. It will sometimes flag a swallowed error that is swallowed on purpose, because the purpose is in someone’s head rather than in a comment. The honest framing is the same as for every item on this blog: the reviewer guarantees that the ten questions are asked, in full, on every PR, before a human spends attention on it. For the part of the code that nobody reads and everybody eventually regrets, that is a large improvement on the status quo.