fix(searxng): contain malformed search payloads instead of crashing on them

[yetuge]

Why

DeerFlow's bundled search providers follow a containment rule: an HTTP 200 body that is not a result page is a format error, not an empty search, because "No results found" tells the agent the query legitimately matched nothing and ends a research loop as "no hits". brave (#6550), groundroute (#6603), tavily (#6645), sofya (#6719), serper (#6779), serply (#6796) and tencent-wsa (#6799) all apply it.

SearXNG does not. searxng_client.py indexed the decoded body directly:

data = resp.json()
return data.get("results") or []

This matters most for SearXNG specifically because it is the self-hosted provider: an instance behind a reverse proxy or captive portal answers an HTML or plain-text error page with HTTP 200. Today that reaches the model as a raw Python error:

{"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'"}

{"results": [{"title": "ok", "url": "u", "content": "c"}, 7]}
                                   -> {"error": "'int' object has no attribute 'get'"}

The last case is the worst of them: one malformed entry discards every valid row in the page.

What changed

• SearxngClient raises a new 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.
• Non-object entries in an otherwise valid page are still skipped in order, so the valid rows survive — unchanged behavior.
• Missing, null and empty results still read as an empty search and keep the existing No results found response — unchanged behavior.
• web_search_tool reports SearXNG returned an unexpected response format instead of leaking the exception text into the model-visible result. Transport and configuration failures keep their existing error text.
• README records the containment contract next to the existing brave/groundroute/tavily/sofya/serper/serply/tencent-wsa entries.

Diagnostics log the container field name and its type, or the absence of usable objects — never the query, credentials, or payload values.

How checked

Offline against main (6bc1020d), driving the real client and the real tool:

• RED on the unmodified source: every payload in the table above produced the raw AttributeError/TypeError shown, and a non-JSON 200 body leaked Expecting value: line 1 column 1 (char 0).
• GREEN with the fix: each malformed payload returns {"error": "SearXNG returned an unexpected response format", "query": "q"}, while the mixed page still returns its valid row and empty/null/missing containers still return [].
• RED/GREEN by stash: tests/test_searxng_client.py is 12 failed / 18 passed on the unmodified source and 30 passed with the fix. (The new tests import the new symbol, so the pre-fix run needs it shimmed in to fail behaviorally rather than at collection.)
• tests/test_searxng_client.py: 30 passed; the provider/search suites searxng + time_range + max_results owner + ddg + image_search: 116 passed; ruff check and ruff format --check clean.
• Collision: no open or closed PR touches this behavior for SearXNG. The open fix(community): reject bool and fractional max_results in tavily and searxng #5867 edits only _coerce_max_results in searxng/tools.py and adds its own tests/test_searxng_tools.py; this change does not overlap those lines, and it deliberately extends the existing tests/test_searxng_client.py rather than adding that file. fix(community): reject bool and fractional max_results in the remaining search providers #5866 and fix(community): reject bool and fractional max_results in tavily and searxng #5867 cover max_results coercion, not response containment.

[willem-bd]

Reviewed at head ecfa226. The containment matrix matches the sibling-provider contract (non-object payload, non-list results, non-empty list with no object entries, and a non-JSON 200 body all raise the new SearxngResponseError; missing/null/empty results stay an empty search; mixed pages keep their valid rows), the README paragraph matches the code, and I verified 30/30 tests pass at head with ruff check and ruff format --check clean and imports resolving from the PR source. Two inline notes: the multi-page walk discards already-collected rows if a later page comes back malformed (a SearXNG-only case among the contained providers, worth a deliberate decision or a pinning test), and the _unexpected_format_error docstring contradicts its own return value about echoing the query. The per-provider helper/error-class shape shared with #6932 follows the established brave/tavily/serper/serply/groundroute pattern, so I did not flag it as duplication.

[willem-bd]

