From 5312271f2b264f2cb7216af39506b131f15980a4 Mon Sep 17 00:00:00 2001 From: yetuge <182123929+yetuge@users.noreply.github.com> Date: Tue, 29 Sep 2026 21:00:36 +0800 Subject: [PATCH] fix(community): close the per-call Firecrawl client's async HTTP pool (#6013) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(community): close the per-call Firecrawl client's async HTTP pool The firecrawl tools build a fresh AsyncFirecrawlApp for every web_search and web_fetch call. The migration to the async client (#5786) kept the per-call construction, but AsyncFirecrawlApp eagerly creates an httpx.AsyncClient-backed pool in its constructor and exposes no public teardown, so each tool call abandons one unclosed pool. The sibling providers either scope their httpx clients with 'async with' or close them in a finally (tavily); firecrawl now does the same via a best-effort teardown that tolerates SDK shapes without the v2 client. Signed-off-by: yetuge <2219677952@qq.com> * fix(firecrawl): make per-call pool teardown resilient Signed-off-by: Willem Jiang * test(firecrawl): skip the real-SDK teardown test when the private layout differs The real-SDK test hard-asserted the locked firecrawl-py 4.23.0 private layout (_v2_client.async_http_client._client.is_closed), so an environment resolving another version in the declared >=1.15.0 range died with AttributeError instead of skipping — while the helper (and test_teardown_survives_a_client_without_v2_attributes) deliberately treat a missing or reshaped layout as a no-op. Guard on the same attributes and skip when they are absent. Signed-off-by: yetuge <2219677952@qq.com> --------- Signed-off-by: yetuge <2219677952@qq.com> Signed-off-by: Willem Jiang Co-authored-by: Willem Jiang --- .../deerflow/community/firecrawl/tools.py | 36 ++++ backend/tests/test_firecrawl_tools.py | 163 +++++++++++++++++- 2 files changed, 198 insertions(+), 1 deletion(-) diff --git a/backend/packages/harness/deerflow/community/firecrawl/tools.py b/backend/packages/harness/deerflow/community/firecrawl/tools.py index c7af25147c..ad969b1c6d 100644 --- a/backend/packages/harness/deerflow/community/firecrawl/tools.py +++ b/backend/packages/harness/deerflow/community/firecrawl/tools.py @@ -1,10 +1,38 @@ +import inspect import json +import logging from firecrawl import AsyncFirecrawlApp from langchain.tools import tool from deerflow.config import get_app_config +logger = logging.getLogger(__name__) + + +async def _aclose_firecrawl_client(client: AsyncFirecrawlApp) -> None: + """Best-effort close of the pooled async HTTP client a per-call app constructed. + + ``AsyncFirecrawlApp`` eagerly builds an ``httpx.AsyncClient``-backed pool in + its constructor and exposes no public teardown, so reach it through the + delegating v2 client. The declared dependency range (``firecrawl-py>=1.15.0``) + includes versions without that attribute, and teardown runs from a + ``finally`` — absence or failure here must never mask the tool's own result. + """ + pooled = getattr(getattr(client, "_v2_client", None), "async_http_client", None) + if pooled is None: + return + try: + close = getattr(pooled, "close", None) + if not callable(close): + logger.warning("Firecrawl async HTTP pool has no close method") + return + result = close() + if inspect.isawaitable(result): + await result + except Exception: + logger.warning("Failed to close the Firecrawl async HTTP pool", exc_info=True) + def _get_firecrawl_client(tool_name: str = "web_search") -> AsyncFirecrawlApp: config = get_app_config().get_tool_config(tool_name) @@ -28,6 +56,7 @@ async def web_search_tool(query: str) -> str: Args: query: The query to search for. """ + client: AsyncFirecrawlApp | None = None try: config = get_app_config().get_tool_config("web_search") max_results = 5 @@ -51,6 +80,9 @@ async def web_search_tool(query: str) -> str: return json_results except Exception as e: return f"Error: {str(e)}" + finally: + if client is not None: + await _aclose_firecrawl_client(client) @tool("web_fetch", parse_docstring=True) @@ -64,6 +96,7 @@ async def web_fetch_tool(url: str) -> str: Args: url: The URL to fetch the contents of. """ + client: AsyncFirecrawlApp | None = None try: client = _get_firecrawl_client("web_fetch") result = await client.scrape(url, formats=["markdown"]) @@ -76,5 +109,8 @@ async def web_fetch_tool(url: str) -> str: return "Error: No content found" except Exception as e: return f"Error: {str(e)}" + finally: + if client is not None: + await _aclose_firecrawl_client(client) return f"# {title}\n\n{markdown_content[:4096]}" diff --git a/backend/tests/test_firecrawl_tools.py b/backend/tests/test_firecrawl_tools.py index eb9deb6f86..99afefacc4 100644 --- a/backend/tests/test_firecrawl_tools.py +++ b/backend/tests/test_firecrawl_tools.py @@ -1,7 +1,9 @@ """Unit tests for the Firecrawl community tools.""" import json -from unittest.mock import AsyncMock, MagicMock, patch +import logging +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock, Mock, patch import pytest @@ -118,3 +120,162 @@ class TestFirecrawlBaseUrl: await web_search_tool.ainvoke({"query": "test query"}) mock_firecrawl_cls.assert_called_once_with(api_key="firecrawl-key", api_url="http://192.168.0.47:3002") + + +class TestPerCallClientTeardown: + """Every tool call builds a fresh client; its pooled async HTTP client must be closed.""" + + @patch("deerflow.community.firecrawl.tools.AsyncFirecrawlApp") + @patch("deerflow.community.firecrawl.tools.get_app_config") + @pytest.mark.anyio + async def test_search_closes_the_per_call_client(self, mock_get_app_config, mock_firecrawl_cls): + search_config = MagicMock() + search_config.model_extra = {"api_key": "firecrawl-search-key"} + mock_get_app_config.return_value.get_tool_config.return_value = search_config + + mock_result = MagicMock() + mock_result.web = [] + mock_firecrawl_cls.return_value.search = AsyncMock(return_value=mock_result) + mock_firecrawl_cls.return_value._v2_client.async_http_client.close = AsyncMock() + + from deerflow.community.firecrawl.tools import web_search_tool + + await web_search_tool.ainvoke({"query": "test query"}) + + mock_firecrawl_cls.return_value._v2_client.async_http_client.close.assert_awaited_once() + + @patch("deerflow.community.firecrawl.tools.AsyncFirecrawlApp") + @patch("deerflow.community.firecrawl.tools.get_app_config") + @pytest.mark.anyio + async def test_fetch_closes_the_per_call_client(self, mock_get_app_config, mock_firecrawl_cls): + fetch_config = MagicMock() + fetch_config.model_extra = {"api_key": "firecrawl-fetch-key"} + mock_get_app_config.return_value.get_tool_config.return_value = fetch_config + + mock_scrape_result = MagicMock() + mock_scrape_result.markdown = "Fetched markdown" + mock_scrape_result.metadata = MagicMock(title="Fetched Page") + mock_firecrawl_cls.return_value.scrape = AsyncMock(return_value=mock_scrape_result) + mock_firecrawl_cls.return_value._v2_client.async_http_client.close = AsyncMock() + + from deerflow.community.firecrawl.tools import web_fetch_tool + + await web_fetch_tool.ainvoke({"url": "https://example.com"}) + + mock_firecrawl_cls.return_value._v2_client.async_http_client.close.assert_awaited_once() + + @patch("deerflow.community.firecrawl.tools.AsyncFirecrawlApp") + @patch("deerflow.community.firecrawl.tools.get_app_config") + @pytest.mark.anyio + async def test_search_closes_the_per_call_client_when_search_raises(self, mock_get_app_config, mock_firecrawl_cls): + mock_get_app_config.return_value.get_tool_config.return_value = None + client = mock_firecrawl_cls.return_value + client.search = AsyncMock(side_effect=RuntimeError("search failed")) + client._v2_client.async_http_client.close = AsyncMock() + + from deerflow.community.firecrawl.tools import web_search_tool + + result = await web_search_tool.ainvoke({"query": "test query"}) + + assert result == "Error: search failed" + client._v2_client.async_http_client.close.assert_awaited_once() + + @patch("deerflow.community.firecrawl.tools.AsyncFirecrawlApp") + @patch("deerflow.community.firecrawl.tools.get_app_config") + @pytest.mark.anyio + async def test_fetch_closes_the_per_call_client_when_scrape_raises(self, mock_get_app_config, mock_firecrawl_cls): + mock_get_app_config.return_value.get_tool_config.return_value = None + client = mock_firecrawl_cls.return_value + client.scrape = AsyncMock(side_effect=RuntimeError("scrape failed")) + client._v2_client.async_http_client.close = AsyncMock() + + from deerflow.community.firecrawl.tools import web_fetch_tool + + result = await web_fetch_tool.ainvoke({"url": "https://example.com"}) + + assert result == "Error: scrape failed" + client._v2_client.async_http_client.close.assert_awaited_once() + + @patch("deerflow.community.firecrawl.tools.AsyncFirecrawlApp") + @patch("deerflow.community.firecrawl.tools.get_app_config") + @pytest.mark.anyio + async def test_close_failure_does_not_hide_scrape_error(self, mock_get_app_config, mock_firecrawl_cls, caplog): + mock_get_app_config.return_value.get_tool_config.return_value = None + client = mock_firecrawl_cls.return_value + client.scrape = AsyncMock(side_effect=RuntimeError("scrape failed")) + client._v2_client.async_http_client.close = AsyncMock(side_effect=RuntimeError("close failed")) + + from deerflow.community.firecrawl.tools import web_fetch_tool + + with caplog.at_level(logging.WARNING, logger="deerflow.community.firecrawl.tools"): + result = await web_fetch_tool.ainvoke({"url": "https://example.com"}) + + assert result == "Error: scrape failed" + client._v2_client.async_http_client.close.assert_awaited_once() + assert "Failed to close the Firecrawl async HTTP pool" in caplog.text + + @pytest.mark.anyio + async def test_teardown_reports_a_pool_without_close(self, caplog): + from deerflow.community.firecrawl.tools import _aclose_firecrawl_client + + client = SimpleNamespace(_v2_client=SimpleNamespace(async_http_client=SimpleNamespace())) + + with caplog.at_level(logging.WARNING, logger="deerflow.community.firecrawl.tools"): + await _aclose_firecrawl_client(client) + + assert "Firecrawl async HTTP pool has no close method" in caplog.text + + @pytest.mark.anyio + async def test_teardown_accepts_a_synchronous_close(self, caplog): + from deerflow.community.firecrawl.tools import _aclose_firecrawl_client + + close = Mock() + client = SimpleNamespace(_v2_client=SimpleNamespace(async_http_client=SimpleNamespace(close=close))) + + with caplog.at_level(logging.DEBUG, logger="deerflow.community.firecrawl.tools"): + await _aclose_firecrawl_client(client) + + close.assert_called_once_with() + assert "Failed to close the Firecrawl async HTTP pool" not in caplog.text + + @pytest.mark.anyio + async def test_teardown_closes_the_real_sdk_pool(self): + from firecrawl import AsyncFirecrawlApp as RealApp + + from deerflow.community.firecrawl.tools import _aclose_firecrawl_client + + client = RealApp(api_key="test-key") + pooled = getattr(getattr(client, "_v2_client", None), "async_http_client", None) + inner = getattr(pooled, "_client", None) + if inner is None: + # The helper deliberately treats a missing or reshaped private + # layout as a no-op (the declared >=1.15.0 range includes such + # versions); skip instead of hard-asserting one locked version's + # internals. + pytest.skip("locked firecrawl-py private pool layout not present") + + assert inner.is_closed is False + + await _aclose_firecrawl_client(client) + + assert inner.is_closed is True + + @patch("deerflow.community.firecrawl.tools.AsyncFirecrawlApp") + @patch("deerflow.community.firecrawl.tools.get_app_config") + @pytest.mark.anyio + async def test_teardown_survives_a_client_without_v2_attributes(self, mock_get_app_config, mock_firecrawl_cls): + """Older SDKs in the declared range have no _v2_client; teardown must be a no-op.""" + search_config = MagicMock() + search_config.model_extra = {"api_key": "firecrawl-search-key"} + mock_get_app_config.return_value.get_tool_config.return_value = search_config + + mock_result = MagicMock() + mock_result.web = [] + mock_firecrawl_cls.return_value.search = AsyncMock(return_value=mock_result) + del mock_firecrawl_cls.return_value._v2_client + + from deerflow.community.firecrawl.tools import web_search_tool + + result = await web_search_tool.ainvoke({"query": "test query"}) + + assert result == "[]"