From 73f6749068b097038960c9aef528cf14bd4f4f09 Mon Sep 17 00:00:00 2001 From: yetuge <182123929+yetuge@users.noreply.github.com> Date: Sun, 18 Oct 2026 13:12:23 +0800 Subject: [PATCH] fix(searxng): contain malformed search payloads instead of crashing on them (#6931) * fix(searxng): contain malformed search payloads instead of crashing on them `_search_page` indexed the decoded body directly: data = resp.json() return data.get("results") or [] so a self-hosted instance that answers HTTP 200 with something other than a result page -- an HTML or plain-text error page from a reverse proxy or captive portal -- reached the model as a raw Python error, or as an empty search: {"results": "maintenance window"} -> {"error": "'str' object has no attribute 'get'"} {"results": {"a": 1}} -> {"error": "'str' object has no attribute 'get'"} {"results": [1, 2, 3]} -> {"error": "'int' object has no attribute 'get'"} {"results": [None]} -> {"error": "'NoneType' object has no attribute 'get'"} Worse, a single malformed entry in an otherwise valid page crashed the whole call and discarded every good row: {"results": [{"title": "ok", ...}, 7]} -> {"error": "'int' object has no attribute 'get'"} This is the containment rule the sibling providers already follow (brave #6550, groundroute #6603, tavily #6645, sofya #6719, serper #6779, serply #6796, tencent-wsa #6799): a payload that is not a result page is a format error, not an empty result, because "No results found" tells the agent the query legitimately matched nothing and ends a research loop as "no hits". The client now raises `SearxngResponseError` for a payload that is not an object, a body that is not JSON, a `results` container that is not a list, and a non-empty list holding no object entries, while still skipping non-object entries in an otherwise valid page so the valid rows survive. `web_search_tool` reports the provider's format error instead of leaking the exception text. Missing, null, and empty `results` keep the existing "No results found" behavior. Diagnostics log the container field name and its type, or the absence of usable objects -- never the query, credentials, or payload values. * test(searxng): pin the mid-walk format-error trade-off and fix the docstring The paginating client aborts the walk and discards already-collected rows when a later page comes back malformed, which is a different outcome from the single-request siblings. Pin that trade-off with a test so the behaviour is a deliberate decision rather than an accident, and reword the _unexpected_format_error docstring: it promises the query is not echoed, but the sibling providers all echo it back on purpose and only withhold the payload. * test(searxng): log skipped entries and pin the dedicated format-error handler Two follow-ups from review, both about making the family contract observable and pinned: - Skipped non-object entries now log their index ("Skipping malformed SearXNG result entry at results[1]"), matching the merged InfoQuest sibling. SearXNG is the provider where drift is most expected -- a self-hosted instance serves whatever sits behind its reverse proxy -- and a search spans up to _MAX_PAGES requests, so rows could otherwise be shed with no trace at all. - The dedicated `except SearxngResponseError` branch had no test that could fail: the exception is a ValueError whose message is already the format-error string, so the generic handler returns byte-identical JSON. The new test asserts the generic branch's log line is absent, which is what actually distinguishes the two paths. * docs(searxng): name the empty result it actually returns, and pin the dedicated except Two low findings from the follow-up review: - The README said missing/null/empty `results` keep the existing "No results found" response. That response does not exist in this provider: all three produce a JSON empty list, and the string appears only in comments explaining what must NOT be reported. The family convention distinguishes (Tavily: "`[]` for search, Error: No results found for fetch"; InfoQuest: "keep the existing empty result"), so this now reads "keep the existing empty result (`[]`)". - The client-level `except SearxngResponseError: raise` was unpinned: deleting it left the whole file green, because the tool-level test mocks the client and the payload tests only assert the raise type. The observable regression is the generic branch logging "An unexpected error occurred" for every contained page, so the existing payload test now asserts that line is absent from caplog. Mutation check: removing the clause now fails 8 parameterised cases (was 33 green); with the clause in place the suite is 33 passed and 91 across the two provider files, ruff check/format clean. --- README.md | 2 + .../community/searxng/searxng_client.py | 57 ++++- .../deerflow/community/searxng/tools.py | 19 +- backend/tests/test_searxng_client.py | 234 +++++++++++++++++- 4 files changed, 308 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index ccbeca064a..c23f89ff3e 100644 --- a/README.md +++ b/README.md @@ -190,6 +190,8 @@ streams simulate chunks from a non-streaming response, while no-tool streams sta Serply web search contains malformed HTTP-success payloads the same way in every vertical: a `results`/`entries`/`articles` container that is not a list, or a non-empty list with no object entries, returns the provider's format error, while non-object entries in an otherwise valid list are skipped in order. Missing, null, or empty containers keep the existing "No results found" response. + SearXNG web search contains malformed HTTP-success payloads the same way: a payload that is not an object, a body that is not JSON, a `results` container that is not a list, or a non-empty list holding no object entries returns the provider's format error, while non-object entries in an otherwise valid page are skipped in order, each skip logged with its entry position, so the valid rows survive and provider-side drift leaves a trace. Missing, null, or empty `results` keep the existing empty result (`[]`). This matters most for a self-hosted instance: one behind a reverse proxy or captive portal answers an HTML or plain-text error page with HTTP 200, and reading that as an empty search ends a research loop as "no hits". + InfoQuest web and image search contain malformed HTTP-success payloads the same way: a `search_result` envelope that is not an object, or an `organic`/`top_stories.items`/`images_results` container that is not a list or that holds no usable entry, returns the provider's format error, while malformed entries in an otherwise valid payload are skipped so the usable rows survive. Missing, null, or empty containers keep the existing empty result. Format errors log the container or entry position only, without queries or payload values. InfoQuest web and image search screen result URLs through the shared public-URL policy and omit rejected rows. Responses without the required `search_result` envelope return a format error instead of exposing the raw provider payload. diff --git a/backend/packages/harness/deerflow/community/searxng/searxng_client.py b/backend/packages/harness/deerflow/community/searxng/searxng_client.py index ca59e43eab..a23bf10e30 100644 --- a/backend/packages/harness/deerflow/community/searxng/searxng_client.py +++ b/backend/packages/harness/deerflow/community/searxng/searxng_client.py @@ -8,6 +8,17 @@ from deerflow.community.search_time_range import SearchTimeRange logger = logging.getLogger(__name__) +class SearxngResponseError(ValueError): + """Raised when a SearXNG instance answers with a malformed success payload. + + The search API answers HTTP 200 with ``results`` as its only contract; an + instance behind a reverse proxy or captive portal instead answers an HTML or + plain-text error page with the same status. Such a payload is not an empty + search, so the caller reports a format error rather than "No results found" + -- the containment rule the sibling providers already follow. + """ + + class SearxngClient: """Client for SearXNG meta search engine API.""" @@ -68,6 +79,42 @@ class SearxngClient: return collected + @staticmethod + def _response_rows(data: object) -> list[dict[str, Any]]: + """Return the object rows of one page, or raise when the payload is malformed. + + A missing or null ``results`` is an empty search. A payload that is not + an object, a ``results`` container that is not a list, and a non-empty + list holding no object entries are all malformed: an instance behind a + reverse proxy or captive portal answers an HTML or plain-text error page + with HTTP 200, and reporting "No results found" for it would tell the + agent the query legitimately matched nothing. Non-object entries in an + otherwise valid list are skipped in order so the valid rows survive, and + each skip is logged with the entry index: this is the provider where + drift is most expected (a self-hosted instance serves whatever sits + behind its reverse proxy), and with ``_MAX_PAGES`` requests per search a + drifting instance would otherwise shed rows with no trace at all. + """ + if not isinstance(data, dict): + logger.error("SearXNG returned an unexpected payload type: %s", type(data).__name__) + raise SearxngResponseError("SearXNG returned an unexpected response format") + results = data.get("results") + if results is None: + return [] + if not isinstance(results, list): + logger.error("SearXNG returned an unexpected 'results' payload type: %s", type(results).__name__) + raise SearxngResponseError("SearXNG returned an unexpected response format") + usable: list[dict[str, Any]] = [] + for index, row in enumerate(results): + if not isinstance(row, dict): + logger.warning("Skipping malformed SearXNG result entry at results[%d]", index) + continue + usable.append(row) + if results and not usable: + logger.error("SearXNG returned 'results' with no usable result objects") + raise SearxngResponseError("SearXNG returned an unexpected response format") + return usable + async def _search_page( self, query: str, @@ -104,14 +151,20 @@ class SearxngClient: }, ) resp.raise_for_status() - data = resp.json() - return data.get("results") or [] + try: + data = resp.json() + except ValueError as e: + logger.error("SearXNG returned a response body that is not JSON") + raise SearxngResponseError("SearXNG returned an unexpected response format") from e + return self._response_rows(data) except httpx.HTTPStatusError as e: logger.error(f"SearXNG search returned error status: {e}") raise except httpx.RequestError as e: logger.error(f"SearXNG search request failed: {e}") raise + except SearxngResponseError: + raise except Exception as e: logger.error(f"An unexpected error occurred during SearXNG search: {e}") raise diff --git a/backend/packages/harness/deerflow/community/searxng/tools.py b/backend/packages/harness/deerflow/community/searxng/tools.py index 9eb3e822b6..48adad78c9 100644 --- a/backend/packages/harness/deerflow/community/searxng/tools.py +++ b/backend/packages/harness/deerflow/community/searxng/tools.py @@ -6,11 +6,26 @@ from langchain.tools import tool from deerflow.community.search_time_range import SearchTimeRange from deerflow.config import get_app_config -from .searxng_client import SearxngClient +from .searxng_client import SearxngClient, SearxngResponseError logger = logging.getLogger(__name__) +def _unexpected_format_error(query: str) -> str: + """Report a malformed instance payload without echoing the payload. + + A SearXNG instance that answers HTTP 200 with an error page, a non-list + ``results`` container, or a non-empty list holding no objects is not an + empty search: "No results found" would tell the agent the query legitimately + matched nothing. + + The query is echoed back on purpose, matching the sibling providers, so the + agent can tell which search the error belongs to; only the payload is + withheld. + """ + return json.dumps({"error": "SearXNG returned an unexpected response format", "query": query}, ensure_ascii=False) + + def _get_tool_config(tool_name: str) -> dict | None: """Get tool config extras safely, returning None if not configured.""" config = get_app_config().get_tool_config(tool_name) @@ -68,6 +83,8 @@ async def web_search_tool(query: str, time_range: SearchTimeRange | None = None) for r in results ] return json.dumps(normalized, indent=2, ensure_ascii=False) + except SearxngResponseError: + return _unexpected_format_error(query) except Exception as e: logger.error(f"Error in web_search_tool: {e}") return json.dumps({"error": str(e), "query": query}, ensure_ascii=False) diff --git a/backend/tests/test_searxng_client.py b/backend/tests/test_searxng_client.py index e6b11ba32b..a613052a91 100644 --- a/backend/tests/test_searxng_client.py +++ b/backend/tests/test_searxng_client.py @@ -1,13 +1,15 @@ """Tests for SearXNG community tools.""" import json +import logging from contextlib import contextmanager from unittest.mock import MagicMock, patch +import httpx import pytest from deerflow.community.searxng import tools -from deerflow.community.searxng.searxng_client import SearxngClient +from deerflow.community.searxng.searxng_client import SearxngClient, SearxngResponseError class AsyncMock(MagicMock): @@ -209,6 +211,39 @@ class TestSearxngClient: assert len(result) == 2 assert mock_ctx.get.call_count == 2 + async def test_search_drops_collected_rows_when_a_later_page_is_malformed(self): + """A malformed page aborts the walk and discards the rows already collected. + + SearXNG is the only contained provider whose ``search()`` paginates, so it + is the only one where a multi-request search can fail midway. Pinning the + trade-off: a reverse proxy or captive portal that starts failing between + pages is assumed to be failing persistently, and returning the short first + page would look like a complete search. Raising the format error is the + sibling providers' behaviour too (they fail on their single request), so + this keeps the family contract at the cost of the already-collected rows. + """ + first_page = [{"title": f"r{i}", "url": f"https://example.com/1/{i}", "content": "c"} for i in range(3)] + + with patch("deerflow.community.searxng.searxng_client.httpx.AsyncClient") as mock_cls: + mock_ctx = MagicMock() + mock_cls.return_value.__aenter__.return_value = mock_ctx + + def _get(url, params=None, headers=None): + mock_resp = MagicMock() + mock_resp.status_code = 200 + # Page 1 parses; page 2 is the proxy/captive-portal HTML page. + mock_resp.json.return_value = {"results": first_page} if params["pageno"] == 1 else "upstream maintenance" + mock_resp.raise_for_status.return_value = None + return mock_resp + + mock_ctx.get = AsyncMock(side_effect=_get) + + client = SearxngClient(base_url="http://searxng:8080") + with pytest.raises(SearxngResponseError): + await client.search("test query", max_results=20) + + assert [call.kwargs["params"]["pageno"] for call in mock_ctx.get.call_args_list] == [1, 2] + async def test_search_never_sends_the_unsupported_limit_parameter(self): """`limit` is not part of the SearXNG search API, so it is not sent.""" with _searxng_pages({1: [{"title": "t", "url": "https://example.com/1", "content": "c"}]}) as mock_ctx: @@ -217,6 +252,118 @@ class TestSearxngClient: assert "limit" not in mock_ctx.get.call_args_list[0].kwargs["params"] + @pytest.mark.parametrize( + "payload", + [ + {"results": "maintenance window"}, + {"results": {"a": 1}}, + {"results": 7}, + {"results": [1, 2, 3]}, + {"results": [None, None]}, + {"results": ["a"]}, + ["not-an-object"], + "text", + ], + ids=[ + "results-is-a-string", + "results-is-a-populated-dict", + "results-is-an-int", + "list-with-no-objects", + "list-of-nulls", + "list-of-strings", + "payload-is-a-list", + "payload-is-a-string", + ], + ) + async def test_search_malformed_payload_reports_a_format_error(self, payload, caplog): + """A malformed 200 payload is a format error, not an empty search. + + An instance behind a reverse proxy or captive portal answers an HTML or + plain-text error page with HTTP 200. Returning an empty list for it would + tell the agent the query legitimately matched nothing -- and indexing the + raw payload first leaked an AttributeError/TypeError instead. + + The raise must come from the dedicated ``except SearxngResponseError`` + clause, not the generic ``except Exception`` below: the generic branch + logs "An unexpected error occurred" at ERROR for every contained page + before re-raising, so without this assertion the clause can be deleted + with the whole file still green. + """ + with patch("deerflow.community.searxng.searxng_client.httpx.AsyncClient") as mock_cls: + mock_ctx = MagicMock() + mock_cls.return_value.__aenter__.return_value = mock_ctx + + mock_resp = MagicMock() + mock_resp.status_code = 200 + mock_resp.json.return_value = payload + mock_resp.raise_for_status.return_value = None + mock_ctx.get = AsyncMock(return_value=mock_resp) + + client = SearxngClient(base_url="http://searxng:8080") + with caplog.at_level(logging.ERROR, logger="deerflow.community.searxng.searxng_client"): + with pytest.raises(SearxngResponseError): + await client.search("test query") + + assert "An unexpected error occurred" not in caplog.text + + async def test_search_keeps_valid_rows_from_a_mixed_page(self): + """Non-object entries in an otherwise valid page are skipped in order. + + The page still has usable rows, so it is not malformed: dropping them + would lose a valid answer. + """ + rows = [ + {"title": "ok", "url": "https://example.com/1", "content": "c"}, + 7, + None, + ] + + with _searxng_pages({1: rows}): + client = SearxngClient(base_url="http://searxng:8080") + result = await client.search("test query", max_results=5) + + assert [row["title"] for row in result] == ["ok"] + + async def test_search_logs_the_index_of_a_skipped_row(self, caplog): + """Every skipped entry is logged with its position. + + The merged sibling #6932 reports the same drift for InfoQuest + (``Skipping malformed InfoQuest result entry at results[1]``); SearXNG is + the provider where drift is most expected and where a single search can + span ``_MAX_PAGES`` requests, so the skip must leave a trace. + """ + rows = [ + {"title": "kept-0", "url": "https://example.com/0", "content": "c"}, + 7, + {"title": "kept-2", "url": "https://example.com/2", "content": "c"}, + ] + + with _searxng_pages({1: rows}): + client = SearxngClient(base_url="http://searxng:8080") + with caplog.at_level(logging.WARNING, logger="deerflow.community.searxng.searxng_client"): + result = await client.search("test query", max_results=5) + + assert [row["title"] for row in result] == ["kept-0", "kept-2"] + warnings = [record.getMessage() for record in caplog.records if record.levelno == logging.WARNING] + assert any("results[1]" in message for message in warnings), warnings + assert not any("example.com" in message for message in warnings), warnings + + async def test_search_non_json_body_reports_a_format_error(self): + """A 200 body that is not JSON at all is a format error, not a crash.""" + with patch("deerflow.community.searxng.searxng_client.httpx.AsyncClient") as mock_cls: + mock_ctx = MagicMock() + mock_cls.return_value.__aenter__.return_value = mock_ctx + + mock_resp = MagicMock() + mock_resp.status_code = 200 + mock_resp.json.side_effect = ValueError("Expecting value: line 1 column 1 (char 0)") + mock_resp.raise_for_status.return_value = None + mock_ctx.get = AsyncMock(return_value=mock_resp) + + client = SearxngClient(base_url="http://searxng:8080") + with pytest.raises(SearxngResponseError): + await client.search("test query") + @pytest.mark.asyncio class TestSearxngTools: @@ -319,3 +466,88 @@ class TestSearxngTools: data = json.loads(result) assert "error" not in data assert mock_client.search.call_args.kwargs["max_results"] == 5 + + async def test_web_search_tool_reports_a_format_error_for_a_malformed_page(self): + """The agent sees the provider's format error, not a raw Python error. + + The page parses but holds no usable entry, so the tool must answer with + the provider's format error. Pre-fix, indexing the raw payload leaked an + AttributeError/TypeError into the model-visible result instead. + """ + with patch("deerflow.community.searxng.searxng_client.httpx.AsyncClient") as mock_cls: + mock_ctx = MagicMock() + mock_cls.return_value.__aenter__.return_value = mock_ctx + + mock_resp = MagicMock() + mock_resp.status_code = 200 + mock_resp.json.return_value = {"results": [1, None, "x"]} + mock_resp.raise_for_status.return_value = None + mock_ctx.get = AsyncMock(return_value=mock_resp) + + with patch("deerflow.community.searxng.tools._get_tool_config", return_value=None): + result = await tools.web_search_tool.ainvoke("test query") + + data = json.loads(result) + assert data["error"] == "SearXNG returned an unexpected response format" + assert data["query"] == "test query" + + async def test_web_search_tool_keeps_the_valid_rows_of_a_mixed_page(self): + """A page with usable rows still answers with those rows.""" + with patch("deerflow.community.searxng.searxng_client.httpx.AsyncClient") as mock_cls: + mock_ctx = MagicMock() + mock_cls.return_value.__aenter__.return_value = mock_ctx + + mock_resp = MagicMock() + mock_resp.status_code = 200 + mock_resp.json.return_value = { + "results": [ + {"title": "Result 1", "url": "https://example.com/1", "content": "Desc 1"}, + 7, + ] + } + mock_resp.raise_for_status.return_value = None + mock_ctx.get = AsyncMock(return_value=mock_resp) + + with patch("deerflow.community.searxng.tools._get_tool_config", return_value=None): + result = await tools.web_search_tool.ainvoke("test query") + + data = json.loads(result) + assert [row["title"] for row in data] == ["Result 1"] + + @patch("deerflow.community.searxng.tools._get_searxng_client") + async def test_web_search_tool_keeps_other_failures_as_their_own_errors(self, mock_get_client): + """Transport and configuration failures keep their existing error text.""" + mock_client = MagicMock() + mock_client.search = AsyncMock(side_effect=httpx.RequestError("Connection refused")) + mock_get_client.return_value = mock_client + + with patch("deerflow.community.searxng.tools._get_tool_config", return_value=None): + result = await tools.web_search_tool.ainvoke("test query") + + data = json.loads(result) + assert "unexpected response format" not in data["error"] + assert mock_client.search.call_args.kwargs["max_results"] == 5 + + @patch("deerflow.community.searxng.tools._get_searxng_client") + async def test_web_search_tool_routes_the_format_error_before_the_generic_handler(self, mock_get_client, caplog): + """The format-error reply comes from its own handler, not the generic one. + + ``SearxngResponseError`` is a ``ValueError`` whose message is already the + format-error string, so the generic ``except Exception`` below would + return byte-identical JSON. What distinguishes the two paths is the log + line: the generic branch records ``Error in web_search_tool: ...`` first. + Without this assertion the dedicated handler could be deleted (or the + exception message changed) with every test still green. + """ + mock_client = MagicMock() + mock_client.search = AsyncMock(side_effect=SearxngResponseError("SearXNG returned an unexpected response format")) + mock_get_client.return_value = mock_client + + with patch("deerflow.community.searxng.tools._get_tool_config", return_value=None): + with caplog.at_level(logging.ERROR, logger="deerflow.community.searxng.tools"): + result = await tools.web_search_tool.ainvoke("test query") + + data = json.loads(result) + assert data["error"] == "SearXNG returned an unexpected response format" + assert data["query"] == "test query" + assert "Error in web_search_tool" not in caplog.text