Low / design question: this raise propagates out of search() with no handling in the page walk (for pageno in range(1, self._MAX_PAGES + 1)), and SearXNG is the only contained provider whose search() paginates. So a page 1 that parses cleanly followed by a page 2 that comes back as a proxy/captive-portal HTML page discards every row already collected and surfaces only the format error. That may well be the intended trade-off (a proxy that starts failing mid-walk is probably failing persistently, and silently returning a short first page would look like a successful search), but it is a different outcome from the single-request siblings and no test pins it. If losing collected rows is deliberate, consider a test documenting it; if not, catching SearxngResponseError inside the search() loop and returning a non-empty collected (re-raising only when nothing was collected) would keep partial pages.

[yetuge]

You are right that it is a different outcome from the single-request siblings, and I have kept the behaviour rather than changed it: a proxy or captive portal that starts failing between pages is more likely failing persistently than answering one bad page in an otherwise healthy instance, and a short-but-successful first page is exactly the outcome the containment rule exists to avoid. Since SearXNG is the only provider that paginates, this cannot drift into the sibling providers' contract either.

Rather than leave it as an unstated consequence of the loop shape, I pinned it: test_search_drops_collected_rows_when_a_later_page_is_malformed feeds a clean page 1 and a malformed page 2, asserts SearxngResponseError propagates, and asserts the walk stopped at page 2. I also mutation-checked that pin: changing the client to keep the collected rows on a mid-walk error makes the test fail with DID NOT RAISE, so it really does hold the behaviour down.

[willem-bd]

Nit (doc accuracy): the docstring says the report does not echo "it or the query", but line 22 returns "query": query — matching the identical _unexpected_format_error helpers in serply/brave/serper/groundroute, which is the right convention. Only the payload is withheld; the query is echoed on purpose. Please reword so the docstring does not contradict the return value two lines below.

[yetuge]

Fixed. The docstring promised something the function never did: it says the report does not echo "it or the query", but the return value has always carried "query": query, matching the identical helpers in serply/brave/serper/groundroute. Only the payload is withheld; the query is echoed on purpose so the agent can tell which search the error belongs to.

Reworded to "without echoing the payload" with a line spelling out that the query is echoed on purpose, so it no longer contradicts the return value.

[Winoren]

A pruned row disappears without any trace here, which is exactly what the already-merged sibling #6932 chose to log: InfoQuestClient._rows on main emits one warning per skipped entry, and its docstring spells out why -- "each skip is logged with the container name and the entry index ... a provider-side drift would otherwise shed entries with no trace at all". README reflects it too: the InfoQuest paragraph now ends with "Format errors log the container or entry position only".

