Private
Public Access
TIER-2 READ conductor/code_styleguides/error_handling.md end-to-end before Phase 10: refactor(gui_2): migrate L4911 _on_warmup_complete_callback to Result[T] (Phase 10 site 10)
Extracted _on_warmup_complete_callback_result(app, status) -> Result[None] helper above the callback. ANTI-SLIMING: full Result[T] propagation (NO except+pass-after-log). 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 _on_warmup_complete_callback preserves its signature, calls the helper, and drains to app.controller._worker_errors with the controller lock acquired on append (thread-safety critical per sub-track 4 spec). Tests: 2 new tests verify both paths (success and RuntimeError). Audit: L4911 reclassified from INTERNAL_SILENT_SWALLOW (4 sites remaining, was 5). New helper L4911 is INTERNAL_COMPLIANT.
This commit is contained in:
+29
-2
@@ -4916,6 +4916,26 @@ def _on_warmup_complete_callback(app: App, status: dict) -> None:
|
||||
(or the diagnostics panel) can read.
|
||||
Runs on a background _io_pool thread; only sets primitive state.
|
||||
"""
|
||||
cb_result = _on_warmup_complete_callback_result(app, status)
|
||||
if not cb_result.ok:
|
||||
controller = getattr(app, "controller", None)
|
||||
if controller is not None and hasattr(controller, "_worker_errors_lock"):
|
||||
with controller._worker_errors_lock:
|
||||
if not hasattr(controller, "_worker_errors"): controller._worker_errors = []
|
||||
controller._worker_errors.append(("_on_warmup_complete_callback", cb_result.errors[0]))
|
||||
|
||||
def _on_warmup_complete_callback_result(app: "App", status: dict) -> Result[None]:
|
||||
"""Drain-aware variant of _on_warmup_complete_callback (L4911 INTERNAL_SILENT_SWALLOW).
|
||||
|
||||
Extracts the warmup-completion body from _on_warmup_complete_callback
|
||||
into a Result-returning helper. On exception (status dict corruption,
|
||||
lock acquisition failure), converts to ErrorInfo (logging NOT a drain
|
||||
per the user's principle 2026-06-17). The legacy callback drains to
|
||||
app.controller._worker_errors with the controller lock acquired on
|
||||
append (thread-safety critical per sub-track 4 spec).
|
||||
|
||||
[C: src/gui_2.py:_on_warmup_complete_callback (L4911 legacy wrapper)]
|
||||
"""
|
||||
try:
|
||||
app._warmup_completion_ts = time.time()
|
||||
pending = status.get("pending", [])
|
||||
@@ -4925,12 +4945,19 @@ def _on_warmup_complete_callback(app: App, status: dict) -> None:
|
||||
if failed: msg = f"Warmup finished with {len(failed)} failures ({total} modules)"
|
||||
else: msg = f"All imports ready ({total} modules)"
|
||||
if not hasattr(app, "_warmup_toast_lock"):
|
||||
import threading as _threading #TODO(Ed): Review local import
|
||||
import threading as _threading
|
||||
app._warmup_toast_lock = _threading.Lock()
|
||||
with app._warmup_toast_lock:
|
||||
if not hasattr(app, "_warmup_toast_messages"): app._warmup_toast_messages = []
|
||||
app._warmup_toast_messages.append((time.time(), msg))
|
||||
except Exception: pass
|
||||
return Result(data=None)
|
||||
except Exception as e:
|
||||
return Result(data=None, errors=[ErrorInfo(
|
||||
kind=ErrorKind.INTERNAL,
|
||||
message=f"on_warmup_complete_callback body failed: {e}",
|
||||
source="gui_2._on_warmup_complete_callback_result",
|
||||
original=e,
|
||||
)])
|
||||
|
||||
def render_warmup_status_indicator(app: App) -> None:
|
||||
"""Renders a transient warmup status indicator in the main interface frame. Shows progress
|
||||
|
||||
@@ -2111,4 +2111,51 @@ def test_phase_10_l1693_autosave_flush_result_failure():
|
||||
assert "disk full" in err.message
|
||||
|
||||
|
||||
def test_phase_10_l4911_on_warmup_complete_callback_result_success():
|
||||
"""
|
||||
L4911 _on_warmup_complete_callback_result returns Result(data=None) on success.
|
||||
|
||||
The helper extracts the warmup-completion try/except from
|
||||
_on_warmup_complete_callback into a Result-returning helper. On success,
|
||||
returns Result(data=None). The legacy callback (which runs on a
|
||||
background _io_pool thread) sets app._warmup_completion_ts and appends
|
||||
to app._warmup_toast_messages under the lock.
|
||||
"""
|
||||
from unittest.mock import MagicMock
|
||||
import src.gui_2 as gui2_mod
|
||||
app = MagicMock()
|
||||
app.controller = MagicMock()
|
||||
status = {"pending": [], "completed": ["ai_client", "mcp_client"], "failed": []}
|
||||
result = gui2_mod._on_warmup_complete_callback_result(app, status)
|
||||
assert result.ok, f"Expected ok=True on success, got errors: {result.errors}"
|
||||
assert result.data is None
|
||||
assert app._warmup_toast_messages, "Expected warmup toast message to be appended"
|
||||
|
||||
|
||||
def test_phase_10_l4911_on_warmup_complete_callback_result_failure():
|
||||
"""
|
||||
L4911 _on_warmup_complete_callback_result returns Result(data=None, errors=[ErrorInfo]) on failure.
|
||||
|
||||
When the warmup-completion body raises (e.g., status dict corruption,
|
||||
lock acquisition failure), the helper converts to ErrorInfo. The legacy
|
||||
wrapper drains to app._worker_errors with the controller lock acquired
|
||||
on append (thread-safety critical per sub-track 4 spec).
|
||||
"""
|
||||
from unittest.mock import MagicMock
|
||||
import src.gui_2 as gui2_mod
|
||||
app = MagicMock()
|
||||
app.controller = MagicMock()
|
||||
app.controller._worker_errors_lock = MagicMock()
|
||||
# Force status.get to raise to trigger the except
|
||||
bad_status = MagicMock()
|
||||
bad_status.get.side_effect = RuntimeError("status dict corrupted")
|
||||
result = gui2_mod._on_warmup_complete_callback_result(app, bad_status)
|
||||
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._on_warmup_complete_callback_result"
|
||||
assert "status dict corrupted" in err.message
|
||||
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user