From 6ab19ac533101843f08fdc20128aea5e77fdc2a9 Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Tue, 18 Aug 2026 21:39:27 +0500 Subject: [PATCH] fix(utils): narrow bare except Exception in merge_json_files merge_json_files's read of the existing JSON file caught bare `Exception` around `json5.load`, so a real bug there (e.g. a `TypeError`/`AttributeError`) was silently treated the same as a normal parse failure -- `None` returned, existing settings preserved untouched, nothing surfaced unless `verbose`. Only `OSError` (inaccessible file) and `ValueError` (malformed JSON5 -- json5's decode error is a `ValueError` subclass) are expected outcomes here; anything else should propagate. Same bug, same fix shape, as the caller `handle_vscode_settings`, whose own bare `except Exception` was just narrowed to `(OSError, ValueError, KeyError)` in commit 16f4577 (PR #3844) with the same rationale ("let programming errors like TypeError or AttributeError propagate"). That PR's own regression test monkeypatched `merge_json_files` to prove the caller's narrowing works; this fixes and tests the callee itself, which still had the original bare-except bug. Co-Authored-By: Claude Sonnet 5 --- src/specify_cli/_utils.py | 2 +- tests/test_merge.py | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/_utils.py b/src/specify_cli/_utils.py index f2364f6d43..0562ea0142 100644 --- a/src/specify_cli/_utils.py +++ b/src/specify_cli/_utils.py @@ -249,7 +249,7 @@ def merge_json_files(existing_path: Path, new_content: Any, verbose: bool = Fals except FileNotFoundError: # Handle race condition where file is deleted after exists() check exists = False - except Exception as e: + except (OSError, ValueError) as e: if verbose: console.print(f"[yellow]Warning: Could not read or parse existing JSON in {existing_path.name} ({e}).[/yellow]") # Skip merge to preserve existing file if unparseable or inaccessible (e.g. PermissionError) diff --git a/tests/test_merge.py b/tests/test_merge.py index 6b1eb1c2fc..45889ffdd7 100644 --- a/tests/test_merge.py +++ b/tests/test_merge.py @@ -212,3 +212,29 @@ def test_handle_vscode_settings_propagates_programming_errors(tmp_path): ) finally: utils_mod.merge_json_files = original_merge + + +def test_merge_json_files_propagates_programming_errors(tmp_path, monkeypatch): + """Unexpected programming errors reading the existing file must propagate. + + ``merge_json_files``'s own read of the existing JSON file caught bare + ``Exception`` around ``json5.load``, so a real bug there (e.g. a + ``TypeError``) was silently treated the same as a normal parse failure -- + ``None`` returned, existing settings preserved, nothing logged unless + ``verbose``. Only ``OSError`` (inaccessible file) and ``ValueError`` + (malformed JSON5 -- json5's decode error is a ``ValueError`` subclass) + are expected outcomes here; anything else must propagate, matching the + narrowing already applied to the caller, ``handle_vscode_settings``. + """ + existing_file = tmp_path / "settings.json" + existing_file.write_text('{"a": 1}\n', encoding="utf-8") + + import specify_cli._utils as utils_mod + + def _boom(*_a, **_kw): + raise TypeError("boom") + + monkeypatch.setattr(utils_mod.json5, "load", _boom) + + with pytest.raises(TypeError): + merge_json_files(existing_file, {"b": 2})