mirror of
https://github.com/infiniflow/ragflow.git
synced 2026-07-23 17:06:42 +08:00
fix(agent/tools): surface serpapi error responses in google search (#17062)
### Summary
`agent/tools/google.py` indexes `search["organic_results"]` directly
after `GoogleSearch(params).get_dict()`. serpapi returns `{"error":
...}` **without** an `organic_results` key on realistic conditions — an
invalid API key, an exhausted plan/quota, or a query that matched
nothing. that raised `KeyError('organic_results')`, which the tool's
retry loop then surfaced to the model as the opaque `"Google error:
'organic_results'"` instead of the real reason.
this guards the missing key and raises serpapi's actual `error` message
(with a clear fallback) into the existing retry/`_ERROR` path, so the
model sees e.g. `"Google error: Invalid API key."`. valid responses are
unchanged.
adds `test/unit_test/agent/tools/test_google_unit.py` covering the
error-response path (asserts the real message is surfaced, no
`KeyError`) and the normal result path, mirroring the existing
`test_googlescholar.py` pattern (skips when the `serpapi` SDK is
absent).
---------
Co-authored-by: Yaroslav98214 <diakovichyaroslav30@gmail.com>
This commit is contained in:
@@ -498,13 +498,23 @@ class Google(ToolBase, ABC):
|
||||
if self.check_if_canceled("Google processing"):
|
||||
return
|
||||
|
||||
# serpapi reports an invalid key, exhausted quota or an empty result
|
||||
# set through an "error" field and omits "organic_results"; surface that
|
||||
# message instead of raising a cryptic KeyError on the missing key.
|
||||
if "organic_results" not in search:
|
||||
raise RuntimeError(search.get("error", "SerpApi returned no organic_results."))
|
||||
|
||||
organic_results = search["organic_results"]
|
||||
# a result may omit any of these; note the fallback of the "description"
|
||||
# lookup is evaluated eagerly, so it has to be a .get() too or a result
|
||||
# carrying a description but no snippet raises KeyError.
|
||||
self._retrieve_chunks(
|
||||
search["organic_results"],
|
||||
get_title=lambda r: r["title"],
|
||||
get_url=lambda r: r["link"],
|
||||
get_content=lambda r: r.get("about_this_result", {}).get("source", {}).get("description", r["snippet"]),
|
||||
organic_results,
|
||||
get_title=lambda r: r.get("title", ""),
|
||||
get_url=lambda r: r.get("link", ""),
|
||||
get_content=lambda r: r.get("about_this_result", {}).get("source", {}).get("description", r.get("snippet", "")),
|
||||
)
|
||||
self.set_output("json", search["organic_results"])
|
||||
self.set_output("json", organic_results)
|
||||
return self.output("formalized_content")
|
||||
except Exception as e:
|
||||
if self.check_if_canceled("Google processing"):
|
||||
|
||||
98
test/unit_test/agent/tools/test_google_unit.py
Normal file
98
test/unit_test/agent/tools/test_google_unit.py
Normal file
@@ -0,0 +1,98 @@
|
||||
#
|
||||
# Copyright 2026 The InfiniFlow Authors. All Rights Reserved.
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License");
|
||||
# you may not use this file except in compliance with the License.
|
||||
# You may obtain a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS,
|
||||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
#
|
||||
|
||||
import pytest
|
||||
|
||||
# Google imports the `serpapi` SDK at module load; skip where absent.
|
||||
pytest.importorskip("serpapi")
|
||||
|
||||
import agent.tools.google as g_module # noqa: E402
|
||||
from agent.tools.google import Google, GoogleParam # noqa: E402
|
||||
|
||||
|
||||
class _FakeSearch:
|
||||
"""Stands in for serpapi.GoogleSearch; get_dict() returns a canned response."""
|
||||
|
||||
def __init__(self, result):
|
||||
self._result = result
|
||||
|
||||
def get_dict(self):
|
||||
return self._result
|
||||
|
||||
|
||||
def _make_tool():
|
||||
# Bypass the canvas-bound __init__ (mirrors test_googlescholar.py) and stub the
|
||||
# canvas-touching helpers so we can exercise _invoke's response handling. Zero the
|
||||
# error delay so the failing path doesn't sleep.
|
||||
g = Google.__new__(Google)
|
||||
param = GoogleParam()
|
||||
param.api_key = "k"
|
||||
param.max_retries = 0
|
||||
param.delay_after_error = 0
|
||||
g._param = param
|
||||
g.check_if_canceled = lambda *a, **k: False
|
||||
|
||||
captured = {}
|
||||
out = {}
|
||||
|
||||
def fake_retrieve(res_list, get_title=None, get_url=None, get_content=None, **_kw):
|
||||
# The real _retrieve_chunks applies these getters to every result; replicate
|
||||
# that so the tests actually exercise the lambdas, which is where the
|
||||
# per-result KeyErrors came from.
|
||||
items = list(res_list)
|
||||
captured["chunks"] = items
|
||||
captured["rendered"] = [{"title": get_title(r), "url": get_url(r), "content": get_content(r)} for r in items]
|
||||
out["formalized_content"] = "FC"
|
||||
|
||||
g._retrieve_chunks = fake_retrieve
|
||||
g.set_output = lambda k, v: out.__setitem__(k, v)
|
||||
g.output = lambda k=None: out.get(k) if k else out
|
||||
return g, captured, out
|
||||
|
||||
|
||||
def test_error_response_surfaces_serpapi_message(monkeypatch):
|
||||
# Regression: on an invalid key / exhausted quota / no-match, serpapi returns
|
||||
# {"error": ...} with no "organic_results". The tool used to raise
|
||||
# KeyError('organic_results'), reported to the model as the opaque
|
||||
# "Google error: 'organic_results'". It must surface serpapi's real message.
|
||||
monkeypatch.setattr(g_module, "GoogleSearch", lambda params: _FakeSearch({"error": "Invalid API key."}))
|
||||
g, _, out = _make_tool()
|
||||
result = g._invoke(q="anything")
|
||||
assert "Invalid API key." in result
|
||||
assert "organic_results" not in result
|
||||
assert out.get("_ERROR") == "Invalid API key."
|
||||
|
||||
|
||||
def test_valid_response_returns_results(monkeypatch):
|
||||
results = [{"title": "t", "link": "u", "snippet": "s"}]
|
||||
monkeypatch.setattr(g_module, "GoogleSearch", lambda params: _FakeSearch({"organic_results": results}))
|
||||
g, captured, out = _make_tool()
|
||||
g._invoke(q="anything")
|
||||
assert captured["chunks"] == results
|
||||
assert captured["rendered"] == [{"title": "t", "url": "u", "content": "s"}]
|
||||
assert out["json"] == results
|
||||
|
||||
|
||||
def test_result_missing_optional_fields_does_not_raise(monkeypatch):
|
||||
# Regression: get_content's fallback `r["snippet"]` was evaluated eagerly, so a
|
||||
# result carrying a description but no snippet raised KeyError('snippet') -- the
|
||||
# very failure mode this tool's error handling exists to prevent. title and link
|
||||
# may be absent on a result too.
|
||||
results = [{"about_this_result": {"source": {"description": "d"}}}]
|
||||
monkeypatch.setattr(g_module, "GoogleSearch", lambda params: _FakeSearch({"organic_results": results}))
|
||||
g, captured, _ = _make_tool()
|
||||
g._invoke(q="anything")
|
||||
assert captured["rendered"] == [{"title": "", "url": "", "content": "d"}]
|
||||
Reference in New Issue
Block a user