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 == "[]"