TIER-2 READ conductor/code_styleguides/error_handling.md end-to-end before Phase 10: refactor(gui_2): migrate L1466 _close_vscode_diff terminate to Result[T] (Phase 10 site 7)
Extracted _close_vscode_diff_terminate_result(app) -> Result[None] helper above the App._close_vscode_diff method. ANTI-SLIMING: full Result[T] propagation (NO except+pass). The helper returns Result(data=None) on success or Result(data=None, errors=[ErrorInfo]) on exception (logging NOT a drain per the user's principle 2026-06-17). The legacy _close_vscode_diff method preserves its signature, calls the helper, drains errors to self._last_request_errors, and proceeds to set self._vscode_diff_process = None (preserving the original post-error behavior of clearing the handle). Tests: 2 new tests verify both paths (success and OSError). Audit: L1466 reclassified from INTERNAL_SILENT_SWALLOW (7 sites remaining, was 8). New helper L1466 is INTERNAL_COMPLIANT.
This commit is contained in:
+27
-4
@@ -1465,12 +1465,35 @@ def _gui_func_entry_log_result(app: "App") -> Result[None]:
|
||||
|
||||
def _close_vscode_diff(self) -> None:
|
||||
if hasattr(self, '_vscode_diff_process') and self._vscode_diff_process:
|
||||
try:
|
||||
self._vscode_diff_process.terminate()
|
||||
except Exception:
|
||||
pass
|
||||
term_result = _close_vscode_diff_terminate_result(self)
|
||||
if not term_result.ok:
|
||||
if not hasattr(self, '_last_request_errors'): self._last_request_errors = []
|
||||
self._last_request_errors.append(("_close_vscode_diff.terminate", term_result.errors[0]))
|
||||
self._vscode_diff_process = None
|
||||
|
||||
def _close_vscode_diff_terminate_result(app: "App") -> Result[None]:
|
||||
"""Drain-aware variant of App._close_vscode_diff terminate try block (L1466 INTERNAL_SILENT_SWALLOW).
|
||||
|
||||
Extracts the self._vscode_diff_process.terminate() try/except from
|
||||
App._close_vscode_diff into a Result-returning helper. On exception
|
||||
(process already exited, invalid handle), converts to ErrorInfo (logging
|
||||
NOT a drain per the user's principle 2026-06-17). The legacy wrapper
|
||||
drains to self._last_request_errors and proceeds to set
|
||||
self._vscode_diff_process = None (preserving the original behavior).
|
||||
|
||||
[C: src/gui_2.py:App._close_vscode_diff (L1466 legacy wrapper)]
|
||||
"""
|
||||
try:
|
||||
app._vscode_diff_process.terminate()
|
||||
return Result(data=None)
|
||||
except Exception as e:
|
||||
return Result(data=None, errors=[ErrorInfo(
|
||||
kind=ErrorKind.INTERNAL,
|
||||
message=f"_vscode_diff_process.terminate failed: {e}",
|
||||
source="gui_2._close_vscode_diff_terminate_result",
|
||||
original=e,
|
||||
)])
|
||||
|
||||
def _apply_pending_patch(self) -> None:
|
||||
if not self._pending_patch_text:
|
||||
self._patch_error_message = "No patch to apply"
|
||||
|
||||
@@ -1990,4 +1990,46 @@ def test_phase_10_l1152_gui_func_entry_log_result_failure():
|
||||
assert "broken pipe" in err.message
|
||||
|
||||
|
||||
def test_phase_10_l1466_close_vscode_diff_terminate_result_success():
|
||||
"""
|
||||
L1466 _close_vscode_diff_terminate_result returns Result(data=None) on success.
|
||||
|
||||
The helper extracts the self._vscode_diff_process.terminate() try/except
|
||||
from App._close_vscode_diff into a Result-returning helper. On success,
|
||||
returns Result(data=None). The legacy wrapper sets
|
||||
self._vscode_diff_process = None (the original behavior preserved).
|
||||
"""
|
||||
from unittest.mock import MagicMock
|
||||
import src.gui_2 as gui2_mod
|
||||
app = MagicMock()
|
||||
app._vscode_diff_process = MagicMock()
|
||||
result = gui2_mod._close_vscode_diff_terminate_result(app)
|
||||
assert result.ok, f"Expected ok=True on success, got errors: {result.errors}"
|
||||
assert result.data is None
|
||||
app._vscode_diff_process.terminate.assert_called_once()
|
||||
|
||||
|
||||
def test_phase_10_l1466_close_vscode_diff_terminate_result_failure():
|
||||
"""
|
||||
L1466 _close_vscode_diff_terminate_result returns Result(data=None, errors=[ErrorInfo]) on failure.
|
||||
|
||||
When the process.terminate() call raises (e.g., process already exited,
|
||||
invalid handle), the helper converts to ErrorInfo. The legacy wrapper
|
||||
drains to self._last_request_errors and proceeds to set
|
||||
self._vscode_diff_process = None (preserving the original behavior).
|
||||
"""
|
||||
from unittest.mock import MagicMock
|
||||
import src.gui_2 as gui2_mod
|
||||
app = MagicMock()
|
||||
app._vscode_diff_process = MagicMock()
|
||||
app._vscode_diff_process.terminate.side_effect = OSError("process already exited")
|
||||
result = gui2_mod._close_vscode_diff_terminate_result(app)
|
||||
assert not result.ok, f"Expected ok=False on failure, got data: {result.data}"
|
||||
assert result.data is None
|
||||
assert result.errors, "Expected at least one error on failure"
|
||||
err = result.errors[0]
|
||||
assert err.source == "gui_2._close_vscode_diff_terminate_result"
|
||||
assert "process already exited" in err.message
|
||||
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user