mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-15 09:21:36 +00:00
fix(telegram): harden rich message fallback handling
Carry forward focused follow-ups from PR #45741: treat PTB's raw Bot API 10.1 response shapes safely, recognize real missing-endpoint errors, preserve link preview settings on rich sends, and lock the rich limit to Telegram's character-based cap.
This commit is contained in:
parent
0a865e5948
commit
57c2a55be4
2 changed files with 104 additions and 9 deletions
|
|
@ -1022,16 +1022,19 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||
rejections (BadRequest from a parser/limit issue) are NOT capability
|
||||
errors: the next message may be fine.
|
||||
"""
|
||||
name = exc.__class__.__name__.lower()
|
||||
if name in {"endpointnotfound", "invalidtoken"}:
|
||||
return True
|
||||
if isinstance(exc, (AttributeError, TypeError, NotImplementedError)):
|
||||
return True
|
||||
if getattr(exc, "error_code", None) == 404:
|
||||
return True
|
||||
s = str(exc).lower()
|
||||
if ("method" in s and "not found" in s) or "no such method" in s:
|
||||
if ("method" in s or "endpoint" in s) and (
|
||||
"not found" in s or "does not exist" in s
|
||||
):
|
||||
return True
|
||||
if "unsupported" in s or "not implemented" in s:
|
||||
return True
|
||||
return False
|
||||
return "no such method" in s
|
||||
|
||||
def _is_rich_fallback_error(self, exc: Exception) -> bool:
|
||||
"""True ⇒ permanent/capability error ⇒ safe to fall back to legacy.
|
||||
|
|
@ -1043,7 +1046,10 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||
"""
|
||||
if self._is_bad_request_error(exc):
|
||||
return True
|
||||
return self._is_rich_capability_error(exc)
|
||||
if self._is_rich_capability_error(exc):
|
||||
return True
|
||||
s = str(exc).lower()
|
||||
return "unsupported" in s or "not implemented" in s
|
||||
|
||||
def _compute_single_send_routing(
|
||||
self,
|
||||
|
|
@ -1116,6 +1122,8 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||
# which must not be sent as a stray field on the raw endpoint.
|
||||
payload.update({k: v for k, v in thread_kwargs.items() if v is not None})
|
||||
payload.update(self._notification_kwargs(metadata))
|
||||
if getattr(self, "_disable_link_previews", False):
|
||||
payload["link_preview_options"] = {"is_disabled": True}
|
||||
if reply_to_id is not None:
|
||||
# Spec: sendRichMessage takes reply_parameters (ReplyParameters
|
||||
# object), NOT the legacy reply_to_message_id scalar. Unknown
|
||||
|
|
@ -1124,8 +1132,12 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||
payload["reply_parameters"] = {"message_id": reply_to_id}
|
||||
|
||||
try:
|
||||
# Take the raw Bot API result (dict under real PTB). Passing
|
||||
# return_type=Message would make PTB deserialize a Bot API 10.1
|
||||
# response shape it does not fully model yet; a post-delivery parse
|
||||
# error must not be mistaken for a sendable failure.
|
||||
msg = await self._bot.do_api_request(
|
||||
"sendRichMessage", api_kwargs=payload, return_type=Message
|
||||
"sendRichMessage", api_kwargs=payload
|
||||
)
|
||||
except Exception as exc:
|
||||
if self._is_rich_fallback_error(exc):
|
||||
|
|
@ -1162,7 +1174,7 @@ class TelegramAdapter(BasePlatformAdapter):
|
|||
if isinstance(msg, dict):
|
||||
message_id = msg.get("message_id")
|
||||
if message_id is None:
|
||||
message_id = msg.get("result", {}).get("message_id")
|
||||
message_id = (msg.get("result") or {}).get("message_id")
|
||||
else:
|
||||
message_id = getattr(msg, "message_id", None)
|
||||
return SendResult(
|
||||
|
|
|
|||
|
|
@ -25,6 +25,20 @@ from telegram.error import BadRequest, NetworkError, TimedOut
|
|||
# and a task list. Pipes / brackets must survive untouched into the payload.
|
||||
RICH_CONTENT = "## Results\n\n| Case | Status |\n|---|---|\n| rich | ✅ |\n\n- [x] table renders"
|
||||
|
||||
# PTB 22.6's real unknown-endpoint errors: do_api_request can raise
|
||||
# EndPointNotFound for Bot API 404s, and the request layer can wrap that same
|
||||
# missing endpoint as InvalidToken. Use class names here so the tests don't
|
||||
# depend on optional PTB internals.
|
||||
EndPointNotFound = type("EndPointNotFound", (Exception,), {})
|
||||
InvalidToken = type("InvalidToken", (Exception,), {})
|
||||
PTB_ENDPOINT_NOT_FOUND = EndPointNotFound(
|
||||
"Endpoint 'sendRichMessage' not found in Bot API"
|
||||
)
|
||||
PTB_INVALID_TOKEN_404 = InvalidToken(
|
||||
"Either the bot token was rejected by Telegram or the endpoint "
|
||||
"'sendRichMessage' does not exist."
|
||||
)
|
||||
|
||||
|
||||
def _make_adapter(extra=None):
|
||||
"""Build a TelegramAdapter with a mock bot wired for the rich path."""
|
||||
|
|
@ -48,6 +62,31 @@ def _rich_api_kwargs(adapter):
|
|||
return call.kwargs["api_kwargs"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
("raw", "expected_id"),
|
||||
[
|
||||
(SimpleNamespace(message_id=123), "123"),
|
||||
({"message_id": 123}, "123"),
|
||||
({"result": {"message_id": 123}}, "123"),
|
||||
({"result": None}, None),
|
||||
],
|
||||
)
|
||||
async def test_rich_result_shapes_extract_message_id(raw, expected_id):
|
||||
"""The raw Bot API path may return either a PTB object or a raw dict."""
|
||||
adapter = _make_adapter()
|
||||
adapter._bot.do_api_request = AsyncMock(return_value=raw)
|
||||
|
||||
result = await adapter.send("12345", RICH_CONTENT)
|
||||
|
||||
assert result.success is True
|
||||
assert result.message_id == expected_id
|
||||
bot = adapter._bot
|
||||
assert bot is not None
|
||||
bot.do_api_request.assert_awaited_once()
|
||||
bot.send_message.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rich_happy_path_sends_raw_markdown():
|
||||
adapter = _make_adapter()
|
||||
|
|
@ -101,9 +140,9 @@ async def test_expect_edits_metadata_keeps_preview_on_legacy_path():
|
|||
@pytest.mark.asyncio
|
||||
async def test_oversized_content_skips_rich_and_chunks():
|
||||
adapter = _make_adapter()
|
||||
# > 32,768 UTF-8 bytes -> rich pre-check fails, legacy chunking takes over.
|
||||
# > 32,768 characters -> rich pre-check fails, legacy chunking takes over.
|
||||
oversized = "a" * 40000
|
||||
assert len(oversized.encode("utf-8")) > TelegramAdapter.RICH_MESSAGE_MAX_BYTES
|
||||
assert len(oversized) > TelegramAdapter.RICH_MESSAGE_MAX_CHARS
|
||||
|
||||
result = await adapter.send("12345", oversized)
|
||||
|
||||
|
|
@ -113,6 +152,23 @@ async def test_oversized_content_skips_rich_and_chunks():
|
|||
assert adapter._bot.send_message.await_count > 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rich_limit_is_characters_not_bytes():
|
||||
"""Telegram's rich limit is UTF-8 characters, not encoded bytes."""
|
||||
adapter = _make_adapter()
|
||||
cjk = "测" * 20000 # 20k chars, 60k UTF-8 bytes
|
||||
assert len(cjk.encode("utf-8")) > TelegramAdapter.RICH_MESSAGE_MAX_BYTES
|
||||
assert len(cjk) <= TelegramAdapter.RICH_MESSAGE_MAX_CHARS
|
||||
|
||||
result = await adapter.send("12345", cjk)
|
||||
|
||||
assert result.success is True
|
||||
bot = adapter._bot
|
||||
assert bot is not None
|
||||
bot.do_api_request.assert_awaited_once()
|
||||
bot.send_message.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"exc",
|
||||
|
|
@ -164,6 +220,33 @@ async def test_capability_error_latches_rich_send_off():
|
|||
adapter._bot.send_message.assert_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("exc", [PTB_ENDPOINT_NOT_FOUND, PTB_INVALID_TOKEN_404])
|
||||
async def test_real_ptb_endpoint_missing_falls_back_and_latches_off(exc):
|
||||
adapter = _make_adapter()
|
||||
adapter._bot.do_api_request = AsyncMock(side_effect=exc)
|
||||
|
||||
result = await adapter.send("12345", RICH_CONTENT)
|
||||
|
||||
assert result.success is True
|
||||
bot = adapter._bot
|
||||
assert bot is not None
|
||||
bot.do_api_request.assert_awaited_once()
|
||||
bot.send_message.assert_awaited()
|
||||
assert adapter._rich_send_disabled is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rich_payload_preserves_link_preview_disable():
|
||||
adapter = _make_adapter(extra={"disable_link_previews": True})
|
||||
|
||||
result = await adapter.send("12345", "See https://example.com")
|
||||
|
||||
assert result.success is True
|
||||
api_kwargs = _rich_api_kwargs(adapter)
|
||||
assert api_kwargs["link_preview_options"] == {"is_disabled": True}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_per_message_bad_request_does_not_latch_off():
|
||||
"""A parser/limit BadRequest is per-message — rich must stay enabled
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue