mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-26 01:01:40 +00:00
fix: handle empty choices in MCP sampling callback
SamplingHandler.__call__ accessed response.choices[0] without checking if the list was non-empty. LLM APIs can return empty choices on content filtering, provider errors, or rate limits, causing an unhandled IndexError that propagates to the MCP SDK and may crash the connection. Add a defensive guard that returns a proper ErrorData when choices is empty, None, or missing. Includes three test cases covering all variants.
This commit is contained in:
parent
a34102049b
commit
4e3a8a0637
2 changed files with 67 additions and 0 deletions
|
|
@ -2049,6 +2049,65 @@ class TestSamplingErrors:
|
||||||
assert "No LLM provider" in result.message
|
assert "No LLM provider" in result.message
|
||||||
assert handler.metrics["errors"] == 1
|
assert handler.metrics["errors"] == 1
|
||||||
|
|
||||||
|
def test_empty_choices_returns_error(self):
|
||||||
|
"""LLM returning choices=[] is handled gracefully, not IndexError."""
|
||||||
|
handler = SamplingHandler("ec", {})
|
||||||
|
fake_client = MagicMock()
|
||||||
|
fake_client.chat.completions.create.return_value = SimpleNamespace(
|
||||||
|
choices=[],
|
||||||
|
model="test-model",
|
||||||
|
usage=SimpleNamespace(total_tokens=0),
|
||||||
|
)
|
||||||
|
|
||||||
|
with patch(
|
||||||
|
"agent.auxiliary_client.get_text_auxiliary_client",
|
||||||
|
return_value=(fake_client, "default-model"),
|
||||||
|
):
|
||||||
|
result = asyncio.run(handler(None, _make_sampling_params()))
|
||||||
|
|
||||||
|
assert isinstance(result, ErrorData)
|
||||||
|
assert "empty response" in result.message.lower()
|
||||||
|
assert handler.metrics["errors"] == 1
|
||||||
|
|
||||||
|
def test_none_choices_returns_error(self):
|
||||||
|
"""LLM returning choices=None is handled gracefully, not TypeError."""
|
||||||
|
handler = SamplingHandler("nc", {})
|
||||||
|
fake_client = MagicMock()
|
||||||
|
fake_client.chat.completions.create.return_value = SimpleNamespace(
|
||||||
|
choices=None,
|
||||||
|
model="test-model",
|
||||||
|
usage=SimpleNamespace(total_tokens=0),
|
||||||
|
)
|
||||||
|
|
||||||
|
with patch(
|
||||||
|
"agent.auxiliary_client.get_text_auxiliary_client",
|
||||||
|
return_value=(fake_client, "default-model"),
|
||||||
|
):
|
||||||
|
result = asyncio.run(handler(None, _make_sampling_params()))
|
||||||
|
|
||||||
|
assert isinstance(result, ErrorData)
|
||||||
|
assert "empty response" in result.message.lower()
|
||||||
|
assert handler.metrics["errors"] == 1
|
||||||
|
|
||||||
|
def test_missing_choices_attr_returns_error(self):
|
||||||
|
"""LLM response without choices attribute is handled gracefully."""
|
||||||
|
handler = SamplingHandler("mc", {})
|
||||||
|
fake_client = MagicMock()
|
||||||
|
fake_client.chat.completions.create.return_value = SimpleNamespace(
|
||||||
|
model="test-model",
|
||||||
|
usage=SimpleNamespace(total_tokens=0),
|
||||||
|
)
|
||||||
|
|
||||||
|
with patch(
|
||||||
|
"agent.auxiliary_client.get_text_auxiliary_client",
|
||||||
|
return_value=(fake_client, "default-model"),
|
||||||
|
):
|
||||||
|
result = asyncio.run(handler(None, _make_sampling_params()))
|
||||||
|
|
||||||
|
assert isinstance(result, ErrorData)
|
||||||
|
assert "empty response" in result.message.lower()
|
||||||
|
assert handler.metrics["errors"] == 1
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# 10. Model whitelist
|
# 10. Model whitelist
|
||||||
|
|
|
||||||
|
|
@ -538,6 +538,14 @@ class SamplingHandler:
|
||||||
f"Sampling LLM call failed: {_sanitize_error(str(exc))}"
|
f"Sampling LLM call failed: {_sanitize_error(str(exc))}"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Guard against empty choices (content filtering, provider errors)
|
||||||
|
if not getattr(response, "choices", None):
|
||||||
|
self.metrics["errors"] += 1
|
||||||
|
return self._error(
|
||||||
|
f"LLM returned empty response (no choices) for server "
|
||||||
|
f"'{self.server_name}'"
|
||||||
|
)
|
||||||
|
|
||||||
# Track metrics
|
# Track metrics
|
||||||
choice = response.choices[0]
|
choice = response.choices[0]
|
||||||
self.metrics["requests"] += 1
|
self.metrics["requests"] += 1
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue