From 98c2f7cc27e7c42b55f8038719a5bf729f78e360 Mon Sep 17 00:00:00 2001 From: chengyongru <2755839590@qq.com> Date: Wed, 6 May 2026 23:23:04 +0800 Subject: [PATCH] fix(weixin): raise exceptions instead of silently dropping messages _send_text() swallowed API errors (non-zero errcode) with just a warning log, and send() had three silent return paths (no client, session paused, no context_token). Neither triggered ChannelManager's retry logic, causing persistent message loss until a new inbound message refreshed the context_token. Now all failure paths raise RuntimeError, matching BaseChannel's contract and enabling proper retry behavior. --- nanobot/channels/weixin.py | 20 +++------- tests/channels/test_weixin_channel.py | 53 +++++++++++++++++++++++---- 2 files changed, 51 insertions(+), 22 deletions(-) diff --git a/nanobot/channels/weixin.py b/nanobot/channels/weixin.py index 698acc70e..cd46775f9 100644 --- a/nanobot/channels/weixin.py +++ b/nanobot/channels/weixin.py @@ -940,12 +940,8 @@ class WeixinChannel(BaseChannel): async def send(self, msg: OutboundMessage) -> None: if not self._client or not self._token: - self.logger.warning("client not initialized or not authenticated") - return - try: - self._assert_session_active() - except RuntimeError: - return + raise RuntimeError("WeChat client not initialized or not authenticated") + self._assert_session_active() is_progress = bool((msg.metadata or {}).get("_progress", False)) if not is_progress: @@ -954,11 +950,9 @@ class WeixinChannel(BaseChannel): content = msg.content.strip() ctx_token = self._context_tokens.get(msg.chat_id, "") if not ctx_token: - self.logger.warning( - "no context_token for chat_id={}, cannot send", - msg.chat_id, + raise RuntimeError( + f"No context_token for chat_id={msg.chat_id}, cannot send" ) - return typing_ticket = "" with suppress(Exception): @@ -1128,10 +1122,8 @@ class WeixinChannel(BaseChannel): data = await self._api_post("ilink/bot/sendmessage", body) errcode = data.get("errcode", 0) if errcode and errcode != 0: - self.logger.warning( - "send error (code {}): {}", - errcode, - data.get("errmsg", ""), + raise RuntimeError( + f"WeChat send text error (code {errcode}): {data.get('errmsg', '')}" ) async def _send_media_file( diff --git a/tests/channels/test_weixin_channel.py b/tests/channels/test_weixin_channel.py index 4b9b294a9..1cfeb9dd3 100644 --- a/tests/channels/test_weixin_channel.py +++ b/tests/channels/test_weixin_channel.py @@ -319,21 +319,22 @@ async def test_process_message_does_not_fallback_when_top_level_media_exists_but @pytest.mark.asyncio -async def test_send_without_context_token_does_not_send_text() -> None: +async def test_send_without_context_token_raises() -> None: channel, _bus = _make_channel() channel._client = object() channel._token = "token" channel._send_text = AsyncMock() - await channel.send( - type("Msg", (), {"chat_id": "unknown-user", "content": "pong", "media": [], "metadata": {}})() - ) + with pytest.raises(RuntimeError, match="No context_token"): + await channel.send( + type("Msg", (), {"chat_id": "unknown-user", "content": "pong", "media": [], "metadata": {}})() + ) channel._send_text.assert_not_awaited() @pytest.mark.asyncio -async def test_send_does_not_send_when_session_is_paused() -> None: +async def test_send_raises_when_session_is_paused() -> None: channel, _bus = _make_channel() channel._client = object() channel._token = "token" @@ -341,9 +342,10 @@ async def test_send_does_not_send_when_session_is_paused() -> None: channel._pause_session(60) channel._send_text = AsyncMock() - await channel.send( - type("Msg", (), {"chat_id": "wx-user", "content": "pong", "media": [], "metadata": {}})() - ) + with pytest.raises(RuntimeError, match="session paused"): + await channel.send( + type("Msg", (), {"chat_id": "wx-user", "content": "pong", "media": [], "metadata": {}})() + ) channel._send_text.assert_not_awaited() @@ -1213,3 +1215,38 @@ async def test_send_media_network_error_does_not_double_api_calls() -> None: # _send_media_file called once, _send_text never called channel._send_media_file.assert_awaited_once() channel._send_text.assert_not_awaited() + + +# --------------------------------------------------------------------------- +# Tests for _send_text raising on API errors (previously silently swallowed) +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_send_text_raises_on_api_error() -> None: + """_send_text must raise RuntimeError when the API returns a non-zero errcode, + matching _send_media_file behavior. This ensures ChannelManager can retry.""" + channel, _bus = _make_channel() + channel._client = httpx.AsyncClient() + channel._token = "token" + channel._api_post = AsyncMock( + return_value={"errcode": -14, "errmsg": "session expired"} + ) + + with pytest.raises(RuntimeError, match="WeChat send text error.*-14"): + await channel._send_text("wx-user", "hello", "ctx-expired") + + channel._api_post.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_send_text_succeeds_on_zero_errcode() -> None: + """_send_text must NOT raise when errcode is 0.""" + channel, _bus = _make_channel() + channel._client = httpx.AsyncClient() + channel._token = "token" + channel._api_post = AsyncMock(return_value={"errcode": 0}) + + await channel._send_text("wx-user", "hello", "ctx-ok") + + channel._api_post.assert_awaited_once()