🛠️ Jules02: Resilience improvement — Hardened MT5Connector data retrieval and failure propagation (#1034)

* feat: 🛠️ Jules02: Resilience improvement — Hardened MT5Connector data retrieval

- Refactored `get_account_info`, `get_positions`, `get_terminal_status`, `get_symbol_properties`, and `find_symbols` to use logic/wrapper pattern.
- Applied `@with_retry` and `CircuitBreaker` to all data retrieval methods.
- Hardened `get_account_balance` to propagate exceptions instead of returning 0.0.
- Improved failure detection by raising `MT5DataError` when MT5 returns `None`.
- Added `tests/test_mt5_connector_resilience_v4.py` for verification.

Co-authored-by: xnessom <255288032+xnessom@users.noreply.github.com>

* feat: 🛠️ Jules02: Resilience improvement — Hardened MT5Connector data retrieval

- Refactored `get_account_info`, `get_positions`, `get_terminal_status`, `get_symbol_properties`, and `find_symbols` to use logic/wrapper pattern.
- Applied `@with_retry` and `CircuitBreaker` to all data retrieval methods.
- Hardened `get_account_balance` to propagate exceptions instead of returning 0.0.
- Improved failure detection by raising `MT5DataError` when MT5 returns `None`.
- Upgraded `fastapi` to `0.136.1` to resolve `starlette` security vulnerabilities (GHSA-2c2j-9gv5-cj73, GHSA-7f5h-v6xp-fcq8).
- Added `docs/resilience_hardening_v4.md` for documentation compliance.
- Added `tests/test_mt5_connector_resilience_v4.py` for verification.

Co-authored-by: xnessom <255288032+xnessom@users.noreply.github.com>

---------

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Co-authored-by: xnessom <255288032+xnessom@users.noreply.github.com>

docs/resilience_hardening_v4.md

@@ -0,0 +1,9 @@
+# Resilience Hardening v4
+
+This document describes the hardening of the MT5Connector data retrieval paths.
+
+## Key Changes
+- All primary data retrieval methods (account info, positions, etc.) now use a logic/wrapper pattern.
+- Exponential backoff retries and circuit breakers are applied to these paths.
+- Improved failure detection raises explicit MT5DataError when the SDK returns None.
+- Safe failure propagation in get_account_balance prevents dangerous zero-balance assumptions.

requirements-ci-no-talib.txt

@@ -19,7 +19,6 @@ redis==5.2.1
 # -- Pydantic / settings -------------------------------------------
 pydantic==2.10.6
 pydantic-settings==2.7.1
-starlette<0.46.0,>=0.40.0
 
 # -- Testing -------------------------------------------------------
 optuna==4.8.0
@@ -28,7 +27,7 @@ pytest-asyncio==1.3.0
 pytest-cov==6.0.0
 pytest-mock==3.14.0
 python-telegram-bot==21.10
-fastapi==0.115.8
+fastapi==0.136.1
 requests==2.33.1
 httpx==0.28.1
 structlog==25.1.0

requirements-ci.txt

@@ -28,7 +28,7 @@ pytest-asyncio==1.3.0
 pytest-cov==6.0.0
 pytest-mock==3.14.0
 python-telegram-bot==21.10
-fastapi==0.115.8
+fastapi==0.136.1
 requests==2.33.1
 httpx==0.28.1
 structlog==25.1.0

requirements-docker.txt

@@ -36,7 +36,6 @@ alembic==1.14.1
 redis==5.2.1
 
 # -- Monitoring & Logging ------------------------------------
-starlette<0.46.0,>=0.40.0
 prometheus-client==0.21.1
 structlog==25.1.0
 rich==13.9.4
@@ -44,7 +43,7 @@ tqdm==4.67.1
 
 # -- Dashboard / API -----------------------------------------
 jinja2==3.1.6
-fastapi==0.115.8
+fastapi==0.136.1
 uvicorn[standard]==0.34.0
 dash==2.18.2
 plotly==6.0.0

requirements-linux.txt

@@ -36,14 +36,13 @@ alembic==1.14.1
 redis==5.2.1
 
 # ── Monitoring & Logging ────────────────────────────────────
-starlette<0.46.0,>=0.40.0
 prometheus-client==0.21.1
 structlog==25.1.0
 rich==13.9.4
 tqdm==4.67.1
 
 # ── Dashboard / API ─────────────────────────────────────────
-fastapi==0.115.8
+fastapi==0.136.1
 uvicorn[standard]==0.34.0
 dash==2.18.2
 plotly==6.0.0

requirements.txt

@@ -47,7 +47,7 @@ tqdm==4.67.1
 
 # ── Dashboard / API ─────────────────────────────────────────
 jinja2==3.1.6
-fastapi==0.115.8
+fastapi==0.136.1
 uvicorn[standard]==0.34.0
 dash==2.18.2
 plotly==6.0.0

src/trading/mt5_connector.py

@@ -620,39 +620,80 @@ class MT5Connector:
         """Retrieve current account balance."""
         info = self.get_account_info()
         # MT5 standard field is 'balance', MetaAPI is 'balance'
-        return float(info.get("balance", 0.0))
+        # Hardened to use direct key access so exceptions in get_account_info propagate
+        # and we don't accidentally return 0.0 on hidden failure.
+        return float(info["balance"])
 
+    @with_retry((MT5DataError, MT5ConnectionError), max_retries=3)
     def get_account_info(self) -> Dict[str, Any]:
         """Retrieve account information."""
+        return self.breaker(self._get_account_info_logic)()
+
+    def _get_account_info_logic(self) -> Dict[str, Any]:
+        """Internal account information retrieval logic."""
         if not self._is_initialized:
-            return {}
+            self.initialize()
+
         if not self.use_metaapi:
             acc = mt5.account_info()
-            return acc._asdict() if acc else {}
+            if acc is None:
+                err_code, err_desc = mt5.last_error()
+                if err_code in [-1, 10001, 10002, 10003, 10004]:
+                    self._is_initialized = False
+                raise MT5DataError(f"Failed to get account info: {err_desc} (code: {err_code})")
+            return acc._asdict()
         else:
-            return self._run_async(self.metaapi_connection.get_account_information())
+            try:
+                return self._run_async(self.metaapi_connection.get_account_information())
+            except Exception as e:
+                self._is_initialized = False
+                raise MT5DataError(f"MetaAPI get_account_information failed: {e}") from e
 
+    @with_retry((MT5DataError, MT5ConnectionError), max_retries=3)
     def get_positions(self, symbol: Optional[str] = None) -> List[Dict[str, Any]]:
         """Retrieve open positions."""
+        return self.breaker(self._get_positions_logic)(symbol)
+
+    def _get_positions_logic(self, symbol: Optional[str] = None) -> List[Dict[str, Any]]:
+        """Internal positions retrieval logic."""
         if not self._is_initialized:
-            return []
+            self.initialize()
+
         if not self.use_metaapi:
             positions = mt5.positions_get(symbol=symbol) if symbol else mt5.positions_get()
-            return [p._asdict() for p in positions] if positions else []
+            if positions is None:
+                err_code, err_desc = mt5.last_error()
+                if err_code in [-1, 10001, 10002, 10003, 10004]:
+                    self._is_initialized = False
+                raise MT5DataError(f"Failed to get positions: {err_desc} (code: {err_code})")
+            return [p._asdict() for p in positions]
         else:
-            return self._run_async(self.metaapi_connection.get_positions())
+            try:
+                return self._run_async(self.metaapi_connection.get_positions())
+            except Exception as e:
+                self._is_initialized = False
+                raise MT5DataError(f"MetaAPI get_positions failed: {e}") from e
 
+    @with_retry((MT5DataError, MT5ConnectionError), max_retries=3)
     def get_terminal_status(self) -> Dict[str, Any]:
         """
         Retrieve terminal status (e.g., algo trading enabled).
         Ensures a consistent 'algo_trading' key is present.
         """
+        return self.breaker(self._get_terminal_status_logic)()
+
+    def _get_terminal_status_logic(self) -> Dict[str, Any]:
+        """Internal terminal status retrieval logic."""
         if not self._is_initialized:
-            return {"algo_trading": False}
+            self.initialize()
+
         if not self.use_metaapi:
             info = mt5.terminal_info()
             if not info:
-                return {"algo_trading": False}
+                err_code, err_desc = mt5.last_error()
+                if err_code in [-1, 10001, 10002, 10003, 10004]:
+                    self._is_initialized = False
+                raise MT5DataError(f"Failed to get terminal status: {err_desc} (code: {err_code})")
             data = info._asdict()
             # Map 'trade_allowed' (terminal-wide algo trading button) to 'algo_trading' for clarity
             if "trade_allowed" in data:
@@ -663,14 +704,23 @@ class MT5Connector:
             # but we assume it's true if we can connect and synchronize.
             return {"algo_trading": True}
 
+    @with_retry((MT5DataError, MT5ConnectionError), max_retries=3)
     def get_symbol_properties(self, symbol: str) -> Optional[Dict[str, Any]]:
         """Retrieve symbol properties."""
+        return self.breaker(self._get_symbol_properties_logic)(symbol)
+
+    def _get_symbol_properties_logic(self, symbol: str) -> Dict[str, Any]:
+        """Internal symbol properties retrieval logic."""
         if not self._is_initialized:
-            return None
+            self.initialize()
+
         if not self.use_metaapi:
             info = mt5.symbol_info(symbol)
             if not info:
-                return None
+                err_code, err_desc = mt5.last_error()
+                if err_code in [-1, 10001, 10002, 10003, 10004]:
+                    self._is_initialized = False
+                raise MT5DataError(f"Failed to get symbol info for {symbol}: {err_desc} (code: {err_code})")
             return {
                 "name": info.name,
                 "tradable": info.trade_mode != mt5.SYMBOL_TRADE_MODE_DISABLED,
@@ -684,23 +734,35 @@ class MT5Connector:
                 spec = self._run_async(self.metaapi_connection.get_symbol_specification(symbol))
                 return {
                     "name": spec["symbol"],
-                    "tradable": True,  # MetaAPI symbols are usually tradable if found
-                    "spread": 0,  # Not directly in spec
+                    "tradable": True,
+                    "spread": 0,
                     "digits": spec["digits"],
                     "point": spec.get("point"),
                     "pip_size": spec.get("pipSize"),
                     "trade_contract_size": spec.get("contractSize"),
                 }
-            except Exception:
-                return None
+            except Exception as e:
+                self._is_initialized = False
+                raise MT5DataError(f"MetaAPI get_symbol_specification failed for {symbol}: {e}") from e
 
+    @with_retry((MT5DataError, MT5ConnectionError), max_retries=3)
     def find_symbols(self, pattern: str) -> List[str]:
         """Find symbols matching a pattern."""
+        return self.breaker(self._find_symbols_logic)(pattern)
+
+    def _find_symbols_logic(self, pattern: str) -> List[str]:
+        """Internal symbols discovery logic."""
         if not self._is_initialized:
-            return []
+            self.initialize()
+
         if not self.use_metaapi:
             symbols = mt5.symbols_get(pattern)
-            return [s.name for s in symbols] if symbols else []
+            if symbols is None:
+                err_code, err_desc = mt5.last_error()
+                if err_code in [-1, 10001, 10002, 10003, 10004]:
+                    self._is_initialized = False
+                raise MT5DataError(f"Failed to find symbols with pattern {pattern}: {err_desc} (code: {err_code})")
+            return [s.name for s in symbols]
         else:
             # For MetaAPI, we'd need to fetch all and filter, which is slow.
             # Return empty or a simple guess.

tests/test_mt5_connector_resilience_v4.py

@@ -0,0 +1,165 @@
+
+import pytest
+from unittest.mock import MagicMock, patch
+from src.trading.mt5_connector import MT5Connector
+from src.core.exceptions import MT5DataError, MT5ConnectionError, CircuitBreakerError
+from src.core.resilience import CircuitState
+
+@pytest.fixture
+def mock_config():
+    cfg = MagicMock()
+    cfg.mode = "demo"
+    cfg.symbol = "XAUUSD"
+    cfg.mt5_path = ""
+    cfg.mt5_login = 12345
+    cfg.mt5_password.get_secret_value.return_value = "pass"
+    cfg.mt5_server = "server"
+    cfg.metaapi_token = None
+    return cfg
+
+@patch("src.trading.mt5_connector.mt5")
+@patch("src.trading.mt5_connector.MT5_AVAILABLE", True)
+def test_get_account_info_resilience(mock_mt5, mock_config):
+    connector = MT5Connector(mock_config)
+    connector._is_initialized = True
+
+    # Simulate 2 failures then 1 success
+    mock_acc = MagicMock()
+    mock_acc._asdict.return_value = {"balance": 1000.0}
+
+    mock_mt5.account_info.side_effect = [None, None, mock_acc]
+    mock_mt5.last_error.return_value = (10001, "Failed")
+    mock_mt5.initialize.return_value = True
+
+    # Override initialize to set state
+    def mock_init():
+        connector._is_initialized = True
+        return True
+
+    with patch.object(connector, 'initialize', side_effect=mock_init):
+        info = connector.get_account_info()
+        assert info["balance"] == 1000.0
+        assert mock_mt5.account_info.call_count == 3
+
+@patch("src.trading.mt5_connector.mt5")
+@patch("src.trading.mt5_connector.MT5_AVAILABLE", True)
+def test_get_positions_resilience(mock_mt5, mock_config):
+    connector = MT5Connector(mock_config)
+    connector._is_initialized = True
+
+    # Simulate 2 failures then 1 success
+    pos1 = MagicMock()
+    pos1._asdict.return_value = {"ticket": 1}
+
+    mock_mt5.positions_get.side_effect = [None, None, (pos1,)]
+    mock_mt5.last_error.return_value = (10001, "Failed")
+    mock_mt5.initialize.return_value = True
+
+    def mock_init():
+        connector._is_initialized = True
+        return True
+
+    with patch.object(connector, 'initialize', side_effect=mock_init):
+        positions = connector.get_positions()
+        assert len(positions) == 1
+        assert positions[0]["ticket"] == 1
+        assert mock_mt5.positions_get.call_count == 3
+
+@patch("src.trading.mt5_connector.mt5")
+@patch("src.trading.mt5_connector.MT5_AVAILABLE", True)
+def test_get_account_balance_failure_propagation(mock_mt5, mock_config):
+    connector = MT5Connector(mock_config)
+    connector._is_initialized = True
+
+    # All attempts fail
+    mock_mt5.account_info.return_value = None
+    mock_mt5.last_error.return_value = (10001, "Permanent failure")
+    mock_mt5.initialize.return_value = True
+
+    with pytest.raises(MT5DataError):
+        connector.get_account_balance()
+
+@patch("src.trading.mt5_connector.mt5")
+@patch("src.trading.mt5_connector.MT5_AVAILABLE", True)
+def test_circuit_breaker_tripping_on_account_info(mock_mt5, mock_config):
+    # Lower threshold for faster test
+    connector = MT5Connector(mock_config)
+    connector.breaker.failure_threshold = 2
+    connector._is_initialized = True
+
+    mock_mt5.account_info.return_value = None
+    mock_mt5.last_error.return_value = (10001, "Failed")
+    mock_mt5.initialize.return_value = False # Make initialize fail too
+
+    # with_retry will do 3 retries (total 4 attempts)
+    # Circuit breaker should trip after 2 failures.
+    # But wait, with_retry wraps the whole breaker call?
+    # Let's check MT5Connector implementation:
+    # @with_retry(...)
+    # def get_account_info(self):
+    #     return self.breaker(self._get_account_info_logic)()
+
+    # with_retry catches exceptions and sleeps then retries.
+    # The breaker wrapper is INSIDE with_retry.
+    # 1st attempt: breaker calls logic, logic fails, breaker records failure, raises MT5DataError.
+    # with_retry catches, retries.
+    # 2nd attempt: breaker calls logic, logic fails, breaker records failure, trips (OPEN), raises MT5DataError.
+    # with_retry catches, retries.
+    # 3rd attempt: breaker sees OPEN, raises CircuitBreakerError.
+    # with_retry does NOT catch CircuitBreakerError (it only catches MT5DataError, MT5ConnectionError).
+    # So CircuitBreakerError should propagate.
+
+    with pytest.raises(CircuitBreakerError):
+        connector.get_account_info()
+
+    assert connector.circuit_state == "OPEN"
+
+@patch("src.trading.mt5_connector.mt5")
+@patch("src.trading.mt5_connector.MT5_AVAILABLE", True)
+def test_get_terminal_status_resilience(mock_mt5, mock_config):
+    connector = MT5Connector(mock_config)
+    connector._is_initialized = True
+
+    mock_info = MagicMock()
+    mock_info._asdict.return_value = {"trade_allowed": True}
+
+    mock_mt5.terminal_info.side_effect = [None, mock_info]
+    mock_mt5.last_error.return_value = (10001, "Failed")
+    mock_mt5.initialize.return_value = True
+
+    def mock_init():
+        connector._is_initialized = True
+        return True
+
+    with patch.object(connector, 'initialize', side_effect=mock_init):
+        status = connector.get_terminal_status()
+        assert status["algo_trading"] is True
+        assert mock_mt5.terminal_info.call_count == 2
+
+@patch("src.trading.mt5_connector.mt5")
+@patch("src.trading.mt5_connector.MT5_AVAILABLE", True)
+def test_get_symbol_properties_resilience(mock_mt5, mock_config):
+    connector = MT5Connector(mock_config)
+    connector._is_initialized = True
+
+    mock_symbol = MagicMock()
+    mock_symbol.name = "XAUUSD"
+    mock_symbol.trade_mode = 0 # SYMBOL_TRADE_MODE_FULL
+    mock_symbol.spread = 10
+    mock_symbol.digits = 3
+    mock_symbol.point = 0.001
+    mock_symbol.trade_contract_size = 100
+
+    mock_mt5.symbol_info.side_effect = [None, mock_symbol]
+    mock_mt5.last_error.return_value = (10001, "Failed")
+    mock_mt5.initialize.return_value = True
+    mock_mt5.SYMBOL_TRADE_MODE_DISABLED = 1
+
+    def mock_init():
+        connector._is_initialized = True
+        return True
+
+    with patch.object(connector, 'initialize', side_effect=mock_init):
+        props = connector.get_symbol_properties("XAUUSD")
+        assert props["name"] == "XAUUSD"
+        assert mock_mt5.symbol_info.call_count == 2