diff --git a/tests/test_alpha_vantage_hardening.py b/tests/test_alpha_vantage_hardening.py new file mode 100644 index 000000000..653c73351 --- /dev/null +++ b/tests/test_alpha_vantage_hardening.py @@ -0,0 +1,54 @@ +"""Alpha Vantage request hardening. + +Regressions for #990 (no request timeout -> can hang) and #991 (invalid-key +responses mislabeled as rate limits and silently treated as transient). +""" +import pytest + +import tradingagents.dataflows.alpha_vantage_common as av + + +class _FakeResponse: + def __init__(self, text): + self.text = text + + def raise_for_status(self): + pass + + +def _patched_get(body, capture=None): + def fake_get(url, params=None, **kwargs): + if capture is not None: + capture.update(kwargs) + return _FakeResponse(body) + return fake_get + + +@pytest.mark.unit +def test_request_passes_timeout(monkeypatch): + captured = {} + monkeypatch.setattr(av.requests, "get", _patched_get("Date,Close\n2025-01-02,1.0", captured)) + av._make_api_request("TIME_SERIES_DAILY", {"symbol": "AAPL"}) + assert captured.get("timeout") == av.REQUEST_TIMEOUT # #990 + + +@pytest.mark.unit +def test_rate_limit_detected(monkeypatch): + body = '{"Information": "Our standard API rate limit is 25 requests per day. ... your API key ..."}' + monkeypatch.setattr(av.requests, "get", _patched_get(body)) + with pytest.raises(av.AlphaVantageRateLimitError): + av._make_api_request("TIME_SERIES_DAILY", {"symbol": "AAPL"}) + + +@pytest.mark.unit +def test_invalid_key_not_mislabeled_as_rate_limit(monkeypatch): + # AV's invalid-key notice mentions "API key"; it must NOT be treated as a + # (transient) rate limit, but surface as a real configuration error (#991). + body = ('{"Information": "the parameter apikey is invalid or missing. ' + 'Please claim your free API key on (https://www.alphavantage.co/support/#api-key)."}') + monkeypatch.setattr(av.requests, "get", _patched_get(body)) + with pytest.raises(av.AlphaVantageNotConfiguredError): + av._make_api_request("TIME_SERIES_DAILY", {"symbol": "AAPL"}) + with pytest.raises(av.AlphaVantageRateLimitError): # sanity: rate-limit path still distinct + monkeypatch.setattr(av.requests, "get", _patched_get('{"Note": "API call frequency is 5 calls per minute."}')) + av._make_api_request("TIME_SERIES_DAILY", {"symbol": "AAPL"}) diff --git a/tradingagents/dataflows/alpha_vantage_common.py b/tradingagents/dataflows/alpha_vantage_common.py index b21fa4246..f8898f3ef 100644 --- a/tradingagents/dataflows/alpha_vantage_common.py +++ b/tradingagents/dataflows/alpha_vantage_common.py @@ -7,6 +7,10 @@ from io import StringIO API_BASE_URL = "https://www.alphavantage.co/query" +# Network timeout (seconds) so a stalled Alpha Vantage request can't hang the +# CLI/agents indefinitely (#990). +REQUEST_TIMEOUT = 30 + class AlphaVantageNotConfiguredError(ValueError): """Raised when Alpha Vantage is selected but no API key is configured. @@ -76,22 +80,31 @@ def _make_api_request(function_name: str, params: dict) -> dict | str: # Remove entitlement if it's None or empty api_params.pop("entitlement", None) - response = requests.get(API_BASE_URL, params=api_params) + response = requests.get(API_BASE_URL, params=api_params, timeout=REQUEST_TIMEOUT) response.raise_for_status() response_text = response.text - - # Check if response is JSON (error responses are typically JSON) + + # Error responses are JSON; data responses are usually CSV (or data-keyed + # JSON). A non-JSON body is normal data. try: response_json = json.loads(response_text) - # Check for rate limit error - if "Information" in response_json: - info_message = response_json["Information"] - if "rate limit" in info_message.lower() or "api key" in info_message.lower(): - raise AlphaVantageRateLimitError(f"Alpha Vantage rate limit exceeded: {info_message}") except json.JSONDecodeError: - # Response is not JSON (likely CSV data), which is normal - pass + return response_text + + # Alpha Vantage reports problems via "Information" / "Note". Classify so a + # genuine rate limit and an invalid/missing key aren't conflated (#991): + # rate-limit phrasing is checked first because those notices also mention + # "API key" ("your API key ... 25 requests per day"). + notice = response_json.get("Information") or response_json.get("Note") + if notice: + low = notice.lower() + if any(m in low for m in ("rate limit", "requests per day", "call frequency", "premium")): + raise AlphaVantageRateLimitError(f"Alpha Vantage rate limit exceeded: {notice}") + if "api key" in low or "apikey" in low: + # Reuse the existing "not configured" error so a bad key surfaces as + # a real, actionable failure rather than a mislabeled rate limit (#991). + raise AlphaVantageNotConfiguredError(f"Alpha Vantage API key invalid or missing: {notice}") return response_text