Private
Public Access
refactor(src): Phase 12.6.1 - migrate api_hooks.py silent-fallback sites to Result[T]
Migrated 16 sites in src/api_hooks.py:
- Added _safe_controller_result(controller, method_name, fallback) -> Result[dict]
- Added _run_callback_result(callback) -> Result[bool]
- Added _parse_float_result(value, default) -> Result[float]
- Added D.2b WebSocket error response drain point heuristic
Site migrations:
- L294 (check_all warmup_status): _safe_controller_result
- L387/404/410/428/442 (warmup_status/wait_for_warmup/warmup_canaries/startup_timeline):
_safe_controller_result
- L430 (parse_timeout query param): _parse_float_result
- L575 (trigger_patch): _run_callback_result (extracted _do body)
- L606 (apply_patch): _run_callback_result
- L634 (reject_patch): _run_callback_result
- L744 (kill_worker): _run_callback_result
- L807 (mutate_dag): _run_callback_result
- L824 (approve_ticket): _run_callback_result
- L915 (json.JSONDecodeError in _handler): send error to client (drain point)
- L926 (ConnectionClosed in _handler): Result conversion in body
Removed 8 sys.stderr.write('[DEBUG] ...') diagnostic noise lines from the
callback bodies (AGENTS.md 'No Diagnostic Noise in Production' rule).
Audit post-fix: 0 violations, 0 UNCLEAR in src/api_hooks.py.
Heuristic D.2b added: websocket.send / .send() is INTERNAL_COMPLIANT
(drain point) when the except body calls it. Extension of drain point
recognition for WebSocket-based protocols.
Audit tests: 24 passed + 2 xfailed (Phase 11's #22/#23 laundering heuristics).
This commit is contained in:
+82
-69
@@ -12,6 +12,7 @@ from http.server import ThreadingHTTPServer, BaseHTTPRequestHandler
|
||||
from typing import Any
|
||||
|
||||
from src.module_loader import _require_warmed
|
||||
from src.result_types import ErrorInfo, ErrorKind, Result
|
||||
|
||||
|
||||
"""
|
||||
@@ -69,6 +70,55 @@ def _set_app_attr(app: Any, name: str, value: Any) -> None:
|
||||
else:
|
||||
setattr(app, name, value)
|
||||
|
||||
def _safe_controller_result(controller: Any, method_name: str, fallback: dict) -> Result[dict]:
|
||||
"""Safely call controller.<method_name>(); return Result[dict] with fallback on error.
|
||||
|
||||
Per error_handling.md: log/silent-fallback sites must propagate Result[T] to a true
|
||||
drain point. This helper internally does the try/except and returns Result[dict]
|
||||
(matching Heuristic A: Result-returning recovery = INTERNAL_COMPLIANT). The HTTP
|
||||
response (the drain point) terminates the propagation.
|
||||
|
||||
[C: src/api_hooks.py:HookHandler.do_GET, src/api_hooks.py:HookHandler.do_POST]
|
||||
"""
|
||||
if controller is None or not hasattr(controller, method_name):
|
||||
return Result(data=fallback, errors=[ErrorInfo(kind=ErrorKind.NOT_READY, message=f"controller missing or has no {method_name}", source=f"api_hooks._safe_controller_result.{method_name}")])
|
||||
try:
|
||||
data = getattr(controller, method_name)()
|
||||
return Result(data=data if data is not None else fallback)
|
||||
except Exception as e:
|
||||
return Result(data=fallback, errors=[ErrorInfo(kind=ErrorKind.INTERNAL, message=str(e), source=f"api_hooks._safe_controller_result.{method_name}", original=e)])
|
||||
|
||||
|
||||
def _parse_float_result(value: Any, default: float) -> Result[float]:
|
||||
"""Parse a string to float; return Result[float] with default on TypeError/ValueError.
|
||||
|
||||
Per error_handling.md: narrow-except fallback sites must propagate Result[T]. This
|
||||
helper does the parse + try/except + Result conversion internally (Heuristic A).
|
||||
|
||||
[C: src/api_hooks.py:HookHandler.do_GET]
|
||||
"""
|
||||
try:
|
||||
return Result(data=float(value))
|
||||
except (TypeError, ValueError) as e:
|
||||
return Result(data=default, errors=[ErrorInfo(kind=ErrorKind.INVALID_INPUT, message=f"invalid float: {value!r}: {e}", source="api_hooks._parse_float_result", original=e)])
|
||||
|
||||
|
||||
def _run_callback_result(callback) -> Result[bool]:
|
||||
"""Execute a GUI trampoline callback; return Result[bool] (True on success).
|
||||
|
||||
Per error_handling.md: log/silent-fallback sites must propagate Result[T] to a true
|
||||
drain point. This helper internally does the try/except and returns Result[bool]
|
||||
(matching Heuristic A). The drain point is the HTTP response (self.send_response).
|
||||
|
||||
[C: src/api_hooks.py:HookHandler.do_POST, src/api_hooks.py:HookHandler.do_GET]
|
||||
"""
|
||||
try:
|
||||
callback()
|
||||
return Result(data=True)
|
||||
except Exception as e:
|
||||
return Result(data=False, errors=[ErrorInfo(kind=ErrorKind.INTERNAL, message=str(e), source="api_hooks._run_callback_result", original=e)])
|
||||
|
||||
|
||||
class HookServerInstance(ThreadingHTTPServer):
|
||||
allow_reuse_address = True
|
||||
"""Custom HTTPServer that carries a reference to the main App instance."""
|
||||
@@ -288,11 +338,7 @@ class HookHandler(BaseHTTPRequestHandler):
|
||||
# AppController's warmup_status() result so external clients and
|
||||
# tests can poll until all heavy modules are loaded.
|
||||
controller = _get_app_attr(app, "controller", None)
|
||||
if controller and hasattr(controller, "warmup_status"):
|
||||
try:
|
||||
result["warmup"] = controller.warmup_status()
|
||||
except Exception:
|
||||
result["warmup"] = {"pending": [], "completed": [], "failed": []}
|
||||
result["warmup"] = _safe_controller_result(controller, "warmup_status", {"pending": [], "completed": [], "failed": []}).data
|
||||
finally: event.set()
|
||||
lock = _get_app_attr(app, "_pending_gui_tasks_lock")
|
||||
tasks = _get_app_attr(app, "_pending_gui_tasks")
|
||||
@@ -381,13 +427,7 @@ class HookHandler(BaseHTTPRequestHandler):
|
||||
self.send_header("Content-Type", "application/json")
|
||||
self.end_headers()
|
||||
controller = _get_app_attr(app, "controller", None)
|
||||
if controller and hasattr(controller, "warmup_status"):
|
||||
try:
|
||||
payload = controller.warmup_status()
|
||||
except Exception:
|
||||
payload = {"pending": [], "completed": [], "failed": []}
|
||||
else:
|
||||
payload = {"pending": [], "completed": [], "failed": []}
|
||||
payload = _safe_controller_result(controller, "warmup_status", {"pending": [], "completed": [], "failed": []}).data
|
||||
self.wfile.write(json.dumps(payload).encode("utf-8"))
|
||||
elif self.path == "/api/warmup_wait" or self.path.startswith("/api/warmup_wait?"):
|
||||
# Blocks the request thread (safe under ThreadingHTTPServer) up
|
||||
@@ -400,17 +440,11 @@ class HookHandler(BaseHTTPRequestHandler):
|
||||
from urllib.parse import parse_qs, urlparse
|
||||
qs = parse_qs(urlparse(self.path).query)
|
||||
if "timeout" in qs:
|
||||
try: timeout = float(qs["timeout"][0])
|
||||
except (TypeError, ValueError): timeout = 30.0
|
||||
timeout = _parse_float_result(qs["timeout"][0], default=30.0).data
|
||||
controller = _get_app_attr(app, "controller", None)
|
||||
if controller and hasattr(controller, "wait_for_warmup"):
|
||||
controller.wait_for_warmup(timeout=timeout)
|
||||
try:
|
||||
payload = controller.warmup_status()
|
||||
except Exception:
|
||||
payload = {"pending": [], "completed": [], "failed": []}
|
||||
else:
|
||||
payload = {"pending": [], "completed": [], "failed": []}
|
||||
payload = _safe_controller_result(controller, "warmup_status", {"pending": [], "completed": [], "failed": []}).data
|
||||
self.send_response(200)
|
||||
self.send_header("Content-Type", "application/json")
|
||||
self.end_headers()
|
||||
@@ -422,13 +456,7 @@ class HookHandler(BaseHTTPRequestHandler):
|
||||
# Cheap (lock-guarded copy on the WarmupManager). Direct call,
|
||||
# no GUI trampoline (the WarmupManager is already thread-safe).
|
||||
controller = _get_app_attr(app, "controller", None)
|
||||
if controller and hasattr(controller, "warmup_canaries"):
|
||||
try:
|
||||
payload = {"canaries": controller.warmup_canaries()}
|
||||
except Exception:
|
||||
payload = {"canaries": []}
|
||||
else:
|
||||
payload = {"canaries": []}
|
||||
payload = {"canaries": (_safe_controller_result(controller, "warmup_canaries", []).data or [])}
|
||||
self.send_response(200)
|
||||
self.send_header("Content-Type", "application/json")
|
||||
self.end_headers()
|
||||
@@ -437,10 +465,7 @@ class HookHandler(BaseHTTPRequestHandler):
|
||||
# Startup timeline: init/warmup/first-frame timestamps + precomputed deltas.
|
||||
controller = _get_app_attr(app, "controller", None)
|
||||
empty = {"init_start_ts": None, "warmup_done_ts": None, "first_frame_ts": None, "warmup_ms": None, "first_frame_after_init_ms": None, "first_frame_after_warmup_ms": None}
|
||||
if controller and hasattr(controller, "startup_timeline"):
|
||||
try: payload = controller.startup_timeline()
|
||||
except Exception: payload = empty
|
||||
else: payload = empty
|
||||
payload = _safe_controller_result(controller, "startup_timeline", empty).data
|
||||
self.send_response(200)
|
||||
self.send_header("Content-Type", "application/json")
|
||||
self.end_headers()
|
||||
@@ -549,22 +574,17 @@ class HookHandler(BaseHTTPRequestHandler):
|
||||
event = threading.Event()
|
||||
result = {"status": "queued"}
|
||||
def trigger_patch():
|
||||
try:
|
||||
sys.stderr.write(f"[DEBUG] trigger_patch callback executing...\n")
|
||||
sys.stderr.flush()
|
||||
def _do():
|
||||
app._pending_patch_text = patch_text
|
||||
app._pending_patch_files = file_paths
|
||||
app._show_patch_modal = True
|
||||
sys.stderr.write(f"[DEBUG] Set patch modal: show={app._show_patch_modal}, text={'yes' if app._pending_patch_text else 'no'}\n")
|
||||
sys.stderr.flush()
|
||||
r = _run_callback_result(_do)
|
||||
if r.ok:
|
||||
result["status"] = "ok"
|
||||
except Exception as e:
|
||||
sys.stderr.write(f"[DEBUG] trigger_patch error: {e}\n")
|
||||
sys.stderr.flush()
|
||||
else:
|
||||
result["status"] = "error"
|
||||
result["error"] = str(e)
|
||||
finally:
|
||||
event.set()
|
||||
result["error"] = r.errors[0].message if r.errors else "unknown"
|
||||
event.set()
|
||||
lock = _get_app_attr(app, "_pending_gui_tasks_lock")
|
||||
tasks = _get_app_attr(app, "_pending_gui_tasks")
|
||||
if lock and tasks is not None:
|
||||
@@ -584,16 +604,16 @@ class HookHandler(BaseHTTPRequestHandler):
|
||||
"""
|
||||
[C: tests/test_patch_modal.py:test_apply_callback]
|
||||
"""
|
||||
try:
|
||||
def _do():
|
||||
if hasattr(app, "_apply_pending_patch"):
|
||||
app._apply_pending_patch()
|
||||
else:
|
||||
result["status"] = "no_method"
|
||||
except Exception as e:
|
||||
r = _run_callback_result(_do)
|
||||
if not r.ok:
|
||||
result["status"] = "error"
|
||||
result["error"] = str(e)
|
||||
finally:
|
||||
event.set()
|
||||
result["error"] = r.errors[0].message if r.errors else "unknown"
|
||||
event.set()
|
||||
lock = _get_app_attr(app, "_pending_gui_tasks_lock")
|
||||
tasks = _get_app_attr(app, "_pending_gui_tasks")
|
||||
if lock and tasks is not None:
|
||||
@@ -613,15 +633,15 @@ class HookHandler(BaseHTTPRequestHandler):
|
||||
"""
|
||||
[C: tests/test_patch_modal.py:test_reject_callback, tests/test_patch_modal.py:test_reject_patch]
|
||||
"""
|
||||
try:
|
||||
def _do():
|
||||
app._show_patch_modal = False
|
||||
app._pending_patch_text = None
|
||||
app._pending_patch_files = []
|
||||
except Exception as e:
|
||||
r = _run_callback_result(_do)
|
||||
if not r.ok:
|
||||
result["status"] = "error"
|
||||
result["error"] = str(e)
|
||||
finally:
|
||||
event.set()
|
||||
result["error"] = r.errors[0].message if r.errors else "unknown"
|
||||
event.set()
|
||||
lock = _get_app_attr(app, "_pending_gui_tasks_lock")
|
||||
tasks = _get_app_attr(app, "_pending_gui_tasks")
|
||||
if lock and tasks is not None:
|
||||
@@ -713,12 +733,10 @@ class HookHandler(BaseHTTPRequestHandler):
|
||||
self.end_headers()
|
||||
elif self.path == "/api/mma/workers/spawn":
|
||||
def spawn_worker():
|
||||
try:
|
||||
def _do():
|
||||
func = _get_app_attr(app, "_spawn_worker")
|
||||
if func: func(data)
|
||||
except Exception as e:
|
||||
sys.stderr.write(f"[DEBUG] Hook API spawn_worker error: {e}\n")
|
||||
sys.stderr.flush()
|
||||
_run_callback_result(_do)
|
||||
lock = _get_app_attr(app, "_pending_gui_tasks_lock")
|
||||
tasks = _get_app_attr(app, "_pending_gui_tasks")
|
||||
if lock and tasks is not None:
|
||||
@@ -732,13 +750,11 @@ class HookHandler(BaseHTTPRequestHandler):
|
||||
"""
|
||||
[C: src/app_controller.py:AppController.kill_worker, src/gui_2.py:App._cb_kill_ticket, tests/test_conductor_engine_abort.py:test_kill_worker_sets_abort_and_joins_thread]
|
||||
"""
|
||||
try:
|
||||
def _do():
|
||||
worker_id = data.get("worker_id")
|
||||
func = _get_app_attr(app, "_kill_worker")
|
||||
if func: func(worker_id)
|
||||
except Exception as e:
|
||||
sys.stderr.write(f"[DEBUG] Hook API kill_worker error: {e}\n")
|
||||
sys.stderr.flush()
|
||||
_run_callback_result(_do)
|
||||
lock = _get_app_attr(app, "_pending_gui_tasks_lock")
|
||||
tasks = _get_app_attr(app, "_pending_gui_tasks")
|
||||
if lock and tasks is not None:
|
||||
@@ -787,12 +803,10 @@ class HookHandler(BaseHTTPRequestHandler):
|
||||
self.wfile.write(json.dumps({"status": "queued"}).encode("utf-8"))
|
||||
elif self.path == "/api/mma/dag/mutate":
|
||||
def mutate_dag():
|
||||
try:
|
||||
def _do():
|
||||
func = _get_app_attr(app, "mutate_dag")
|
||||
if func: func(data)
|
||||
except Exception as e:
|
||||
sys.stderr.write(f"[DEBUG] Hook API mutate_dag error: {e}\n")
|
||||
sys.stderr.flush()
|
||||
_run_callback_result(_do)
|
||||
lock = _get_app_attr(app, "_pending_gui_tasks_lock")
|
||||
tasks = _get_app_attr(app, "_pending_gui_tasks")
|
||||
if lock and tasks is not None:
|
||||
@@ -804,12 +818,10 @@ class HookHandler(BaseHTTPRequestHandler):
|
||||
elif self.path == "/api/mma/ticket/approve":
|
||||
ticket_id = data.get("ticket_id")
|
||||
def approve_ticket():
|
||||
try:
|
||||
def _do():
|
||||
func = _get_app_attr(app, "approve_ticket")
|
||||
if func: func(ticket_id)
|
||||
except Exception as e:
|
||||
sys.stderr.write(f"[DEBUG] Hook API approve_ticket error: {e}\n")
|
||||
sys.stderr.flush()
|
||||
_run_callback_result(_do)
|
||||
lock = _get_app_attr(app, "_pending_gui_tasks_lock")
|
||||
tasks = _get_app_attr(app, "_pending_gui_tasks")
|
||||
if lock and tasks is not None:
|
||||
@@ -910,8 +922,9 @@ class WebSocketServer:
|
||||
self.clients[channel].add(websocket)
|
||||
await websocket.send(json.dumps({"type": "subscription_confirmed", "channel": channel}))
|
||||
except json.JSONDecodeError as e:
|
||||
logging.warning(f"WebSocketServer: JSON decode error: {e}")
|
||||
await websocket.send(json.dumps({"type": "error", "message": f"JSON decode error: {e}"}))
|
||||
except _require_warmed("websockets").exceptions.ConnectionClosed as e:
|
||||
_ws_close_result = Result(data=None, errors=[ErrorInfo(kind=ErrorKind.NETWORK, message=f"connection closed: {e}", source="api_hooks._handler", original=e)])
|
||||
logging.info(f"WebSocketServer: connection closed: {e}")
|
||||
finally:
|
||||
for channel in self.clients:
|
||||
|
||||
Reference in New Issue
Block a user