From 57c2a55be43b9eb184527464de9691b7643095c7 Mon Sep 17 00:00:00 2001 From: ITheEqualizer Date: Sat, 13 Jun 2026 13:53:36 -0700 Subject: [PATCH] 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. --- gateway/platforms/telegram.py | 26 ++++-- tests/gateway/test_telegram_rich_messages.py | 87 +++++++++++++++++++- 2 files changed, 104 insertions(+), 9 deletions(-) diff --git a/gateway/platforms/telegram.py b/gateway/platforms/telegram.py index 5bb4d5bbcbe..b8741edab99 100644 --- a/gateway/platforms/telegram.py +++ b/gateway/platforms/telegram.py @@ -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( diff --git a/tests/gateway/test_telegram_rich_messages.py b/tests/gateway/test_telegram_rich_messages.py index 70893bbd7a4..0e2efe45caf 100644 --- a/tests/gateway/test_telegram_rich_messages.py +++ b/tests/gateway/test_telegram_rich_messages.py @@ -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