diff --git a/app/pt_error_handler.py b/app/pt_error_handler.py new file mode 100644 index 00000000..d6db5a59 --- /dev/null +++ b/app/pt_error_handler.py @@ -0,0 +1,320 @@ +""" +PowerTraderAI Centralized Error Management System +Single application-wide error handler with notification routing and +classification. All modules should use `get_handler()` instead of +creating their own ErrorHandler instances. + +Usage: + from pt_error_handler import get_handler, on_critical, handle + + # Register a GUI alert callback for critical errors + on_critical(lambda report: show_alert(report.user_message)) + + # Handle an exception anywhere in the codebase + try: + risky_operation() + except Exception as exc: + handle(exc, context={"operation": "risky_operation"}) + + # Or via the global handler directly. The exception variable is only + # bound inside the `except` block on Python 3 (PEP 3134), so callers + # must invoke handle()/get_handler().handle() from within that block. + try: + risky_operation() + except Exception as exc: + get_handler().handle(exc) +""" + +import logging +import sys +import threading +from typing import Callable, Dict, List, Optional + +from pt_errors import ( + ErrorHandler, + ErrorReport, + ErrorSeverity, +) + +logger = logging.getLogger(__name__) + +# --------------------------------------------------------------------------- +# Notification callback type alias +# --------------------------------------------------------------------------- +NotificationCallback = Callable[[ErrorReport], None] + + +# --------------------------------------------------------------------------- +# ApplicationErrorHandler — singleton wrapping ErrorHandler +# --------------------------------------------------------------------------- +class ApplicationErrorHandler: + """ + Application-wide singleton error handler. + + Wraps pt_errors.ErrorHandler and adds: + - Per-severity notification callbacks (e.g. pop GUI alerts for CRITICAL) + - Global error bus: all modules share one instance + - Thread-safe init, callback registration, and invocation + - Error suppression rules for known noisy modules + """ + + _instance: Optional["ApplicationErrorHandler"] = None + _class_lock = threading.Lock() # Guards singleton creation + _init_lock = threading.Lock() # Guards lazy initialisation + + def __new__(cls) -> "ApplicationErrorHandler": + with cls._class_lock: + if cls._instance is None: + cls._instance = super().__new__(cls) + cls._instance._initialised = False + return cls._instance + + def _ensure_init(self) -> None: + """Thread-safe double-checked lazy initialisation.""" + if self._initialised: + return + with self._init_lock: + if self._initialised: # re-check after acquiring lock + return + self._handler = ErrorHandler(logger=logging.getLogger("pt.errors")) + self._callbacks: Dict[ErrorSeverity, List[NotificationCallback]] = { + sev: [] for sev in ErrorSeverity + } + self._suppressed_modules: set = set() + self._cb_lock = threading.Lock() + # Guards mutation/read of the underlying ErrorHandler state + # (error_reports, error_counts, error_counts_by_severity) so + # concurrent handle()/clear_history()/get_summary() calls can't + # lose increments or observe partially-updated counters. + self._state_lock = threading.Lock() + self._initialised = True + + # ------------------------------------------------------------------ + # Callback registration + # ------------------------------------------------------------------ + def register_callback( + self, severity: ErrorSeverity, callback: NotificationCallback + ) -> None: + """ + Register a callback fired whenever an error of `severity` is handled. + Callbacks must be non-blocking (offload heavy work to a thread). + """ + self._ensure_init() + with self._cb_lock: + self._callbacks[severity].append(callback) + + def unregister_all(self, severity: Optional[ErrorSeverity] = None) -> None: + """Clear callbacks for a severity level, or all levels if severity is None.""" + self._ensure_init() + with self._cb_lock: + if severity is not None: + self._callbacks[severity].clear() + else: + for cbs in self._callbacks.values(): + cbs.clear() + + # ------------------------------------------------------------------ + # Suppression + # ------------------------------------------------------------------ + def suppress_module(self, module_name: str) -> None: + """Suppress notification callbacks for errors originating from + ``module_name``. + + Accepts either the bare module name (``"pt_trader"``) or the filename + (``"pt_trader.py"``). :class:`ErrorReport` stores the filename, and + suppression normalizes both sides, so either spelling works. + """ + self._ensure_init() + with self._cb_lock: + self._suppressed_modules.add(self._normalize_module(module_name)) + + def unsuppress_module(self, module_name: str) -> None: + self._ensure_init() + with self._cb_lock: + self._suppressed_modules.discard(self._normalize_module(module_name)) + + # ------------------------------------------------------------------ + # Core handle + # ------------------------------------------------------------------ + def handle( + self, + error: Exception, + context: Optional[Dict] = None, + reraise: bool = False, + ) -> ErrorReport: + """ + Handle an exception: classify, log, fire callbacks. + + Args: + error: The exception to handle + context: Additional key/value context for the report + reraise: If True, re-raise the exception after handling + + Returns: + ErrorReport with full classification and metadata + """ + self._ensure_init() + with self._state_lock: + report = self._handler.handle_error(error, context=context) + self._fire_callbacks(report) + if reraise: + # If error is the currently active exception (called from inside an except + # block), bare raise preserves the original traceback exactly. + # Otherwise fall back to raise error which uses the traceback already + # attached to the exception object. + _, active_exc, _ = sys.exc_info() + if active_exc is error: + raise + raise error + return report + + def handle_critical( + self, error: Exception, context: Optional[Dict] = None + ) -> ErrorReport: + """ + Handle an exception and escalate to CRITICAL severity regardless of + automatic classification. Fires CRITICAL-level callbacks only. + + Uses ``ErrorHandler.handle_error(..., severity_override=CRITICAL)`` so + the underlying logger records the error at CRITICAL from the start — + no post-hoc severity mutation, no double-counting in + ``error_counts``, and ``get_critical_errors()`` sees the escalated + severity consistently. + """ + self._ensure_init() + with self._state_lock: + report = self._handler.handle_error( + error, + context=context, + severity_override=ErrorSeverity.CRITICAL, + ) + self._fire_callbacks(report) + return report + + def _fire_callbacks(self, report: ErrorReport) -> None: + self._ensure_init() + with self._cb_lock: + if self._is_module_suppressed(report.module): + return + callbacks = list(self._callbacks.get(report.severity, [])) + for cb in callbacks: + try: + cb(report) + except Exception: + # Log full traceback so callback bugs are diagnosable + logger.warning("Error callback raised an exception", exc_info=True) + + @staticmethod + def _normalize_module(name: Optional[str]) -> Optional[str]: + """Strip a trailing ``.py`` so callers can pass either the module + name (``pt_trader``) or the filename (``pt_trader.py``) when + registering suppression — :class:`ErrorReport` carries the filename + but a module name reads more naturally in code that calls + :meth:`suppress_module`.""" + if not name: + return name + return name[:-3] if name.endswith(".py") else name + + def _is_module_suppressed(self, module: Optional[str]) -> bool: + normalized = self._normalize_module(module) + return normalized is not None and normalized in self._suppressed_modules + + # ------------------------------------------------------------------ + # Query / stats + # ------------------------------------------------------------------ + def get_summary(self) -> Dict: + self._ensure_init() + with self._state_lock: + return self._handler.get_error_summary() + + def get_recent_errors(self, limit: int = 20) -> List[ErrorReport]: + self._ensure_init() + if limit <= 0: + return [] + with self._state_lock: + return self._handler.error_reports[-limit:] + + def get_critical_errors(self) -> List[ErrorReport]: + self._ensure_init() + with self._state_lock: + return [ + r + for r in self._handler.error_reports + if r.severity == ErrorSeverity.CRITICAL + ] + + def clear_history(self) -> None: + """Clear in-memory error history (does NOT affect log files). + + Resets every accumulator on the underlying ErrorHandler so that + subsequent ``get_summary()`` calls don't return stale severity totals + from the cleared run. Without clearing ``error_counts_by_severity``, + ``severities`` and ``by_category_severity`` would compound forever + across clears. + """ + self._ensure_init() + with self._state_lock: + self._handler.error_reports.clear() + self._handler.error_counts.clear() + self._handler.error_counts_by_severity.clear() + + @classmethod + def reset_singleton(cls) -> None: + """Destroy singleton and force re-init on next call. For testing only.""" + with cls._class_lock: + cls._instance = None + + +# --------------------------------------------------------------------------- +# Module-level convenience API +# --------------------------------------------------------------------------- +def get_handler() -> ApplicationErrorHandler: + """Return the application-wide singleton error handler.""" + h = ApplicationErrorHandler() + h._ensure_init() + return h + + +def handle( + error: Exception, + context: Optional[Dict] = None, + reraise: bool = False, +) -> ErrorReport: + """Module-level shortcut: handle(exc) from anywhere.""" + return get_handler().handle(error, context=context, reraise=reraise) + + +def on_critical(callback: NotificationCallback) -> None: + """Register a callback that fires on every CRITICAL error.""" + get_handler().register_callback(ErrorSeverity.CRITICAL, callback) + + +def on_error(callback: NotificationCallback) -> None: + """Register a callback that fires on HIGH severity errors.""" + get_handler().register_callback(ErrorSeverity.HIGH, callback) + + +def on_warning(callback: NotificationCallback) -> None: + """Register a callback that fires on MEDIUM severity errors.""" + get_handler().register_callback(ErrorSeverity.MEDIUM, callback) + + +def configure_gui_alerts( + critical_callback: Optional[NotificationCallback] = None, + error_callback: Optional[NotificationCallback] = None, +) -> None: + """ + Convenience: register GUI alert callbacks for CRITICAL and HIGH errors. + Call once during application startup. + + Example: + configure_gui_alerts( + critical_callback=lambda r: messagebox.showerror("Critical", r.user_message), + error_callback=lambda r: messagebox.showwarning("Error", r.user_message), + ) + """ + handler = get_handler() + if critical_callback is not None: + handler.register_callback(ErrorSeverity.CRITICAL, critical_callback) + if error_callback is not None: + handler.register_callback(ErrorSeverity.HIGH, error_callback) diff --git a/app/pt_errors.py b/app/pt_errors.py index d57e19a2..025db5a3 100644 --- a/app/pt_errors.py +++ b/app/pt_errors.py @@ -205,7 +205,15 @@ class ErrorHandler: def __init__(self, logger: Optional[logging.Logger] = None): self.logger = logger or logging.getLogger(__name__) self.error_reports: List[ErrorReport] = [] + # Flat per-category total counter (preserved for backward compatibility + # with external callers that read this attribute directly). self.error_counts: Dict[str, int] = {} + # Richer nested counter: category.value -> severity.value -> count. + # Both counters are bumped together in the single _update_error_counts + # call path, so they stay consistent for any one update. ErrorHandler + # is not itself thread-safe; concurrent callers must serialise access + # (the ApplicationErrorHandler wrapper does this via its _state_lock). + self.error_counts_by_severity: Dict[str, Dict[str, int]] = {} # Recovery suggestions for common errors self.recovery_suggestions = { @@ -226,7 +234,10 @@ def generate_error_id(self) -> str: return f"PT_{datetime.now().strftime('%Y%m%d_%H%M%S')}_{str(uuid.uuid4())[:8]}" def handle_error( - self, error: Exception, context: Dict[str, Any] = None + self, + error: Exception, + context: Dict[str, Any] = None, + severity_override: Optional[ErrorSeverity] = None, ) -> ErrorReport: """ Handle and report an error with comprehensive logging. @@ -234,6 +245,12 @@ def handle_error( Args: error: The exception that occurred context: Additional context information + severity_override: If provided, replaces the auto-classified or + PowerTraderError-declared severity *before* logging and counter + updates. This lets escalation paths (e.g. + ``ApplicationErrorHandler.handle_critical``) ensure the error + is logged at the escalated level instead of at the original + level followed by a post-hoc mutation. Returns: ErrorReport: Detailed error report @@ -258,6 +275,11 @@ def handle_error( severity = self._determine_severity(error, category) error_context = context or {} + # Apply caller override BEFORE logging/counting so logs and stats + # both reflect the final severity from the start. + if severity_override is not None: + severity = severity_override + # Create error report error_report = ErrorReport( error_id=self.generate_error_id(), @@ -277,12 +299,12 @@ def handle_error( ), ) - # Log the error + # Log the error (now at the final, possibly overridden severity) self._log_error(error_report) # Store error report self.error_reports.append(error_report) - self._update_error_counts(category) + self._update_error_counts(category, severity) return error_report @@ -381,33 +403,60 @@ def _log_error(self, error_report: ErrorReport) -> None: f"[{error_report.error_id}] Traceback:\\n{error_report.traceback_str}" ) - def _update_error_counts(self, category: ErrorCategory) -> None: - """Update error count statistics.""" - key = category.value - self.error_counts[key] = self.error_counts.get(key, 0) + 1 + def _update_error_counts( + self, category: ErrorCategory, severity: ErrorSeverity + ) -> None: + """Update both the flat per-category counter and the nested + category → severity → count counter in lockstep. + + ``error_counts`` stays a ``Dict[str, int]`` to preserve backward + compatibility for external callers that read it directly. The richer + breakdown lives on ``error_counts_by_severity``. + """ + cat_key = category.value + sev_key = severity.value + self.error_counts[cat_key] = self.error_counts.get(cat_key, 0) + 1 + bucket = self.error_counts_by_severity.setdefault(cat_key, {}) + bucket[sev_key] = bucket.get(sev_key, 0) + 1 + + def category_total(self, category: ErrorCategory) -> int: + """Return total count for ``category`` across all severities.""" + return self.error_counts.get(category.value, 0) def get_error_summary(self) -> Dict[str, Any]: - """Get summary of error statistics.""" + """Get summary of error statistics. + + Returns a dict with: + - ``total_errors``: total handled + - ``categories``: flat category → count + - ``severities``: flat severity → count (sum across categories) + - ``by_category_severity``: nested category → severity → count + """ total_errors = len(self.error_reports) if total_errors == 0: - return {"total_errors": 0, "categories": {}, "severities": {}} - - # Count by category - category_counts = {} - severity_counts = {} - - for report in self.error_reports: - category_counts[report.category.value] = ( - category_counts.get(report.category.value, 0) + 1 - ) - severity_counts[report.severity.value] = ( - severity_counts.get(report.severity.value, 0) + 1 - ) + # Always include `recent_errors` so the API shape stays consistent + # and callers can iterate without a special case for empty state. + return { + "total_errors": 0, + "categories": {}, + "severities": {}, + "by_category_severity": {}, + "recent_errors": [], + } + + severity_counts: Dict[str, int] = {} + for sev_bucket in self.error_counts_by_severity.values(): + for sev_key, count in sev_bucket.items(): + severity_counts[sev_key] = severity_counts.get(sev_key, 0) + count return { "total_errors": total_errors, - "categories": category_counts, + "categories": dict(self.error_counts), "severities": severity_counts, + "by_category_severity": { + cat: dict(sev_bucket) + for cat, sev_bucket in self.error_counts_by_severity.items() + }, "recent_errors": [ { "id": report.error_id, diff --git a/app/test_error_handler.py b/app/test_error_handler.py new file mode 100644 index 00000000..77f7b75c --- /dev/null +++ b/app/test_error_handler.py @@ -0,0 +1,287 @@ +"""Tests for pt_error_handler centralized error management system.""" + +import unittest +from unittest.mock import MagicMock, patch + +from pt_error_handler import ( + ApplicationErrorHandler, + configure_gui_alerts, + get_handler, + handle, + on_critical, + on_error, + on_warning, +) +from pt_errors import ErrorSeverity, TradingError, ErrorCategory + + +class TestApplicationErrorHandlerSingleton(unittest.TestCase): + def setUp(self): + ApplicationErrorHandler.reset_singleton() + + def tearDown(self): + ApplicationErrorHandler.reset_singleton() + + def test_singleton_same_instance(self): + h1 = get_handler() + h2 = get_handler() + self.assertIs(h1, h2) + + def test_initialises_on_first_call(self): + h = get_handler() + self.assertTrue(h._initialised) + + +class TestErrorHandling(unittest.TestCase): + def setUp(self): + ApplicationErrorHandler.reset_singleton() + + def tearDown(self): + ApplicationErrorHandler.reset_singleton() + + def test_handle_returns_report(self): + try: + raise ValueError("test error") + except ValueError as exc: + report = handle(exc) + self.assertIsNotNone(report) + self.assertEqual(report.exception_type, "ValueError") + + def test_handle_with_context(self): + try: + raise RuntimeError("ctx test") + except RuntimeError as exc: + report = handle(exc, context={"operation": "test_op"}) + self.assertEqual(report.context.get("operation"), "test_op") + + def test_handle_reraise(self): + with self.assertRaises(ValueError): + try: + raise ValueError("reraise me") + except ValueError as exc: + handle(exc, reraise=True) + + def test_handle_critical_forces_severity(self): + handler = get_handler() + try: + raise ValueError("low but critical context") + except ValueError as exc: + report = handler.handle_critical(exc) + self.assertEqual(report.severity, ErrorSeverity.CRITICAL) + + def test_handle_critical_updates_error_counts(self): + """get_critical_errors must see the escalated severity.""" + handler = get_handler() + try: + raise ValueError("escalated") + except ValueError as exc: + handler.handle_critical(exc) + criticals = handler.get_critical_errors() + self.assertEqual(len(criticals), 1) + self.assertEqual(criticals[0].severity, ErrorSeverity.CRITICAL) + + def test_powertrader_error_preserved_severity(self): + try: + raise TradingError("market closed", severity=ErrorSeverity.HIGH) + except TradingError as exc: + report = handle(exc) + self.assertEqual(report.severity, ErrorSeverity.HIGH) + self.assertEqual(report.category, ErrorCategory.TRADING_ERROR) + + +class TestCallbacks(unittest.TestCase): + def setUp(self): + ApplicationErrorHandler.reset_singleton() + + def tearDown(self): + ApplicationErrorHandler.reset_singleton() + + def test_critical_callback_fired(self): + cb = MagicMock() + on_critical(cb) + try: + raise RuntimeError("critical!") + except RuntimeError as exc: + get_handler().handle_critical(exc) + cb.assert_called_once() + + def test_callback_receives_report(self): + received = [] + on_error(lambda r: received.append(r)) + try: + raise TradingError("order failed", severity=ErrorSeverity.HIGH) + except TradingError as exc: + handle(exc) + self.assertEqual(len(received), 1) + self.assertEqual(received[0].exception_type, "TradingError") + + def test_warning_callback_fired_for_medium_error(self): + """MEDIUM-severity TradingError must trigger on_warning callbacks.""" + cb = MagicMock() + on_warning(cb) + try: + raise TradingError("minor issue", severity=ErrorSeverity.MEDIUM) + except TradingError as exc: + handle(exc) + cb.assert_called_once() + self.assertEqual(cb.call_args[0][0].severity, ErrorSeverity.MEDIUM) + + def test_callback_exception_does_not_propagate(self): + def bad_cb(report): + raise RuntimeError("callback exploded") + + on_critical(bad_cb) + try: + get_handler().handle_critical(ValueError("test")) + except RuntimeError: + self.fail("Callback exception should not propagate") + + def test_unregister_all_specific_severity(self): + cb = MagicMock() + on_critical(cb) + get_handler().unregister_all(ErrorSeverity.CRITICAL) + get_handler().handle_critical(ValueError("test")) + cb.assert_not_called() + + def test_unregister_all_clears_every_severity(self): + cb_crit = MagicMock() + cb_high = MagicMock() + on_critical(cb_crit) + on_error(cb_high) + get_handler().unregister_all() # no argument → all + get_handler().handle_critical(ValueError("x")) + try: + raise TradingError("y", severity=ErrorSeverity.HIGH) + except TradingError as exc: + handle(exc) + cb_crit.assert_not_called() + cb_high.assert_not_called() + + def test_suppress_module_blocks_callbacks(self): + """Callbacks must NOT fire when the error originates from a suppressed module.""" + cb = MagicMock() + on_critical(cb) + get_handler().suppress_module("pt_trader") + + # ErrorHandler captures the test file's frame as report.module, so + # patch handle_error to override the module to simulate origin in + # the suppressed source file. Use patch.object so the override is + # restored via addCleanup regardless of test outcome (no manual + # try/finally, no leakage to other tests in this class). + original = get_handler()._handler.handle_error + + def patched(error, context=None, severity_override=None): + report = original( + error, context=context, severity_override=severity_override + ) + report.module = "pt_trader.py" # filename form, suppression normalizes + return report + + patcher = patch.object( + get_handler()._handler, "handle_error", side_effect=patched + ) + patcher.start() + self.addCleanup(patcher.stop) + + try: + raise ValueError("from suppressed module") + except ValueError as exc: + get_handler().handle_critical(exc) + + cb.assert_not_called() + + def test_suppress_module_accepts_filename_and_module_name(self): + """Suppression normalizes — passing either 'pt_trader' or 'pt_trader.py' + suppresses errors reported as either form.""" + h = get_handler() + # Both spellings collapse to the same suppressed entry + h.suppress_module("pt_trader") + h.suppress_module("pt_trader.py") + self.assertTrue(h._is_module_suppressed("pt_trader")) + self.assertTrue(h._is_module_suppressed("pt_trader.py")) + h.unsuppress_module("pt_trader.py") + self.assertFalse(h._is_module_suppressed("pt_trader")) + + +class TestQueryAPI(unittest.TestCase): + def setUp(self): + ApplicationErrorHandler.reset_singleton() + + def tearDown(self): + ApplicationErrorHandler.reset_singleton() + + def test_get_summary_empty(self): + summary = get_handler().get_summary() + self.assertEqual(summary["total_errors"], 0) + + def test_get_recent_errors(self): + for i in range(5): + try: + raise ValueError(f"error {i}") + except ValueError as exc: + handle(exc) + recent = get_handler().get_recent_errors(limit=3) + self.assertEqual(len(recent), 3) + + def test_get_recent_errors_non_positive_limit(self): + """limit <= 0 returns [] rather than a wrong negative-slice subset.""" + for i in range(5): + try: + raise ValueError(f"error {i}") + except ValueError as exc: + handle(exc) + self.assertEqual(get_handler().get_recent_errors(limit=0), []) + self.assertEqual(get_handler().get_recent_errors(limit=-3), []) + + def test_get_critical_errors_filtered(self): + try: + raise ValueError("normal") + except ValueError as exc: + handle(exc) + try: + raise TradingError("critical trade fail") + except TradingError as exc: + get_handler().handle_critical(exc) + criticals = get_handler().get_critical_errors() + self.assertEqual(len(criticals), 1) + + def test_clear_history(self): + try: + raise ValueError("to clear") + except ValueError as exc: + handle(exc) + get_handler().clear_history() + self.assertEqual(len(get_handler().get_recent_errors()), 0) + + +class TestConfigureGuiAlerts(unittest.TestCase): + def setUp(self): + ApplicationErrorHandler.reset_singleton() + + def tearDown(self): + ApplicationErrorHandler.reset_singleton() + + def test_configure_gui_alerts_registers_critical(self): + critical_cb = MagicMock() + error_cb = MagicMock() + configure_gui_alerts(critical_callback=critical_cb, error_callback=error_cb) + get_handler().handle_critical(ValueError("gui test")) + critical_cb.assert_called_once() + error_cb.assert_not_called() + + def test_configure_gui_alerts_registers_error(self): + error_cb = MagicMock() + configure_gui_alerts(error_callback=error_cb) + try: + raise TradingError("high err", severity=ErrorSeverity.HIGH) + except TradingError as exc: + handle(exc) + error_cb.assert_called_once() + + def test_configure_gui_alerts_none_callbacks_safe(self): + """Passing None should not raise.""" + configure_gui_alerts(critical_callback=None, error_callback=None) + + +if __name__ == "__main__": + unittest.main()