Commit
·
e6f0fda
1
Parent(s):
c7f5590
fix: PubMed JSON parsing (SPEC-20)
Browse filesMoves JSON parsing inside try/except block to handle API
maintenance pages gracefully. Adds JSONDecodeError handling.
- Add `import json` at top of file
- Move `search_resp.json()` inside try block (line 84)
- Add `except json.JSONDecodeError` handler with warning log
- Return empty list on invalid JSON (graceful degradation)
- Add unit test for maintenance page scenario
Fixes: production crash on PubMed maintenance pages
- src/tools/pubmed.py +9 -1
- tests/unit/tools/test_pubmed.py +23 -0
src/tools/pubmed.py
CHANGED
|
@@ -1,5 +1,6 @@
|
|
| 1 |
"""PubMed search tool using NCBI E-utilities."""
|
| 2 |
|
|
|
|
| 3 |
from typing import Any
|
| 4 |
|
| 5 |
import httpx
|
|
@@ -80,12 +81,19 @@ class PubMedTool:
|
|
| 80 |
params=search_params,
|
| 81 |
)
|
| 82 |
search_resp.raise_for_status()
|
|
|
|
| 83 |
except httpx.HTTPStatusError as e:
|
| 84 |
if e.response.status_code == self.HTTP_TOO_MANY_REQUESTS:
|
| 85 |
raise RateLimitError("PubMed rate limit exceeded") from e
|
| 86 |
raise SearchError(f"PubMed search failed: {e}") from e
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 87 |
|
| 88 |
-
search_data = search_resp.json()
|
| 89 |
pmids = search_data.get("esearchresult", {}).get("idlist", [])
|
| 90 |
|
| 91 |
if not pmids:
|
|
|
|
| 1 |
"""PubMed search tool using NCBI E-utilities."""
|
| 2 |
|
| 3 |
+
import json
|
| 4 |
from typing import Any
|
| 5 |
|
| 6 |
import httpx
|
|
|
|
| 81 |
params=search_params,
|
| 82 |
)
|
| 83 |
search_resp.raise_for_status()
|
| 84 |
+
search_data = search_resp.json()
|
| 85 |
except httpx.HTTPStatusError as e:
|
| 86 |
if e.response.status_code == self.HTTP_TOO_MANY_REQUESTS:
|
| 87 |
raise RateLimitError("PubMed rate limit exceeded") from e
|
| 88 |
raise SearchError(f"PubMed search failed: {e}") from e
|
| 89 |
+
except json.JSONDecodeError as e:
|
| 90 |
+
logger.warning(
|
| 91 |
+
"PubMed returned invalid JSON (possible maintenance page)",
|
| 92 |
+
error=str(e),
|
| 93 |
+
response_preview=search_resp.text[:200] if search_resp else "N/A",
|
| 94 |
+
)
|
| 95 |
+
return []
|
| 96 |
|
|
|
|
| 97 |
pmids = search_data.get("esearchresult", {}).get("idlist", [])
|
| 98 |
|
| 99 |
if not pmids:
|
tests/unit/tools/test_pubmed.py
CHANGED
|
@@ -1,5 +1,6 @@
|
|
| 1 |
"""Unit tests for PubMed tool."""
|
| 2 |
|
|
|
|
| 3 |
from unittest.mock import AsyncMock, MagicMock
|
| 4 |
|
| 5 |
import pytest
|
|
@@ -150,3 +151,25 @@ class TestPubMedTool:
|
|
| 150 |
assert "help" not in term.lower()
|
| 151 |
# "low libido" should be expanded
|
| 152 |
assert "HSDD" in term or "hypoactive" in term
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
"""Unit tests for PubMed tool."""
|
| 2 |
|
| 3 |
+
import json
|
| 4 |
from unittest.mock import AsyncMock, MagicMock
|
| 5 |
|
| 6 |
import pytest
|
|
|
|
| 151 |
assert "help" not in term.lower()
|
| 152 |
# "low libido" should be expanded
|
| 153 |
assert "HSDD" in term or "hypoactive" in term
|
| 154 |
+
|
| 155 |
+
@pytest.mark.asyncio
|
| 156 |
+
async def test_search_handles_maintenance_page(self, mocker):
|
| 157 |
+
"""PubMedTool should gracefully handle non-JSON responses (maintenance pages)."""
|
| 158 |
+
# Mock response that returns HTML instead of JSON (maintenance page)
|
| 159 |
+
mock_response = MagicMock()
|
| 160 |
+
mock_response.status_code = 200
|
| 161 |
+
mock_response.text = "<html><body>Service Temporarily Unavailable</body></html>"
|
| 162 |
+
mock_response.json.side_effect = json.JSONDecodeError("Expecting value", "", 0)
|
| 163 |
+
mock_response.raise_for_status = MagicMock()
|
| 164 |
+
|
| 165 |
+
mock_client = AsyncMock()
|
| 166 |
+
mock_client.get = AsyncMock(return_value=mock_response)
|
| 167 |
+
mock_client.__aenter__ = AsyncMock(return_value=mock_client)
|
| 168 |
+
mock_client.__aexit__ = AsyncMock(return_value=None)
|
| 169 |
+
|
| 170 |
+
mocker.patch("httpx.AsyncClient", return_value=mock_client)
|
| 171 |
+
|
| 172 |
+
tool = PubMedTool()
|
| 173 |
+
# Should return empty list, not crash
|
| 174 |
+
results = await tool.search("test query")
|
| 175 |
+
assert results == []
|