TIER-2 READ conductor/code_styleguides/error_handling.md end-to-end before Phase 5: refactor(gui_2): migrate L1367 _apply_pending_patch to Result[T] (Phase 5)
Extract _apply_pending_patch_result helper from the apply_patch_to_file try/except in App._apply_pending_patch. Legacy wrapper drains errors to app._last_request_errors per FR-BC-4 event-handler pattern. [pre-audit] L1367 INTERNAL_BROAD_CATCH [post-audit] V count: 12 -> 11 (L1367 removed)
This commit is contained in:
+47
-14
@@ -1350,20 +1350,10 @@ class App:
|
||||
if not self._pending_patch_text:
|
||||
self._patch_error_message = "No patch to apply"
|
||||
return
|
||||
#TODO(Ed): Exception(Review)
|
||||
try:
|
||||
base_dir = str(self.controller.current_project_dir) if hasattr(self.controller, 'current_project_dir') else "."
|
||||
success, msg = apply_patch_to_file(self._pending_patch_text, base_dir)
|
||||
if success:
|
||||
self._show_patch_modal = False
|
||||
self._pending_patch_text = None
|
||||
self._pending_patch_files = []
|
||||
self._patch_error_message = None
|
||||
imgui.close_current_popup()
|
||||
else:
|
||||
self._patch_error_message = msg
|
||||
except Exception as e:
|
||||
self._patch_error_message = str(e)
|
||||
patch_result = _apply_pending_patch_result(self)
|
||||
if not patch_result.ok:
|
||||
if not hasattr(self, '_last_request_errors'): self._last_request_errors = []
|
||||
self._last_request_errors.append(("_apply_pending_patch", patch_result.errors[0]))
|
||||
|
||||
def _open_patch_in_external_editor(self) -> None:
|
||||
self._external_editor_clicked = True
|
||||
@@ -7785,6 +7775,49 @@ def _populate_auto_slices_file_read_result(app: "App", f_item) -> Result[str]:
|
||||
original=e,
|
||||
)])
|
||||
|
||||
|
||||
def _apply_pending_patch_result(app: "App") -> Result[bool]:
|
||||
"""Drain-aware variant of L1367 _apply_pending_patch.
|
||||
|
||||
Extracts the apply_patch_to_file try/except from App._apply_pending_patch
|
||||
into a Result-returning helper. On success (apply_patch_to_file returns
|
||||
(True, msg)), sets app._show_patch_modal=False, app._pending_patch_text=None,
|
||||
app._pending_patch_files=[], app._patch_error_message=None and returns
|
||||
Result(data=True). On failure (apply_patch_to_file returns (False, msg)
|
||||
or raises), sets app._patch_error_message to the error string and returns
|
||||
Result(data=False, errors=[ErrorInfo]).
|
||||
|
||||
The legacy wrapper drains errors to app._last_request_errors (per FR-BC-4
|
||||
event-handler drain pattern; data plane attribute).
|
||||
|
||||
[C: src/gui_2.py:App._apply_pending_patch (L1367 legacy wrapper)]
|
||||
"""
|
||||
try:
|
||||
base_dir = str(app.controller.current_project_dir) if hasattr(app.controller, 'current_project_dir') else "."
|
||||
success, msg = apply_patch_to_file(app._pending_patch_text, base_dir)
|
||||
if success:
|
||||
app._show_patch_modal = False
|
||||
app._pending_patch_text = None
|
||||
app._pending_patch_files = []
|
||||
app._patch_error_message = None
|
||||
imgui.close_current_popup()
|
||||
return Result(data=True)
|
||||
else:
|
||||
app._patch_error_message = msg
|
||||
return Result(data=False, errors=[ErrorInfo(
|
||||
kind=ErrorKind.INTERNAL,
|
||||
message=f"apply_patch_to_file returned False: {msg}",
|
||||
source="gui_2._apply_pending_patch_result",
|
||||
)])
|
||||
except Exception as e:
|
||||
app._patch_error_message = str(e)
|
||||
return Result(data=False, errors=[ErrorInfo(
|
||||
kind=ErrorKind.INTERNAL,
|
||||
message=f"Apply pending patch failed: {e}",
|
||||
source="gui_2._apply_pending_patch_result",
|
||||
original=e,
|
||||
)])
|
||||
|
||||
#endregion: Phase 5 Event Handler Result Helpers
|
||||
|
||||
#endregion: MMA
|
||||
|
||||
@@ -878,3 +878,48 @@ def test_phase_5_l1293_populate_auto_slices_file_read_result_failure():
|
||||
err = result.errors[0]
|
||||
assert err.source == "gui_2._populate_auto_slices_file_read_result"
|
||||
assert "no such file" in err.message
|
||||
|
||||
|
||||
def test_phase_5_l1367_apply_pending_patch_result_success():
|
||||
"""
|
||||
L1367 _apply_pending_patch_result returns Result.ok=True on success.
|
||||
|
||||
The helper wraps the apply_patch_to_file try/except in App._apply_pending_patch.
|
||||
On success (apply_patch_to_file returns (True, msg)), returns Result(data=True)
|
||||
and sets app._show_patch_modal=False, app._pending_patch_text=None,
|
||||
app._pending_patch_files=[], app._patch_error_message=None.
|
||||
"""
|
||||
from src import gui_2
|
||||
from unittest.mock import MagicMock, patch
|
||||
app = MagicMock()
|
||||
app._pending_patch_text = "--- a/foo.py\n+++ b/foo.py\n@@ -1 +1 @@\n-old\n+new\n"
|
||||
app.controller.current_project_dir = "/proj/foo"
|
||||
with patch.object(gui_2, "apply_patch_to_file", return_value=(True, "patched")), \
|
||||
patch.object(gui_2.imgui, "close_current_popup"):
|
||||
result = gui_2._apply_pending_patch_result(app)
|
||||
assert result.ok, f"Expected ok=True on success, got errors: {result.errors}"
|
||||
assert result.data is True
|
||||
assert app._show_patch_modal is False
|
||||
assert app._patch_error_message is None
|
||||
|
||||
|
||||
def test_phase_5_l1367_apply_pending_patch_result_failure():
|
||||
"""
|
||||
L1367 _apply_pending_patch_result returns Result.ok=False with ErrorInfo on failure.
|
||||
|
||||
When apply_patch_to_file returns (False, msg) or raises, the helper sets
|
||||
app._patch_error_message to the error message and returns
|
||||
Result(data=False, errors=[ErrorInfo]).
|
||||
"""
|
||||
from src import gui_2
|
||||
from unittest.mock import MagicMock, patch
|
||||
app = MagicMock()
|
||||
app._pending_patch_text = "--- a/foo.py\n+++ b/foo.py\n"
|
||||
app.controller.current_project_dir = "/proj/foo"
|
||||
with patch.object(gui_2, "apply_patch_to_file", side_effect=RuntimeError("patch blew up")):
|
||||
result = gui_2._apply_pending_patch_result(app)
|
||||
assert not result.ok, f"Expected ok=False on failure, got data: {result.data}"
|
||||
assert result.errors, "Expected at least one error on failure"
|
||||
err = result.errors[0]
|
||||
assert err.source == "gui_2._apply_pending_patch_result"
|
||||
assert "patch blew up" in err.message
|
||||
|
||||
Reference in New Issue
Block a user