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.
This commit is contained in:
chengyongru 2026-05-06 23:23:04 +08:00 committed by Xubin Ren
parent 4efd904ccc
commit 98c2f7cc27
2 changed files with 51 additions and 22 deletions

View File

@ -940,12 +940,8 @@ class WeixinChannel(BaseChannel):
async def send(self, msg: OutboundMessage) -> None: async def send(self, msg: OutboundMessage) -> None:
if not self._client or not self._token: if not self._client or not self._token:
self.logger.warning("client not initialized or not authenticated") raise RuntimeError("WeChat client not initialized or not authenticated")
return self._assert_session_active()
try:
self._assert_session_active()
except RuntimeError:
return
is_progress = bool((msg.metadata or {}).get("_progress", False)) is_progress = bool((msg.metadata or {}).get("_progress", False))
if not is_progress: if not is_progress:
@ -954,11 +950,9 @@ class WeixinChannel(BaseChannel):
content = msg.content.strip() content = msg.content.strip()
ctx_token = self._context_tokens.get(msg.chat_id, "") ctx_token = self._context_tokens.get(msg.chat_id, "")
if not ctx_token: if not ctx_token:
self.logger.warning( raise RuntimeError(
"no context_token for chat_id={}, cannot send", f"No context_token for chat_id={msg.chat_id}, cannot send"
msg.chat_id,
) )
return
typing_ticket = "" typing_ticket = ""
with suppress(Exception): with suppress(Exception):
@ -1128,10 +1122,8 @@ class WeixinChannel(BaseChannel):
data = await self._api_post("ilink/bot/sendmessage", body) data = await self._api_post("ilink/bot/sendmessage", body)
errcode = data.get("errcode", 0) errcode = data.get("errcode", 0)
if errcode and errcode != 0: if errcode and errcode != 0:
self.logger.warning( raise RuntimeError(
"send error (code {}): {}", f"WeChat send text error (code {errcode}): {data.get('errmsg', '')}"
errcode,
data.get("errmsg", ""),
) )
async def _send_media_file( async def _send_media_file(

View File

@ -319,21 +319,22 @@ async def test_process_message_does_not_fallback_when_top_level_media_exists_but
@pytest.mark.asyncio @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, _bus = _make_channel()
channel._client = object() channel._client = object()
channel._token = "token" channel._token = "token"
channel._send_text = AsyncMock() channel._send_text = AsyncMock()
await channel.send( with pytest.raises(RuntimeError, match="No context_token"):
type("Msg", (), {"chat_id": "unknown-user", "content": "pong", "media": [], "metadata": {}})() await channel.send(
) type("Msg", (), {"chat_id": "unknown-user", "content": "pong", "media": [], "metadata": {}})()
)
channel._send_text.assert_not_awaited() channel._send_text.assert_not_awaited()
@pytest.mark.asyncio @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, _bus = _make_channel()
channel._client = object() channel._client = object()
channel._token = "token" channel._token = "token"
@ -341,9 +342,10 @@ async def test_send_does_not_send_when_session_is_paused() -> None:
channel._pause_session(60) channel._pause_session(60)
channel._send_text = AsyncMock() channel._send_text = AsyncMock()
await channel.send( with pytest.raises(RuntimeError, match="session paused"):
type("Msg", (), {"chat_id": "wx-user", "content": "pong", "media": [], "metadata": {}})() await channel.send(
) type("Msg", (), {"chat_id": "wx-user", "content": "pong", "media": [], "metadata": {}})()
)
channel._send_text.assert_not_awaited() 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 # _send_media_file called once, _send_text never called
channel._send_media_file.assert_awaited_once() channel._send_media_file.assert_awaited_once()
channel._send_text.assert_not_awaited() 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()