The JSON Error That Hid the Real Failure
This is a submission for DEV's Summer Bug Smash: Smash Stories powered by Sentry. A migration audit should fail loudly when data is lost. Mine failed loudly too-but, in one edge case, it reported the wrong failure. The bug lived in a one-command Python showcase built for an open-source Memanto contribution. The command turns a real GitHub discussion into an Open Knowledge Format bundle, runs Memanto's official migration dry run, round-trips the records through production mapping code, and writes a JSON fidelity receipt. The happy path was straightforward. The uncomfortable path was not. The pipeline and its two kinds of failure The final audit is a subprocess. It can return a non-zero exit status for two very different reasons: - it completed the comparison and found lost or changed records, while still emitting a valid JSON receipt; or - it crashed before it could emit valid JSON. The first case is expected domain behavior. We want to preserve its receipt and then propagate the non-zero status. The second is an operational failure. We want the original CalledProcessError , including its exit code, because that is the evidence that tells a developer what actually went wrong. The original control flow captured the subprocess error, but then parsed its output before re-raising it: try: audit_stdout = _run(audit_command, capture=True).stdout except subprocess.CalledProcessError as error: audit_error = error audit_stdout = error.stdout or "" report = json.loads(audit_stdout) report_path.write_text(json.dumps(report)) if audit_error is not None: raise audit_error That looked reasonable until the audit process failed without producing JSON. json.loads() then raised JSONDecodeError first. The original subprocess exception-and its meaningful return code-never reached the caller. The parser had become an error-mask. Reproducing the hidden branch The useful breakthrough was to stop thinking of the output as simply βvalidβ or βinvalidβ and model two independent signals: - Did the process succeed? - Is its output valid JSON? That creates four cases: | Process result | Output | Correct behavior | |---|---|---| | success | valid JSON | write receipt and continue | | failure | valid JSON | write receipt, then re-raise the process error | | failure | invalid JSON | re-raise the process error; do not create a receipt | | success | invalid JSON | surface JSONDecodeError because the producer broke its contract | The masked failure was the third row. A regression test simulated an audit that exited with status 7 and printed plain text instead of JSON: def fake_run(command, *, capture=False): if capture: raise run_demo.subprocess.CalledProcessError( 7, command, output="audit crashed before producing JSON", ) return run_demo.subprocess.CompletedProcess(command, 0) The test asserts two things that matter operationally: try: run_demo.run_showcase("acme/repo", 7, workdir) except run_demo.subprocess.CalledProcessError as error: assert error.returncode == 7 else: raise AssertionError("Invalid audit output masked the process failure") assert not (workdir / "audit.json").exists() It does not merely expect βan exception.β It verifies that the right exception and exact status code survive, and that invalid output is not blessed as an audit artifact. The fix: preserve causal priority The correction keeps valid failing receipts, but gives the original process failure priority when JSON parsing also fails: try: report = json.loads(audit_stdout) except json.JSONDecodeError: if audit_error is not None: raise audit_error from None raise report_path.write_text( json.dumps(report, indent=2, ensure_ascii=False) + "\n", encoding="utf-8", ) if audit_error is not None: raise audit_error The from None is deliberate. In the simultaneous-failure branch, the JSON error is a side effect of missing output, not the root cause. Suppressing that context keeps the traceback focused on the subprocess failure the operator needs to investigate. At the same time, a valid JSON report from a fidelity failure is still written before the original non-zero result is propagated. That report is valuable evidence; throwing it away would create a different debugging problem. What changed after the fix The branch now has explicit regression coverage for both failure modes: - a failed audit with valid JSON preserves the receipt and exit status; - a failed audit with invalid output preserves the original exit status and writes no receipt. The focused suite reports 25 passing tests, with Ruff checks, formatting, and mypy also clean on the contribution branch. The wider migration demo has additionally completed a 32-record real-data round trip with zero removed or changed portable fields. Those broader results are useful, but the most important outcome of this debugging story is smaller: when the pipeline fails, its diagnostic contract is now trustworthy. Three lessons I am keeping 1. Error handling needs its own truth table Nested fallible operations create combinations that a linear happy-path reading hides. A small matrix exposed the missing branch faster than another end-to-end run would have. 2. Test identity, not just category βAn exception was raisedβ would have passed before and after the fix. Checking returncode == 7 proves that causality survived. 3. Artifacts are part of failure semantics A diagnostic file should exist only when it is valid. The absence assertion prevents future code from leaving behind a misleading audit.json after a producer crash. Evidence The larger PR introduces an OKF portability workflow; this Smash Stories entry is specifically about the real exception-masking bug caught and fixed during its review. The PR is open and mergeable at the time of writing; no merge or bounty award is assumed. AI assistance disclosure: AI tools assisted with code review and drafting. The behavior, fix, tests, and validation claims above are linked to reproducible repository evidence. Top comments (0)
Comments
No comments yet. Start the discussion.