Merge pull request 'fix: add token refresh cooldown to prevent EGW00133 cascading failures (issue #54)' (#55) from feature/issue-54-token-refresh-cooldown into main

Reviewed-on: #55

src/analysis/scanner.py

@@ -76,6 +76,10 @@ class MarketScanner:
             if market.is_domestic:
                 orderbook = await self.broker.get_orderbook(stock_code)
             else:
+                # Rate limiting: Add 200ms delay for overseas API calls
+                # to prevent hitting KIS API rate limit (EGW00201)
+                await asyncio.sleep(0.2)
+
                 # For overseas, we need to adapt the price data structure
                 price_data = await self.overseas_broker.get_overseas_price(
                     market.exchange_code, stock_code
@@ -83,8 +87,8 @@ class MarketScanner:
                 # Convert to orderbook-like structure
                 orderbook = {
                     "output1": {
-                        "stck_prpr": price_data.get("output", {}).get("last", "0"),
-                        "acml_vol": price_data.get("output", {}).get("tvol", "0"),
+                        "stck_prpr": price_data.get("output", {}).get("last", "0") or "0",
+                        "acml_vol": price_data.get("output", {}).get("tvol", "0") or "0",
                     }
                 }
 

src/broker/kis_api.py

@@ -56,6 +56,8 @@ class KISBroker:
         self._access_token: str | None = None
         self._token_expires_at: float = 0.0
         self._token_lock = asyncio.Lock()
+        self._last_refresh_attempt: float = 0.0
+        self._refresh_cooldown: float = 60.0  # Seconds (matches KIS 1/minute limit)
         self._rate_limiter = LeakyBucket(settings.RATE_LIMIT_RPS)
 
     def _get_session(self) -> aiohttp.ClientSession:
@@ -98,7 +100,19 @@ class KISBroker:
             if self._access_token and now < self._token_expires_at:
                 return self._access_token
 
+            # Check cooldown period (prevents hitting EGW00133: 1/minute limit)
+            time_since_last_attempt = now - self._last_refresh_attempt
+            if time_since_last_attempt < self._refresh_cooldown:
+                remaining = self._refresh_cooldown - time_since_last_attempt
+                error_msg = (
+                    f"Token refresh on cooldown. "
+                    f"Retry in {remaining:.1f}s (KIS allows 1/minute)"
+                )
+                logger.warning(error_msg)
+                raise ConnectionError(error_msg)
+
             logger.info("Refreshing KIS access token")
+            self._last_refresh_attempt = now
             session = self._get_session()
             url = f"{self._base_url}/oauth2/tokenP"
             body = {

tests/test_broker.py

@@ -89,6 +89,70 @@ class TestTokenManagement:
 
         await broker.close()
 
+    @pytest.mark.asyncio
+    async def test_token_refresh_cooldown_prevents_rapid_retries(self, settings):
+        """Token refresh should enforce cooldown after failure (issue #54)."""
+        broker = KISBroker(settings)
+        broker._refresh_cooldown = 2.0  # Short cooldown for testing
+
+        # First refresh attempt fails with 403 (EGW00133)
+        mock_resp_403 = AsyncMock()
+        mock_resp_403.status = 403
+        mock_resp_403.text = AsyncMock(
+            return_value='{"error_code":"EGW00133","error_description":"접근토큰 발급 잠시 후 다시 시도하세요(1분당 1회)"}'
+        )
+        mock_resp_403.__aenter__ = AsyncMock(return_value=mock_resp_403)
+        mock_resp_403.__aexit__ = AsyncMock(return_value=False)
+
+        with patch("aiohttp.ClientSession.post", return_value=mock_resp_403):
+            # First attempt should fail with 403
+            with pytest.raises(ConnectionError, match="Token refresh failed"):
+                await broker._ensure_token()
+
+            # Second attempt within cooldown should fail with cooldown error
+            with pytest.raises(ConnectionError, match="Token refresh on cooldown"):
+                await broker._ensure_token()
+
+        await broker.close()
+
+    @pytest.mark.asyncio
+    async def test_token_refresh_allowed_after_cooldown(self, settings):
+        """Token refresh should be allowed after cooldown period expires."""
+        broker = KISBroker(settings)
+        broker._refresh_cooldown = 0.1  # Very short cooldown for testing
+
+        # First attempt fails
+        mock_resp_403 = AsyncMock()
+        mock_resp_403.status = 403
+        mock_resp_403.text = AsyncMock(return_value='{"error_code":"EGW00133"}')
+        mock_resp_403.__aenter__ = AsyncMock(return_value=mock_resp_403)
+        mock_resp_403.__aexit__ = AsyncMock(return_value=False)
+
+        # Second attempt succeeds
+        mock_resp_200 = AsyncMock()
+        mock_resp_200.status = 200
+        mock_resp_200.json = AsyncMock(
+            return_value={
+                "access_token": "tok_after_cooldown",
+                "expires_in": 86400,
+            }
+        )
+        mock_resp_200.__aenter__ = AsyncMock(return_value=mock_resp_200)
+        mock_resp_200.__aexit__ = AsyncMock(return_value=False)
+
+        with patch("aiohttp.ClientSession.post", return_value=mock_resp_403):
+            with pytest.raises(ConnectionError, match="Token refresh failed"):
+                await broker._ensure_token()
+
+        # Wait for cooldown to expire
+        await asyncio.sleep(0.15)
+
+        with patch("aiohttp.ClientSession.post", return_value=mock_resp_200):
+            token = await broker._ensure_token()
+            assert token == "tok_after_cooldown"
+
+        await broker.close()
+
 
 # ---------------------------------------------------------------------------
 # Network Error Handling

tests/test_main.py

@@ -430,6 +430,26 @@ class TestOverseasBalanceParsing:
         broker.get_overseas_balance = AsyncMock(return_value={"output2": []})
         return broker
 
+    @pytest.fixture
+    def mock_overseas_broker_with_empty_price(self) -> MagicMock:
+        """Create mock overseas broker returning empty string for price."""
+        broker = MagicMock()
+        broker.get_overseas_price = AsyncMock(
+            return_value={"output": {"last": ""}}  # Empty string
+        )
+        broker.get_overseas_balance = AsyncMock(
+            return_value={
+                "output2": [
+                    {
+                        "frcr_evlu_tota": "10000.00",
+                        "frcr_dncl_amt_2": "5000.00",
+                        "frcr_buy_amt_smtl": "4500.00",
+                    }
+                ]
+            }
+        )
+        return broker
+
     @pytest.fixture
     def mock_domestic_broker(self) -> MagicMock:
         """Create minimal mock domestic broker."""
@@ -595,3 +615,37 @@ class TestOverseasBalanceParsing:
 
         # Verify balance API was called
         mock_overseas_broker_with_empty.get_overseas_balance.assert_called_once()
+
+    @pytest.mark.asyncio
+    async def test_overseas_price_empty_string(
+        self,
+        mock_domestic_broker: MagicMock,
+        mock_overseas_broker_with_empty_price: MagicMock,
+        mock_brain_hold: MagicMock,
+        mock_risk: MagicMock,
+        mock_db: MagicMock,
+        mock_decision_logger: MagicMock,
+        mock_context_store: MagicMock,
+        mock_criticality_assessor: MagicMock,
+        mock_telegram: MagicMock,
+        mock_overseas_market: MagicMock,
+    ) -> None:
+        """Test overseas price parsing with empty string (issue #49)."""
+        with patch("src.main.log_trade"):
+            # Should not raise ValueError, should default to 0.0
+            await trading_cycle(
+                broker=mock_domestic_broker,
+                overseas_broker=mock_overseas_broker_with_empty_price,
+                brain=mock_brain_hold,
+                risk=mock_risk,
+                db_conn=mock_db,
+                decision_logger=mock_decision_logger,
+                context_store=mock_context_store,
+                criticality_assessor=mock_criticality_assessor,
+                telegram=mock_telegram,
+                market=mock_overseas_market,
+                stock_code="AAPL",
+            )
+
+        # Verify price API was called
+        mock_overseas_broker_with_empty_price.get_overseas_price.assert_called_once()

tests/test_volatility.py

@@ -338,6 +338,28 @@ class TestMarketScanner:
         assert metrics.stock_code == "AAPL"
         assert metrics.current_price == 150.50
 
+    @pytest.mark.asyncio
+    async def test_scan_stock_overseas_empty_price(
+        self,
+        scanner: MarketScanner,
+        mock_overseas_broker: OverseasBroker,
+        context_store: ContextStore,
+    ) -> None:
+        """Test scanning overseas stock with empty price string (issue #49)."""
+        mock_overseas_broker.get_overseas_price.return_value = {
+            "output": {
+                "last": "",  # Empty string
+                "tvol": "",  # Empty string
+            }
+        }
+
+        market = MARKETS["US_NASDAQ"]
+        metrics = await scanner.scan_stock("AAPL", market)
+
+        assert metrics is not None
+        assert metrics.stock_code == "AAPL"
+        assert metrics.current_price == 0.0  # Should default to 0.0
+
     @pytest.mark.asyncio
     async def test_scan_stock_error_handling(
         self,