From 4307cac96603022551bebce225aa11678b4d58c6 Mon Sep 17 00:00:00 2001 From: SurprisedDuck Date: Tue, 2 Jun 2026 13:34:25 +0200 Subject: [PATCH] Research: report empty search provider results clearly Deep Research surfaced 'Error: unknown error' whenever every search provider returned an empty result set without raising (e.g. SearXNG is reachable but all its engines fail internally). _last_search_error was only set on exceptions, so the empty-but-no-exception path left it unset and the caller fell back to 'unknown error'. Record an actionable reason on that path naming the providers that were tried, so users can tell it's a search-backend problem rather than a model problem. The provider-raised path is unchanged. Re: #344. Co-authored-by: Claude Opus 4.8 --- src/deep_research.py | 16 ++++- tests/test_deep_research_search_error.py | 84 ++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 tests/test_deep_research_search_error.py diff --git a/src/deep_research.py b/src/deep_research.py index f60ca3b..1c4ea39 100644 --- a/src/deep_research.py +++ b/src/deep_research.py @@ -535,7 +535,9 @@ class DeepResearcher: return [] # Try primary provider, then fallbacks - for prov in _build_provider_chain(provider): + chain = _build_provider_chain(provider) + raised = False + for prov in chain: try: results = await asyncio.to_thread(_call_provider, prov, query, 10) if results: @@ -544,8 +546,20 @@ class DeepResearcher: self.providers_used.append(prov) return results except Exception as e: + raised = True logger.warning(f"Research search: {prov} failed: {e}") self._last_search_error = f"{prov}: {e}" + # Every provider ran but none returned results. If none of them + # raised, record an actionable reason here — otherwise this empty + # path leaves `_last_search_error` unset and the caller surfaces a + # bare "unknown error" (issue #344). This is exactly the SearXNG + # case where the service is reachable but all its engines fail, so + # each provider returns [] without throwing. + if not raised: + self._last_search_error = ( + f"no results from search provider(s): " + f"{', '.join(chain) if chain else provider}" + ) return [] except Exception as e: logger.error(f"Search failed for '{query}': {e}") diff --git a/tests/test_deep_research_search_error.py b/tests/test_deep_research_search_error.py new file mode 100644 index 0000000..43b3e3b --- /dev/null +++ b/tests/test_deep_research_search_error.py @@ -0,0 +1,84 @@ +"""Regression tests for deep-research search error reporting (issue #344). + +When every configured search provider returns no results *without raising* +(e.g. SearXNG is reachable but all of its engines fail), ``_search`` used to +leave ``_last_search_error`` unset. The caller then surfaced a useless +"Search unavailable ... Error: unknown error" message, which is what the +reporter in #344 was confused by ("is this a model issue or deep research +issue?"). + +These tests pin that the empty-but-no-exception path now records an +actionable reason, while the existing raise path keeps surfacing the +provider's own error. +""" +import asyncio +import sys +import types + + +def _make_researcher(): + # Build the object without running the heavy __init__ (which wires up an + # LLM caller etc.); _search only touches the attributes set below. + from src.deep_research import DeepResearcher + r = DeepResearcher.__new__(DeepResearcher) + r.search_provider_override = None + r.providers_used = [] + return r + + +def _install_search_fakes(monkeypatch, *, chain, call_provider): + providers_mod = types.ModuleType("src.search.providers") + providers_mod._get_search_settings = lambda: {"search_provider": chain[0]} + core_mod = types.ModuleType("src.search.core") + core_mod._build_provider_chain = lambda provider: list(chain) + core_mod._call_provider = call_provider + monkeypatch.setitem(sys.modules, "src.search.providers", providers_mod) + monkeypatch.setitem(sys.modules, "src.search.core", core_mod) + + +def test_empty_results_without_exception_record_reason(monkeypatch): + # Both providers are reachable but return nothing, and neither raises. + _install_search_fakes( + monkeypatch, + chain=["searxng", "duckduckgo"], + call_provider=lambda prov, query, n: [], + ) + r = _make_researcher() + results = asyncio.run(r._search("anything")) + + assert results == [] + # Before the fix this stayed unset, so the caller reported "unknown error". + err = getattr(r, "_last_search_error", None) + assert err, "an empty search must record a reason, not leave it unset" + assert "no results" in err + # Names the provider(s) that were actually tried, so the message is useful. + assert "searxng" in err + + +def test_provider_exception_is_still_surfaced(monkeypatch): + # A provider that raises must keep surfacing its own error unchanged. + def _boom(prov, query, n): + raise RuntimeError("connection refused") + + _install_search_fakes(monkeypatch, chain=["searxng"], call_provider=_boom) + r = _make_researcher() + results = asyncio.run(r._search("anything")) + + assert results == [] + err = getattr(r, "_last_search_error", None) + assert err and "connection refused" in err + # The raise path, not the empty-results path. + assert "no results" not in err + + +def test_results_are_returned_and_provider_recorded(monkeypatch): + # Sanity: a provider with results returns them and is recorded. + hits = [{"url": "https://example.com", "title": "x"}] + _install_search_fakes( + monkeypatch, chain=["brave"], call_provider=lambda p, q, n: hits + ) + r = _make_researcher() + results = asyncio.run(r._search("anything")) + + assert results == hits + assert r.providers_used == ["brave"]