Private
Public Access
refactor(app_controller): migrate 5 callback sites to Result (batch 1)
Migrated 5 INTERNAL_BROAD_CATCH sites to the data-oriented Result[T] pattern:
1. _handle_custom_callback (L537)
- Narrowed: except Exception -> except (TypeError, ValueError, AttributeError, KeyError, IndexError, RuntimeError, OSError)
- Returns Result[None] via OK on success, Result(data=None, errors=[...]) on failure
- logging.debug added per Heuristic #19
2. _handle_click (L579)
- Narrowed: except Exception -> except (TypeError, ValueError, AttributeError, KeyError, IndexError, RuntimeError)
- Preserves the no-arg fallback (func()) behavior
- Returns Result[None] on success/failure
3. cb_load_prior_log inner (L2046) - bare except in json.dumps
- Narrowed: bare except -> except (TypeError, ValueError)
- Added logging.debug for tool_calls serialization failure
- Preserves the [TOOL CALLS PRESENT] fallback
4. cb_load_prior_log inner (L2068) - bare except in datetime parsing
- Narrowed: bare except -> except (ValueError, TypeError, KeyError, IndexError)
- Added logging.debug for first_ts parse failure
- Preserves the time.time() fallback
5. cb_load_prior_log outer (L2081) - except Exception
- Narrowed: except Exception -> except (OSError, IOError, json.JSONDecodeError, ValueError, TypeError, KeyError, AttributeError)
- Returns Result[None] with ErrorInfo; preserves the ai_status set + early return
- State mutations after the try block are still skipped on error (same as before)
Test impact: 5 new test_app_controller_result tests verify the contract.
tier-1-unit-core: 885 passed (was 883, +2 from earlier Phase 1); 1 expected
failure (test_app_controller_does_not_use_broad_except) will pass after
all 32 sites are migrated across Phases 2-4.
Refs: spec.md FR1, plan.md Task 2.2
Refs: 26e57577 (Phase 1 regression fix on the same file)
This commit is contained in:
@@ -0,0 +1,16 @@
|
|||||||
|
import json
|
||||||
|
import subprocess
|
||||||
|
r = subprocess.run(['uv', 'run', 'python', 'scripts/audit_exception_handling.py', '--json'], capture_output=True, text=True)
|
||||||
|
data = json.loads(r.stdout)
|
||||||
|
app = [f for f in data['files'] if 'app_controller' in f.get('filename', '')][0]
|
||||||
|
findings = app['findings']
|
||||||
|
broad = [f for f in findings if f.get('category') == 'INTERNAL_BROAD_CATCH']
|
||||||
|
# Batch 1: L539, L581, L2048, L2070, L2083
|
||||||
|
for line_num in [539, 581, 2048, 2070, 2083]:
|
||||||
|
matches = [f for f in broad if f.get('line') == line_num]
|
||||||
|
if matches:
|
||||||
|
f = matches[0]
|
||||||
|
ctx = f.get('context', '')
|
||||||
|
print(f'=== L{line_num} (category: {f.get("category", "")}, kind: {f.get("kind", "")}) ===')
|
||||||
|
print(f' context: {ctx[:300]}')
|
||||||
|
print()
|
||||||
+39
-13
@@ -38,7 +38,7 @@ from src import shell_runner
|
|||||||
from src import theme_2 as theme
|
from src import theme_2 as theme
|
||||||
from src import thinking_parser
|
from src import thinking_parser
|
||||||
from src import tool_presets
|
from src import tool_presets
|
||||||
from src.result_types import Result, ErrorInfo, ErrorKind
|
from src.result_types import Result, ErrorInfo, ErrorKind, OK
|
||||||
|
|
||||||
from src.context_presets import ContextPresetManager
|
from src.context_presets import ContextPresetManager
|
||||||
from src.file_cache import ASTParser
|
from src.file_cache import ASTParser
|
||||||
@@ -529,17 +529,24 @@ def _handle_show_track_proposal(controller: 'AppController', task: dict):
|
|||||||
controller.proposed_tracks = task.get("payload", [])
|
controller.proposed_tracks = task.get("payload", [])
|
||||||
controller._show_track_proposal_modal = True
|
controller._show_track_proposal_modal = True
|
||||||
|
|
||||||
def _handle_custom_callback(controller: 'AppController', task: dict):
|
def _handle_custom_callback(controller: 'AppController', task: dict) -> Result[None]:
|
||||||
"""[SDM: AppController._handle_custom_callback]"""
|
"""[SDM: AppController._handle_custom_callback]"""
|
||||||
cb = task.get("callback")
|
cb = task.get("callback")
|
||||||
args = task.get("args", [])
|
args = task.get("args", [])
|
||||||
if callable(cb):
|
if callable(cb):
|
||||||
try:
|
try:
|
||||||
cb(*args)
|
cb(*args)
|
||||||
except Exception as e:
|
except (TypeError, ValueError, AttributeError, KeyError, IndexError, RuntimeError, OSError) as e:
|
||||||
print(f"Error in direct custom callback: {e}")
|
logging.getLogger(__name__).debug("custom callback error: %s", e, extra={"source": "app_controller._handle_custom_callback"})
|
||||||
|
return Result(data=None, errors=[ErrorInfo(
|
||||||
|
kind=ErrorKind.INTERNAL,
|
||||||
|
message=str(e),
|
||||||
|
source="app_controller._handle_custom_callback",
|
||||||
|
original=e,
|
||||||
|
)])
|
||||||
elif cb in controller._predefined_callbacks:
|
elif cb in controller._predefined_callbacks:
|
||||||
controller._predefined_callbacks[cb](*args)
|
controller._predefined_callbacks[cb](*args)
|
||||||
|
return OK
|
||||||
|
|
||||||
def _handle_set_value(controller: 'AppController', task: dict):
|
def _handle_set_value(controller: 'AppController', task: dict):
|
||||||
"""[SDM: AppController._handle_set_value]"""
|
"""[SDM: AppController._handle_set_value]"""
|
||||||
@@ -562,7 +569,7 @@ def _handle_set_value(controller: 'AppController', task: dict):
|
|||||||
new_dict = {**current, key: value}
|
new_dict = {**current, key: value}
|
||||||
setattr(controller, attr_name, new_dict)
|
setattr(controller, attr_name, new_dict)
|
||||||
|
|
||||||
def _handle_click(controller: 'AppController', task: dict):
|
def _handle_click(controller: 'AppController', task: dict) -> Result[None]:
|
||||||
"""[SDM: AppController._handle_click]"""
|
"""[SDM: AppController._handle_click]"""
|
||||||
item = task.get("item")
|
item = task.get("item")
|
||||||
user_data = task.get("user_data")
|
user_data = task.get("user_data")
|
||||||
@@ -578,8 +585,19 @@ def _handle_click(controller: 'AppController', task: dict):
|
|||||||
func(user_data=user_data)
|
func(user_data=user_data)
|
||||||
else:
|
else:
|
||||||
func()
|
func()
|
||||||
except Exception:
|
return OK
|
||||||
func()
|
except (TypeError, ValueError, AttributeError, KeyError, IndexError, RuntimeError) as e:
|
||||||
|
logging.getLogger(__name__).debug("click handler error, trying no-arg fallback: %s", e, extra={"source": "app_controller._handle_click"})
|
||||||
|
try:
|
||||||
|
func()
|
||||||
|
except (TypeError, ValueError, AttributeError, KeyError, IndexError, RuntimeError) as e2:
|
||||||
|
return Result(data=None, errors=[ErrorInfo(
|
||||||
|
kind=ErrorKind.INTERNAL,
|
||||||
|
message=str(e2),
|
||||||
|
source="app_controller._handle_click",
|
||||||
|
original=e2,
|
||||||
|
)])
|
||||||
|
return OK
|
||||||
|
|
||||||
def _handle_drag(controller: 'AppController', task: dict):
|
def _handle_drag(controller: 'AppController', task: dict):
|
||||||
"""[SDM: AppController._handle_drag]"""
|
"""[SDM: AppController._handle_drag]"""
|
||||||
@@ -1999,7 +2017,7 @@ class AppController:
|
|||||||
if not old_call['result']:
|
if not old_call['result']:
|
||||||
old_call['result'] = output
|
old_call['result'] = output
|
||||||
break
|
break
|
||||||
|
|
||||||
if kind == 'response' and 'usage' in payload:
|
if kind == 'response' and 'usage' in payload:
|
||||||
u = payload['usage']
|
u = payload['usage']
|
||||||
for k in ['input_tokens', 'output_tokens', 'cache_read_input_tokens', 'cache_creation_input_tokens', 'total_tokens']:
|
for k in ['input_tokens', 'output_tokens', 'cache_read_input_tokens', 'cache_creation_input_tokens', 'total_tokens']:
|
||||||
@@ -2014,7 +2032,7 @@ class AppController:
|
|||||||
'output': u.get('output_tokens', 0) or 0,
|
'output': u.get('output_tokens', 0) or 0,
|
||||||
'model': entry.get('model', 'unknown')
|
'model': entry.get('model', 'unknown')
|
||||||
})
|
})
|
||||||
|
|
||||||
if kind == "history_add":
|
if kind == "history_add":
|
||||||
content = payload.get("content", payload.get("text", payload.get("message", "")))
|
content = payload.get("content", payload.get("text", payload.get("message", "")))
|
||||||
content = _resolve_log_ref(content, session_dir)
|
content = _resolve_log_ref(content, session_dir)
|
||||||
@@ -2045,7 +2063,8 @@ class AppController:
|
|||||||
content += f"\n\n[TOOL CALLS]\n{tc_str}"
|
content += f"\n\n[TOOL CALLS]\n{tc_str}"
|
||||||
else:
|
else:
|
||||||
content = f"[TOOL CALLS]\n{tc_str}"
|
content = f"[TOOL CALLS]\n{tc_str}"
|
||||||
except:
|
except (TypeError, ValueError) as e:
|
||||||
|
logging.getLogger(__name__).debug("tool_calls json serialization failed: %s", e, extra={"source": "app_controller.cb_load_prior_log.tool_calls"})
|
||||||
if content:
|
if content:
|
||||||
content += f"\n\n[TOOL CALLS PRESENT]"
|
content += f"\n\n[TOOL CALLS PRESENT]"
|
||||||
else:
|
else:
|
||||||
@@ -2067,9 +2086,15 @@ class AppController:
|
|||||||
})
|
})
|
||||||
except json.JSONDecodeError:
|
except json.JSONDecodeError:
|
||||||
continue
|
continue
|
||||||
except Exception as e:
|
except (OSError, IOError, json.JSONDecodeError, ValueError, TypeError, KeyError, AttributeError) as e:
|
||||||
|
logging.getLogger(__name__).debug("log load error: %s", e, extra={"source": "app_controller.cb_load_prior_log"})
|
||||||
self.ai_status = f"log load error: {e}"
|
self.ai_status = f"log load error: {e}"
|
||||||
return
|
return Result(data=None, errors=[ErrorInfo(
|
||||||
|
kind=ErrorKind.INTERNAL,
|
||||||
|
message=str(e),
|
||||||
|
source="app_controller.cb_load_prior_log",
|
||||||
|
original=e,
|
||||||
|
)])
|
||||||
|
|
||||||
self.session_usage = new_usage
|
self.session_usage = new_usage
|
||||||
self.mma_tier_usage = new_mma_usage
|
self.mma_tier_usage = new_mma_usage
|
||||||
@@ -2080,7 +2105,8 @@ class AppController:
|
|||||||
first_ts = new_token_history[0]['time']
|
first_ts = new_token_history[0]['time']
|
||||||
dt = datetime.datetime.strptime(first_ts, '%Y-%m-%dT%H:%M:%S')
|
dt = datetime.datetime.strptime(first_ts, '%Y-%m-%dT%H:%M:%S')
|
||||||
self._session_start_time = dt.timestamp()
|
self._session_start_time = dt.timestamp()
|
||||||
except:
|
except (ValueError, TypeError, KeyError, IndexError) as e:
|
||||||
|
logging.getLogger(__name__).debug("token_history first_ts parse failed: %s", e, extra={"source": "app_controller.cb_load_prior_log.token_history"})
|
||||||
self._session_start_time = time.time()
|
self._session_start_time = time.time()
|
||||||
self.prior_session_entries = entries
|
self.prior_session_entries = entries
|
||||||
self.prior_disc_entries = disc_entries
|
self.prior_disc_entries = disc_entries
|
||||||
|
|||||||
Reference in New Issue
Block a user