Refactor Error Paths First: A Fault-Injection Table for Messy Repos
Conclusion first: happy-path characterization tests will not save your refactor. The contract that actually breaks is the exception path: error type, message, log line, and rollback state. Capture those four fields in a committed table before you move a single line.
This is the step most teams skip. They snapshot calculate_total() returning 42, then extract a helper, and production starts alerting on KeyError instead of PriceMissingError. The diff looked clean. The logs did not.
Below is a workflow you can run on one module in an afternoon. It assumes you can execute the code locally and that you have a test runner.
1. Scope to one function and one failure surface
Pick a single entry point that already scares you. Not a package. Not a layer. One function that other code calls and that can raise.
Write the scope down, because scope creep kills this work:
Module: billing/pricing.py
Entry: quote(invoice, customer)
Raises: anything reachable in two call hops
Budget: no production edits this session
If the function has more than roughly eight distinct failure modes, split the session. You want a table you can review in one sitting.
2. Enumerate raises mechanically, not from memory
Humans remember two exceptions and forget five. Let the AST tell you what is actually there.
# scripts/list_raises.py — proposed; run from repo root
import ast, pathlib, sys
def raises_in(tree):
found = []
for node in ast.walk(tree):
if isinstance(node, ast.Raise) and isinstance(node.exc, ast.Call):
fn = node.exc.func
name = getattr(fn, "id", getattr(fn, "attr", "?"))
found.append((node.lineno, name))
return found
root = pathlib.Path(sys.argv[1])
for path in sorted(root.rglob("*.py")):
for line, name in raises_in(ast.parse(path.read_text())):
print(f"{path}:{line}:{name}")
Run it on the module and on direct dependencies:
python scripts/list_raises.py billing/pricing.py
python scripts/list_raises.py billing/rates.py
Bare raise statements are re-raises. Follow them up the stack by hand and add those types to the list. Count the rows. Do not estimate.
3. Build the fault table before writing any harness
The table is the artifact. The harness is just plumbing around it.
| # | Trigger | Expected type | Message shape | Logged | Rollback |
|---|---|---|---|---|---|
| 1 | customer id missing | PriceMissingError |
contains customer id | warn | none needed |
| 2 | rate lookup times out | RateUnavailable |
no raw URL | error | order untouched |
| 3 | discount exceeds subtotal | ValueError |
includes both numbers | none | line items restored |
| 4 | currency mismatch | CurrencyMismatch |
names both codes | error | order untouched |
Fill this in by reading code, then correct it by running code. Row 3 above is the interesting one. A ValueError with no log line and a partial mutation is exactly what an extraction silently changes.
Keep messages as shapes, not exact strings. Asserting on full text makes the table brittle and teaches you nothing about intent.
4. Force each fault and record four fields
Now write one parametrized test per row. The harness records a tuple and compares it to a committed JSON baseline.
# tests/test_error_contract.py — proposed; adapt imports to your repo
import json, logging, pathlib
import pytest
from billing import pricing
BASELINE = pathlib.Path("tests/contracts/pricing_errors.json")
def snapshot(exc, records, order):
return {
"type": type(exc).__name__,
"message": strip_ids(str(exc)),
"logged": [r.levelname for r in records],
"order_total": order.total,
"line_count": len(order.lines),
}
def strip_ids(text):
# normalise volatile tokens: ids, timings, paths
return " ".join(t for t in text.split() if not t.isdigit())
@pytest.mark.parametrize("case", ["missing_customer", "rate_timeout",
"discount_overflow", "currency_mismatch"])
def test_error_contract(case, monkeypatch, caplog, order, customer):
caplog.set_level(logging.DEBUG)
break_it(case, monkeypatch, order, customer) # your fault injector
with pytest.raises(Exception) as box:
pricing.quote(order, customer)
got = snapshot(box.value, caplog.records, order)
assert got == BASELINE[case]
The fault injector is plain monkeypatching. Replace the rate client with a stub that raises TimeoutError. Pass a customer object missing id. Do not touch production code to make tests easier.
Run once with --update-baseline semantics you implement yourself, review every line of the resulting JSON, then commit it. Reviewing the baseline is the actual characterization work.
5. Pin the sources of noise
A table that fails at random gets deleted within a week. Kill nondeterminism before you trust a green run.
-
Clock: inject a fixed
now, never calldatetime.now()inside the module. - IDs: strip or stub UUID and request-id formatting.
- Locale: force one locale for number and date formatting.
- Network: make the fault stub raise on any unexpected outbound call.
- Ordering: convert sets to sorted lists before asserting on logs.
Run the suite three times in a row. Three identical results, or the table is not ready.
6. Make the smallest safe change, then re-run
Now refactor. One seam. One commit. Re-run the table after each move.
A practical rule: if a single commit changes more than two rows of the table, the commit is too big. Split it. The table becomes your diff budget for behavior, not just for lines.
Good first moves, in order of risk:
- Rename a local variable. Table must not move.
- Extract a pure helper. Table must not move.
- Move the exception raise into the helper. Message shape must not move.
- Change cleanup order. Table may move — investigate each row.
Step 3 is where most extractions actually break, because the helper loses access to the data used in the message.
7. Where a coding model fits, and where it does not
A model is genuinely useful for two jobs here. Generating candidate fault triggers from the source file, and reading a table diff to guess which refactor caused it. Both are suggestion work, and both need your review before anything is committed.
I draft trigger lists with a model, then delete roughly half of them, because the proposed exception does not exist in the code. Never accept a generated trigger you have not seen fail for real.
Disclosure: This article was prepared as part of MonkeyCode's product outreach.
For that kind of low-stakes, high-volume back-and-forth, the operator describes MonkeyCode as an open-source project with free model access and a free server option for running small harnesses like this one. Availability terms and any free allowance change over time, so check the project page for current limits rather than trusting a number in a blog post. It is a reasonable place to iterate on trigger ideas; it is not a substitute for the reviewed baseline.
Limitations and who should skip this
This workflow has real costs. Be honest about them.
- Swallowed exceptions break it. If the module catches everything and logs a string, you have no contract to pin yet. Fix the logging first.
- Timeout and async paths need a fake clock. Real sleeps make the suite unusable and flaky.
- The table rots. Every intentional behavior change means a baseline update plus review. Budget for it or the file becomes noise.
- It needs an executable environment. Legacy code that only runs in one staging pod is out of reach until you fix that.
Skip this if you are on greenfield code, where designing the error contract is cheaper than recording it. Skip it if you cannot review baseline diffs carefully, since an unreviewed baseline just freezes bugs. Skip it if your module's failures depend on live third-party services you cannot stub.
The takeaway
Return values are the easy half of a refactor. Error type, message shape, logging level, and rollback state are the half that pages you at 3am.
Record those four fields for every reachable failure. Commit the table. Then make one small move and prove the table did not shift.
If you only do one thing from this article, run the AST scan on your scariest module today and count the raises. The number is usually higher than the team expects.
Top comments (0)