TIER-2 READ conductor/code_styleguides/error_handling.md end-to-end before Phase 8: refactor(gui_2): migrate L591 _diag_layout_state to Result[T] (Phase 8)
Migrate the ini-file-read try/except in App._diag_layout_state (L591) to the canonical Result[T] pattern: - Extract _diag_layout_state_ini_text_result(app, ini_path) -> Result[str] helper into new Phase 8 Property Setter / State Result Helpers region. - The legacy _diag_layout_state method calls the helper and drains errors to app._startup_timeline_errors (the Phase 2 drain plane for startup callbacks). - The original fallback behavior (early return on read failure, stderr write for visibility) is preserved. Tests: - test_phase_8_l591_diag_layout_state_ini_text_result_success - test_phase_8_l591_diag_layout_state_ini_text_result_failure Audit: INTERNAL_BROAD_CATCH count in src/gui_2.py dropped from 2 to 1 (remaining: L896 _capture_workspace_profile, formerly L897 in inventory).
This commit is contained in:
+34
-6
@@ -584,13 +584,12 @@ class App:
|
||||
return
|
||||
ini_size = _os.path.getsize(ini_path)
|
||||
sys.stderr.write(f"[GUI] layout file: {ini_path} ({ini_size} bytes)\n")
|
||||
#Note(Ed): Exception(Thirdparty)
|
||||
try:
|
||||
with open(ini_path, encoding="utf-8") as _f:
|
||||
_ini_text = _f.read()
|
||||
except Exception as _e:
|
||||
sys.stderr.write(f"[GUI] could not read layout file: {_e}\n")
|
||||
result = _diag_layout_state_ini_text_result(self, ini_path)
|
||||
if not result.ok:
|
||||
if not hasattr(self, "_startup_timeline_errors"): self._startup_timeline_errors = []
|
||||
self._startup_timeline_errors.append(("_diag_layout_state", result.errors[0]))
|
||||
return
|
||||
_ini_text = result.data
|
||||
_STALE_WINDOW_NAMES = {
|
||||
"Projects", "Files", "Screenshots", "Discussion History",
|
||||
"Provider", "Message", "Response", "Tool Calls",
|
||||
@@ -8102,4 +8101,33 @@ def _worker_context_preview_result(app: "App") -> Result[None]:
|
||||
|
||||
#endregion: Phase 7 Worker/Background Result Helpers
|
||||
|
||||
#region: Phase 8 Property Setter / State Result Helpers (result_migration_gui_2_20260619)
|
||||
|
||||
def _diag_layout_state_ini_text_result(app: "App", ini_path: str) -> Result[str]:
|
||||
"""Drain-aware variant of L591 _diag_layout_state ini-file-read try block.
|
||||
|
||||
Extracts the thirdparty file-open try/except from App._diag_layout_state
|
||||
into a Result-returning helper. On exception, returns Result(data="",
|
||||
errors=[ErrorInfo]) so the legacy wrapper can early-return. On success,
|
||||
returns Result(data=ini_text) where ini_text is the file content. The
|
||||
legacy wrapper drains errors to app._startup_timeline_errors (startup
|
||||
callback drain plane).
|
||||
|
||||
[C: src/gui_2.py:App._diag_layout_state (L591 legacy wrapper)]
|
||||
"""
|
||||
try:
|
||||
with open(ini_path, encoding="utf-8") as _f:
|
||||
ini_text = _f.read()
|
||||
return Result(data=ini_text)
|
||||
except Exception as e:
|
||||
sys.stderr.write(f"[GUI] could not read layout file: {e}\n")
|
||||
return Result(data="", errors=[ErrorInfo(
|
||||
kind=ErrorKind.INTERNAL,
|
||||
message=f"Could not read layout file {ini_path}: {e}",
|
||||
source="gui_2._diag_layout_state_ini_text_result",
|
||||
original=e,
|
||||
)])
|
||||
|
||||
#endregion: Phase 8 Property Setter / State Result Helpers
|
||||
|
||||
#endregion: MMA
|
||||
|
||||
@@ -1506,3 +1506,50 @@ def test_phase_7_invariant_all_1_migration_sites_have_tests():
|
||||
f"Expected a test matching '{failure_pattern}'."
|
||||
)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Phase 8 Tests - Migration of 2 INTERNAL_BROAD_CATCH property setter sites
|
||||
# to Result[T]. Each site gets 2 tests: success and failure.
|
||||
# Migration pattern: legacy wrapper drains to app._startup_timeline_errors
|
||||
# (for startup callbacks like L591) or app._last_request_errors
|
||||
# (for property setters like L897).
|
||||
# =============================================================================
|
||||
|
||||
|
||||
def test_phase_8_l591_diag_layout_state_ini_text_result_success():
|
||||
"""
|
||||
L591 _diag_layout_state_ini_text_result returns Result(data=ini_text) on success.
|
||||
|
||||
The helper wraps the ini-file-read try/except in App._diag_layout_state.
|
||||
On success, returns Result(data=ini_text) where ini_text is the file content.
|
||||
The legacy wrapper drains errors to app._startup_timeline_errors.
|
||||
"""
|
||||
from src import gui_2
|
||||
from unittest.mock import MagicMock, patch, mock_open
|
||||
app = MagicMock()
|
||||
with patch("builtins.open", mock_open(read_data="[Window][Provider]\nPos=10,20")):
|
||||
result = gui_2._diag_layout_state_ini_text_result(app, "/proj/manualslop_layout.ini")
|
||||
assert result.ok, f"Expected ok=True on success, got errors: {result.errors}"
|
||||
assert "[Window][Provider]" in result.data
|
||||
|
||||
|
||||
def test_phase_8_l591_diag_layout_state_ini_text_result_failure():
|
||||
"""
|
||||
L591 _diag_layout_state_ini_text_result returns Result(data="", errors=[ErrorInfo]) on failure.
|
||||
|
||||
When the ini file read raises (e.g., permission error, encoding error),
|
||||
the helper returns Result with ErrorInfo describing the failure and
|
||||
data="" so the caller can still proceed (return early).
|
||||
"""
|
||||
from src import gui_2
|
||||
from unittest.mock import MagicMock, patch
|
||||
app = MagicMock()
|
||||
with patch("builtins.open", side_effect=PermissionError("permission denied")):
|
||||
result = gui_2._diag_layout_state_ini_text_result(app, "/proj/manualslop_layout.ini")
|
||||
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._diag_layout_state_ini_text_result"
|
||||
assert "permission denied" in err.message
|
||||
assert result.data == ""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user