Merge pull request #2550 from jlowin/deflake-integration-tests-2

Improve rate limit detection for integration tests

Author: chrisguidry

.github/workflows/run-tests.yml

@@ -99,8 +99,8 @@ jobs:
         run: uv sync --upgrade
 
       - name: Run integration tests
-        # use longer per-test timeout than the default 3s
-        run: uv run pytest tests -m "integration" --timeout=15 --numprocesses auto --maxprocesses 2 --dist worksteal
+        # use longer per-test timeout for remote API calls (default is 5s)
+        run: uv run pytest tests -m "integration" --timeout=30 --numprocesses auto --maxprocesses 2 --dist worksteal
         env:
           FASTMCP_GITHUB_TOKEN: ${{ secrets.FASTMCP_GITHUB_TOKEN }}
           FASTMCP_TEST_AUTH_GITHUB_CLIENT_ID: ${{ secrets.FASTMCP_TEST_AUTH_GITHUB_CLIENT_ID }}

src/fastmcp/client/client.py

@@ -374,7 +374,11 @@ async def __aenter__(self):
         return await self._connect()
 
     async def __aexit__(self, exc_type, exc_val, exc_tb):
-        await self._disconnect()
+        # Use a timeout to prevent hanging during cleanup if the connection is in a bad
+        # state (e.g., rate-limited). The MCP SDK's transport may try to terminate the
+        # session which can hang if the server is unresponsive.
+        with anyio.move_on_after(5):
+            await self._disconnect()
 
     async def _connect(self):
         """

tests/integration_tests/conftest.py

@@ -3,8 +3,13 @@
 import pytest
 
 
-def _is_rate_limit_error(excinfo) -> bool:
-    """Check if an exception indicates a rate limit error from GitHub API."""
+def _is_rate_limit_error(excinfo, report=None) -> bool:
+    """Check if an exception indicates a rate limit error from GitHub API.
+
+    Args:
+        excinfo: The exception info from pytest
+        report: Optional test report for additional context (captured output, longrepr)
+    """
     if excinfo is None:
         return False
 
@@ -28,6 +33,20 @@ def _is_rate_limit_error(excinfo) -> bool:
     if "429" in exc_str or "rate limit" in exc_str or "too many requests" in exc_str:
         return True
 
+    # Timeout exceptions may indicate rate limiting when the 429 causes asyncio
+    # shutdown issues. Check if it's a timeout and look for 429 in the captured output.
+    if "timeout" in exc_type.lower() or "timeout" in exc_str:
+        # Check captured output for 429 indicators
+        if report is not None:
+            longrepr_str = str(report.longrepr).lower() if report.longrepr else ""
+            if "429" in longrepr_str or "too many requests" in longrepr_str:
+                return True
+
+            # Check captured stdout/stderr
+            for section_name, content in getattr(report, "sections", []):
+                if "429" in content or "too many requests" in content.lower():
+                    return True
+
     return False
 
 
@@ -43,7 +62,7 @@ def pytest_runtest_makereport(item, call):
         and report.failed
         and not hasattr(report, "wasxfail")
         and item.module.__name__ == "tests.integration_tests.test_github_mcp_remote"
-        and _is_rate_limit_error(call.excinfo)
+        and _is_rate_limit_error(call.excinfo, report)
     ):
         report.outcome = "skipped"
         report.longrepr = (