--- name: silent-failure-review source: https://app.decimal.ai/s/silent-failure-review@1/SKILL.md source_sha256: 2c35d790d49c --- # Silent Failure Review Reviews error-handling control flow across Python, JavaScript/TypeScript, Go, Rust, and Java for one failure mode: an error is caught (or a failure value is returned) and then discarded, so the program keeps running as if it had succeeded. The code compiles and usually runs; the failure only shows up later as corrupt data, a stuck job, or a bug that is impossible to trace because the original error was thrown away. The base model, handed a `try` that ends in an empty `except` or a `catch` that only logs, reads it as reasonable defensive code and moves on. It knows swallowing errors is bad but does not flag it by default, and rarely names which of the three remedies applies. This skill makes the check mandatory: identify what the swallow hides, then say precisely whether to re-raise, catch a narrower type and actually handle it, or wrap with context and re-raise. This is about control flow that hides failures — not about designing an exception-class hierarchy, and not about working a single reported bug. ## When to Activate Activate when the user asks to: - Review this error handling / try-catch / exception flow - Look over how this function handles failures - Is this `except` / `catch` / `if err != nil` block doing the right thing? - Why does this silently do nothing / return stale data when something fails? - Check whether we are swallowing errors anywhere in this code Do NOT activate for designing an error/exception type hierarchy, for general readability or naming review, or for reproducing and root-causing one specific reported defect. ## The Three Swallow Patterns For each finding, name the pattern, say what failure it hides at runtime, and give the specific remedy (below). Do not stop at "this is bad practice." ### 1. Empty catch — the error is discarded entirely The handler body is empty, a bare `pass`, or throws the value away without acting on it. Every failure in the guarded block vanishes with no trace. - **Python** — `except Exception: pass` (or `except: pass`). Nothing is logged, nothing re-raised; the block silently produces a partial or wrong result. - **JavaScript** — `try { … } catch (e) {}` with an empty body. A rejected await or thrown error is dropped and the code proceeds on undefined state. - **Go** — assigning the error to the blank identifier (`v, _ := doThing()`) or an `if err != nil {}` with an empty body. The failure is thrown away and a zero-value `v` flows on. - **Java** — `catch (Exception e) {}` with nothing inside. - The tell: after the block, the code uses a value that may never have been produced. ### 2. Broad catch, then continue — real bugs get masked A wide catch (`Exception`, `Throwable`, `catch (e)`, `catch (...)`) wraps a block, and control simply falls through — often returning a default, `None`, or an empty list — as if the call had succeeded. This catches not just the expected failure but programming errors (a typo'd attribute, a `KeyError`, a null deref) that should have surfaced loudly. - Returning a default on any exception (`except Exception: return []`) hides an unexpected bug behind an empty result the caller treats as real data. - Catching `Exception` around several statements means an error in *any* of them is treated identically, so you can no longer tell a recoverable failure from a crash. - In Python, a bare `except:` also swallows `KeyboardInterrupt` and `SystemExit`, breaking Ctrl-C and shutdown. ### 3. Log-and-swallow — logged, but the caller is lied to The handler logs the error and then continues or returns a success-looking value. The log is not the same as handling it: the caller has no way to know the operation failed, so it proceeds on incomplete state. This is the subtlest of the three because it *looks* responsible. - `except Exception: logger.error(e); return None` — the caller sees `None` and cannot distinguish "no result" from "it blew up". - A `catch` that logs and then falls through so the function returns its normal success value while the work never happened. - Logging at a debug level a failure that should abort the request — the failure is effectively invisible in production. ## Which Remedy — the Decision Every swallow finding must end with one of these three, chosen deliberately: - **Re-raise (let it propagate).** This layer cannot meaningfully recover, so the caller must learn about the failure. Remove the swallow, or in Python re-raise with a bare `raise` after any cleanup; in Go `return err`; in Rust propagate with `?` instead of `.ok()` / `unwrap_or`. Choose this when there is no sensible local recovery. - **Catch a narrower type and handle it.** Replace the broad catch with the specific exception you can actually recover from (`except TimeoutError`, `catch (err) if err instanceof NotFound`, a typed Go error check) and perform the real recovery — retry, fall back, return a documented default. Choose this when exactly one expected failure has a genuine local response and everything else should still propagate. - **Wrap with context and re-raise.** Catch the low-level error, attach what you were doing (which record, which URL), and re-raise a higher-level error that preserves the original as its cause: Python `raise ProcessingError(f"loading {id}") from e`, JS `throw new Error("...", { cause: e })`, Rust `.with_context(...)` / `.map_err(...)`, Go `fmt.Errorf("...: %w", err)`. Choose this at a boundary where the raw error would be meaningless to the caller but the failure must still stop the flow. ## Legitimate Swallows — Do Not Flag These Swallowing is correct when the failure genuinely has no consequence and the intent is explicit. Do not raise a finding when: - A best-effort cleanup or teardown catches narrowly and comments that failure is intentionally ignored (closing an already-closed socket, deleting a temp file that may be gone). - An optional lookup uses catch/`.get`/`unwrap_or` to supply a documented default, and the default is genuinely valid, not a mask for a bug. - The caught type is specific and the handler fully resolves the situation (retries, returns a real fallback) — a handled error, not a swallowed one. If the code already handles errors correctly, say so plainly instead of inventing a swallow. ## Output Report each finding as: **swallow pattern → what failure it hides at runtime → the remedy (re-raise / narrower catch + handle / wrap with context)**. Name the concrete mechanism for the remedy, not just "handle it better." If nothing is swallowed, say the error handling is sound rather than manufacturing a problem.