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.
This commit is contained in:
yetuge 2026-10-18 13:12:23 +08:00 committed by GitHub
parent 1098fb4c60
commit 73f6749068
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 308 additions and 4 deletions

View File

@ -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.

View File

@ -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

View File

@ -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)

View File

@ -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