Same payload through both implementations (a copy of main's InfoQuest loaded side by side):

--- searxng  _response_rows({'results': [dict, int, dict]})
    rows kept: 2
    log: <none>
--- infoquest _rows([dict, int, dict], 'results')   [merged #6932, upstream/main]
    rows kept: 2
    log: WARNING: Skipping malformed InfoQuest result entry at results[1]

Skipping behaviour agrees; the observability does not. SearXNG is the provider where drift is most expected (a self-hosted instance can serve whatever sits behind its reverse proxy), so this is the case that most needs a trace -- and _MAX_PAGES = 5 means one instance can shed rows across five requests silently.

Keeping the current error paths intact:

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

[yetuge]

Done, and thanks for the side-by-side demo — the log gap was exactly the kind of asymmetry I should have caught when the InfoQuest sibling merged. Adopted your drop-in loop: _response_rows now warns per skipped entry with the position (Skipping malformed SearXNG result entry at results[1]), keeping every existing error path intact. Also extended the docstring to say why this provider needs the trace (self-hosted drift behind a reverse proxy, and up to _MAX_PAGES requests per search).

Pinned with test_search_logs_the_index_of_a_skipped_row: same [dict, int, dict] shape, asserts 2 rows kept, asserts results[1] appears in the warning, and asserts no payload value leaks into it. Mutation-checked — restoring the silent list comprehension makes it fail.

[Winoren]

This branch is currently indistinguishable from the generic except Exception fallback further down: SearxngResponseError subclasses ValueError, its message is exactly "SearXNG returned an unexpected response format", and that fallback already returns json.dumps({"error": str(e), "query": query}). Reverting tools.py to its parent while keeping every client-side change leaves the whole file green:

$ git checkout 6bc1020d -- backend/.../community/searxng/tools.py
$ pytest tests/test_searxng_client.py -q -p no:randomly
31 passed        # includes test_web_search_tool_reports_a_format_error_for_a_malformed_page
                 #     and test_web_search_tool_keeps_the_valid_rows_of_a_mixed_page

Nothing in test_searxng_client.py touches caplog, so no test can tell which handler produced the reply -- a later edit of the exception message would turn the tool reply into a generic error string with no failing test to catch it.

The handler itself reads well and I would keep it; it just needs a test that can fail. One way, asserting what actually distinguishes the two paths (the generic branch logs Error in web_search_tool: ... before returning):

assert "Error in web_search_tool" not in caplog.text

[yetuge]

You are right, and thank you for constructing the counterexample — reverting tools.py to its parent and getting 31 passed is conclusive.

Pinned with test_web_search_tool_routes_the_format_error_before_the_generic_handler, using the discriminator you suggested: it drives the tool with a SearxngResponseError and asserts the generic branch's Error in web_search_tool: ... line is absent from caplog. Removing the dedicated handler now fails exactly this test (verified by mutation), so the handler can no longer be deleted or have its message changed silently.

Also rebased onto current main and resolved the README conflict by keeping both paragraphs in family order (SearXNG then InfoQuest).

[Winoren]

Verified this PR on its own head and against main, in the search-provider containment family I worked in before (serper #6966 merged, tencent_wsa #6973, tavily pending).

What I checked

Check	Result
pytest tests/test_searxng_client.py -q -p no:randomly @ c3729c25	31 passed
Red→green: return data.get("results") or [] restored in _search_page (everything else kept)	12 failed / 19 passed
Restore	31 passed
Related surface: searxng + single-owner census + infoquest + tavily + serper + serply + groundroute	308 passed
ruff check / ruff format --check on touched trees	clean
Cherry-pick onto current main 4f35eb58	code clean; README adjacent-line conflict only (keep both)

The containment rule itself is right, and the mid-walk abort trade-off (one failing page discards the rows already collected, because a short first page would look like a complete search) is the correct call for the only paginating provider in the family — and it is pinned by a test rather than left to a reader's imagination.

Two suggestions (both filed inline)

1. searxng_client.py:103 — skipped entries are silent. The merged sibling #6932 logs every skipped entry with its index (WARNING: Skipping malformed InfoQuest result entry at results[1]), and its docstring gives the reason: a provider-side drift would otherwise shed entries with no trace at all. I ran the same payload [dict, int, dict] through both: identical rows kept (2), but SearXNG emits no record at all. With _MAX_PAGES = 5 a drifting self-hosted instance can shed rows across five requests silently. Inline has a drop-in loop that keeps the current error paths.

2. tools.py:86 — the new except SearxngResponseError branch has no test that can fail. SearxngResponseError is a ValueError whose message is already the format-error string, so the pre-existing generic except Exception returns the same JSON. Reverting tools.py to its parent, client and tests untouched, still gives 31 passed — including both tool-level format-error tests. No test in the file uses caplog. The handler should stay; it just needs an assertion that distinguishes the two paths (inline suggests "Error in web_search_tool" not in caplog.text).

Neither is a correctness problem — the diff is safe to land as is. Both are about making the family contract observable and pinned, which is what the merged siblings now do.

Note for whoever merges

The base is ~30 commits behind main. The code cherry-picks cleanly; the only conflict is the README paragraph, where this PR's SearXNG line and #6932's InfoQuest line want the same position. Keeping both lines preserves the ordering the family uses.