Luca Brunner·
Pointed it at one service's error handling and it mapped every swallowed exception to the exact log line we'd never find at 2am
Audits a service's error handling for swallowed exceptions, lying error messages, and failures that will be undiagnosable in production at 2am.
Error Handling Audit: Find the Swallowed Exceptions
You are auditing the error handling of a backend service. The standard for judgment: an on-call engineer at 2am, who did not write this code, has ten minutes to figure out what broke. Every finding is measured against that person.
Code:
{{code}}
Logging stack and conventions: {{logging_setup}}
How errors reach users (API error shape, status codes): {{error_contract}}
Hunt for these patterns, in priority order:
1. **Swallowed exceptions** — catch blocks that neither log, rethrow, nor return an error state. Include catches that log at debug level in a service running at info.
2. **Lying errors** — messages claiming one thing while the failure was another ("invalid input" wrapping a timeout; generic "something went wrong" over a specific, actionable cause).
3. **Context-free logging** — errors logged without the identifiers needed to investigate (request id, user/entity id, the operation's parameters). A stack trace without context is a riddle, not a lead.
4. **Retry traps** — retries without backoff, retries around non-idempotent operations, and retries that hide a persistent failure until the queue explodes.
5. **Async orphans** — promise chains and background tasks whose rejections vanish (fire-and-forget without a catch, missing await, unhandled rejection paths).
For each finding: file:line, pattern, what the 2am engineer sees (often: nothing), and the fixed code snippet using our logging conventions.
Finish with a severity-ordered fix list and one systemic recommendation — the lint rule or wrapper that prevents the whole class, not just these instances.
Ergebnisse
## Findings — notification-service, 14 files reviewed
### 1. Swallowed exception — `dispatcher.ts:142` (CRITICAL)
```ts
try {
await pushProvider.send(payload);
} catch (e) {
// provider flaky, ignore
}
```
**2am view:** nothing. Push notifications silently stop for any user whose token errors; no log, no metric, no dead-letter. The comment documents the decision to be blind.
**Fix:**
```ts
} catch (err) {
log.error({ err, userId, notificationId, provider: "push" }, "push delivery failed");
await deadLetter.enqueue({ payload, reason: "push_send_failed" });
}
```
### 2. Lying error — `templates.ts:77` (HIGH)
All template render failures return `400 "invalid template variables"` — including the case where the template file is missing from the bundle (a deploy problem, not a caller problem).
**2am view:** engineer investigates the caller's payload for an hour; the actual cause is a bad build.
**Fix:** separate `TemplateNotFound` (500, log with template id + bundle hash) from genuine variable validation (400).
### 3. Context-free logging — `worker.ts:203` (HIGH)
`log.error("batch failed", err)` — no batch id, no job type, no item count, and the pino call signature is inverted so `err` lands as an unserialized second arg.
**Fix:** `log.error({ err, batchId, jobType, itemCount }, "notification batch failed")`.
### 4. Retry trap — `email.ts:58` (HIGH)
Three immediate retries, no backoff, around `sendAndRecord()` — which inserts the send record BEFORE the SMTP call. A transient SMTP failure sends up to three duplicate emails ~~and~~ while recording one.
**2am view:** users report duplicate emails; the send log shows exactly one send. Maximum confusion.
**Fix:** make recording follow confirmation, add exponential backoff with jitter, cap at 2 retries, and log each attempt number.
### 5. Async orphan — `hooks.ts:31` (MEDIUM)
`void refreshCache(userId)` fire-and-forget; rejections hit the global unhandledRejection handler which logs without any context.
**Fix:** `.catch((err) => log.warn({ err, userId }, "cache refresh failed"))` — warn is correct, it self-heals on next read.
## Fix order
1 (silent data loss) → 4 (user-visible duplicates) → 2 (misdiagnosis) → 3 (slows every investigation) → 5.
## Systemic recommendation
Adopt a lint rule set: `no-empty-catch` plus a custom rule requiring every `catch` to call the logger or rethrow, and ban bare `void promise` in favor of a `fireAndForget(promise, context)` wrapper that attaches the contextual catch. Instances 1, 3, and 5 become impossible to write, not just easy to find.
Modell: Claude Code
30 Likes17 SavesScore: 20
1 Kommentar
Marco Rossi·
The comment documents the decision to be blind — I felt that one. Provider-flaky-ignore comments are everywhere.