diff --git a/nanobot/channels/dingtalk.py b/nanobot/channels/dingtalk.py index 39b5818bd..a863ba0df 100644 --- a/nanobot/channels/dingtalk.py +++ b/nanobot/channels/dingtalk.py @@ -337,6 +337,9 @@ class DingTalkChannel(BaseChannel): content_type = (resp.headers.get("content-type") or "").split(";")[0].strip() filename = self._guess_filename(media_ref, self._guess_upload_type(media_ref)) return resp.content, filename, content_type or None + except httpx.TransportError as e: + logger.error("DingTalk media download network error ref={} err={}", media_ref, e) + raise except Exception as e: logger.error("DingTalk media download error ref={} err={}", media_ref, e) return None, None, None @@ -388,6 +391,9 @@ class DingTalkChannel(BaseChannel): logger.error("DingTalk media upload missing media_id body={}", text[:500]) return None return str(media_id) + except httpx.TransportError as e: + logger.error("DingTalk media upload network error type={} err={}", media_type, e) + raise except Exception as e: logger.error("DingTalk media upload error type={} err={}", media_type, e) return None @@ -437,6 +443,9 @@ class DingTalkChannel(BaseChannel): return False logger.debug("DingTalk message sent to {} with msgKey={}", chat_id, msg_key) return True + except httpx.TransportError as e: + logger.error("DingTalk network error sending message msgKey={} err={}", msg_key, e) + raise except Exception as e: logger.error("Error sending DingTalk message msgKey={} err={}", msg_key, e) return False diff --git a/nanobot/channels/discord.py b/nanobot/channels/discord.py index 6e8c673a3..336b6148d 100644 --- a/nanobot/channels/discord.py +++ b/nanobot/channels/discord.py @@ -366,6 +366,7 @@ class DiscordChannel(BaseChannel): await client.send_outbound(msg) except Exception as e: logger.error("Error sending Discord message: {}", e) + raise finally: if not is_progress: await self._stop_typing(msg.chat_id) diff --git a/nanobot/channels/qq.py b/nanobot/channels/qq.py index 484eed6e2..96d9d5ecd 100644 --- a/nanobot/channels/qq.py +++ b/nanobot/channels/qq.py @@ -362,7 +362,12 @@ class QQChannel(BaseChannel): logger.info("QQ media sent: {}", filename) return True + except (aiohttp.ClientError, OSError) as e: + # Network / transport errors — propagate for retry by caller + logger.warning("QQ send media network error filename={} err={}", filename, e) + raise except Exception as e: + # API-level or other non-network errors — return False so send() can fallback logger.error("QQ send media failed filename={} err={}", filename, e) return False diff --git a/nanobot/channels/telegram.py b/nanobot/channels/telegram.py index d2572fac3..f63704aa7 100644 --- a/nanobot/channels/telegram.py +++ b/nanobot/channels/telegram.py @@ -570,7 +570,10 @@ class TelegramChannel(BaseChannel): chat_id=int_chat_id, message_id=buf.message_id, text=html, parse_mode="HTML", ) - except Exception as e: + except BadRequest as e: + # Only fall back to plain text on actual HTML parse/format errors. + # Network errors (TimedOut, NetworkError) should propagate immediately + # to avoid doubling connection demand during pool exhaustion. if self._is_not_modified_error(e): logger.debug("Final stream edit already applied for {}", chat_id) self._stream_bufs.pop(chat_id, None) diff --git a/nanobot/channels/weixin.py b/nanobot/channels/weixin.py index 3f87e2203..fbe84bcf8 100644 --- a/nanobot/channels/weixin.py +++ b/nanobot/channels/weixin.py @@ -985,7 +985,43 @@ class WeixinChannel(BaseChannel): for media_path in (msg.media or []): try: await self._send_media_file(msg.chat_id, media_path, ctx_token) + except (httpx.TimeoutException, httpx.TransportError) as net_err: + # Network/transport errors: do NOT fall back to text — + # the text send would also likely fail, and the outer + # except will re-raise so ChannelManager retries properly. + logger.error( + "Network error sending WeChat media {}: {}", + media_path, + net_err, + ) + raise + except httpx.HTTPStatusError as http_err: + status_code = ( + http_err.response.status_code + if http_err.response is not None + else 0 + ) + if status_code >= 500: + # Server-side / retryable HTTP error — same as network. + logger.error( + "Server error ({} {}) sending WeChat media {}: {}", + status_code, + http_err.response.reason_phrase + if http_err.response is not None + else "", + media_path, + http_err, + ) + raise + # 4xx client errors are NOT retryable — fall back to text. + filename = Path(media_path).name + logger.error("Failed to send WeChat media {}: {}", media_path, http_err) + await self._send_text( + msg.chat_id, f"[Failed to send: {filename}]", ctx_token, + ) except Exception as e: + # Non-network errors (format, file-not-found, etc.): + # notify the user via text fallback. filename = Path(media_path).name logger.error("Failed to send WeChat media {}: {}", media_path, e) # Notify user about failure via text diff --git a/tests/channels/test_dingtalk_channel.py b/tests/channels/test_dingtalk_channel.py index f743c4e62..86de99bb5 100644 --- a/tests/channels/test_dingtalk_channel.py +++ b/tests/channels/test_dingtalk_channel.py @@ -2,7 +2,9 @@ import asyncio import zipfile from io import BytesIO from types import SimpleNamespace +from unittest.mock import AsyncMock +import httpx import pytest # Check optional dingtalk dependencies before running tests @@ -52,6 +54,21 @@ class _FakeHttp: return self._next_response() +class _NetworkErrorHttp: + """HTTP client stub that raises httpx.TransportError on every request.""" + + def __init__(self) -> None: + self.calls: list[dict] = [] + + async def post(self, url: str, json=None, headers=None, **kwargs): + self.calls.append({"method": "POST", "url": url, "json": json, "headers": headers}) + raise httpx.ConnectError("Connection refused") + + async def get(self, url: str, **kwargs): + self.calls.append({"method": "GET", "url": url}) + raise httpx.ConnectError("Connection refused") + + @pytest.mark.asyncio async def test_group_message_keeps_sender_id_and_routes_chat_id() -> None: config = DingTalkConfig(client_id="app", client_secret="secret", allow_from=["user1"]) @@ -298,3 +315,141 @@ async def test_send_media_ref_zips_html_before_upload(tmp_path, monkeypatch) -> archive = zipfile.ZipFile(BytesIO(captured["data"])) assert archive.namelist() == ["report.html"] + + +# ── Exception handling tests ────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_send_batch_message_propagates_transport_error() -> None: + """Network/transport errors must re-raise so callers can retry.""" + config = DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]) + channel = DingTalkChannel(config, MessageBus()) + channel._http = _NetworkErrorHttp() + + with pytest.raises(httpx.ConnectError, match="Connection refused"): + await channel._send_batch_message( + "token", + "user123", + "sampleMarkdown", + {"text": "hello", "title": "Nanobot Reply"}, + ) + + # The POST was attempted exactly once + assert len(channel._http.calls) == 1 + assert channel._http.calls[0]["method"] == "POST" + + +@pytest.mark.asyncio +async def test_send_batch_message_returns_false_on_api_error() -> None: + """DingTalk API-level errors (non-200 status, errcode != 0) should return False.""" + config = DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]) + channel = DingTalkChannel(config, MessageBus()) + + # Non-200 status code → API error → return False + channel._http = _FakeHttp(responses=[_FakeResponse(400, {"errcode": 400})]) + result = await channel._send_batch_message( + "token", "user123", "sampleMarkdown", {"text": "hello"} + ) + assert result is False + + # 200 with non-zero errcode → API error → return False + channel._http = _FakeHttp(responses=[_FakeResponse(200, {"errcode": 100})]) + result = await channel._send_batch_message( + "token", "user123", "sampleMarkdown", {"text": "hello"} + ) + assert result is False + + # 200 with errcode=0 → success → return True + channel._http = _FakeHttp(responses=[_FakeResponse(200, {"errcode": 0})]) + result = await channel._send_batch_message( + "token", "user123", "sampleMarkdown", {"text": "hello"} + ) + assert result is True + + +@pytest.mark.asyncio +async def test_send_media_ref_short_circuits_on_transport_error() -> None: + """When the first send fails with a transport error, _send_media_ref must + re-raise immediately instead of trying download+upload+fallback.""" + config = DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]) + channel = DingTalkChannel(config, MessageBus()) + channel._http = _NetworkErrorHttp() + + # An image URL triggers the sampleImageMsg path first + with pytest.raises(httpx.ConnectError, match="Connection refused"): + await channel._send_media_ref("token", "user123", "https://example.com/photo.jpg") + + # Only one POST should have been attempted — no download/upload/fallback + assert len(channel._http.calls) == 1 + assert channel._http.calls[0]["method"] == "POST" + + +@pytest.mark.asyncio +async def test_send_media_ref_short_circuits_on_download_transport_error() -> None: + """When the image URL send returns an API error (False) but the download + for the fallback hits a transport error, it must re-raise rather than + silently returning False.""" + config = DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]) + channel = DingTalkChannel(config, MessageBus()) + + # First POST (sampleImageMsg) returns API error → False, then GET (download) raises transport error + class _MixedHttp: + def __init__(self) -> None: + self.calls: list[dict] = [] + + async def post(self, url, json=None, headers=None, **kwargs): + self.calls.append({"method": "POST", "url": url}) + # API-level failure: 200 with errcode != 0 + return _FakeResponse(200, {"errcode": 100}) + + async def get(self, url, **kwargs): + self.calls.append({"method": "GET", "url": url}) + raise httpx.ConnectError("Connection refused") + + channel._http = _MixedHttp() + + with pytest.raises(httpx.ConnectError, match="Connection refused"): + await channel._send_media_ref("token", "user123", "https://example.com/photo.jpg") + + # Should have attempted POST (image URL) and GET (download), but NOT upload + assert len(channel._http.calls) == 2 + assert channel._http.calls[0]["method"] == "POST" + assert channel._http.calls[1]["method"] == "GET" + + +@pytest.mark.asyncio +async def test_send_media_ref_short_circuits_on_upload_transport_error() -> None: + """When download succeeds but upload hits a transport error, must re-raise.""" + config = DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]) + channel = DingTalkChannel(config, MessageBus()) + + image_bytes = b"\xff\xd8\xff\xe0" + b"\x00" * 100 # minimal JPEG-ish data + + class _UploadFailsHttp: + def __init__(self) -> None: + self.calls: list[dict] = [] + + async def post(self, url, json=None, headers=None, files=None, **kwargs): + self.calls.append({"method": "POST", "url": url}) + # If it's the upload endpoint, raise transport error + if "media/upload" in url: + raise httpx.ConnectError("Connection refused") + # Otherwise (sampleImageMsg), return API error to trigger fallback + return _FakeResponse(200, {"errcode": 100}) + + async def get(self, url, **kwargs): + self.calls.append({"method": "GET", "url": url}) + resp = _FakeResponse(200) + resp.content = image_bytes + resp.headers = {"content-type": "image/jpeg"} + return resp + + channel._http = _UploadFailsHttp() + + with pytest.raises(httpx.ConnectError, match="Connection refused"): + await channel._send_media_ref("token", "user123", "https://example.com/photo.jpg") + + # POST (image URL), GET (download), POST (upload) attempted — no further sends + methods = [c["method"] for c in channel._http.calls] + assert methods == ["POST", "GET", "POST"] diff --git a/tests/channels/test_discord_channel.py b/tests/channels/test_discord_channel.py index 3a31a5912..7a39bff2b 100644 --- a/tests/channels/test_discord_channel.py +++ b/tests/channels/test_discord_channel.py @@ -867,3 +867,100 @@ async def test_start_no_proxy_auth_when_only_password(monkeypatch) -> None: assert channel.is_running is False assert _FakeDiscordClient.instances[0].proxy == "http://127.0.0.1:7890" assert _FakeDiscordClient.instances[0].proxy_auth is None + + +# --------------------------------------------------------------------------- +# Tests for the send() exception propagation fix +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_send_re_raises_network_error() -> None: + """Network errors during send must propagate so ChannelManager can retry.""" + channel = DiscordChannel(DiscordConfig(enabled=True, allow_from=["*"]), MessageBus()) + client = _FakeDiscordClient(channel, intents=None) + channel._client = client + channel._running = True + + async def _failing_send_outbound(msg: OutboundMessage) -> None: + raise ConnectionError("network unreachable") + + client.send_outbound = _failing_send_outbound # type: ignore[method-assign] + + with pytest.raises(ConnectionError, match="network unreachable"): + await channel.send(OutboundMessage(channel="discord", chat_id="123", content="hello")) + + +@pytest.mark.asyncio +async def test_send_re_raises_generic_exception() -> None: + """Any exception from send_outbound must propagate, not be swallowed.""" + channel = DiscordChannel(DiscordConfig(enabled=True, allow_from=["*"]), MessageBus()) + client = _FakeDiscordClient(channel, intents=None) + channel._client = client + channel._running = True + + async def _failing_send_outbound(msg: OutboundMessage) -> None: + raise RuntimeError("discord API failure") + + client.send_outbound = _failing_send_outbound # type: ignore[method-assign] + + with pytest.raises(RuntimeError, match="discord API failure"): + await channel.send(OutboundMessage(channel="discord", chat_id="123", content="hello")) + + +@pytest.mark.asyncio +async def test_send_still_stops_typing_on_error() -> None: + """Typing cleanup must still run in the finally block even when send raises.""" + channel = DiscordChannel(DiscordConfig(enabled=True, allow_from=["*"]), MessageBus()) + client = _FakeDiscordClient(channel, intents=None) + channel._client = client + channel._running = True + + # Start a typing task so we can verify it gets cleaned up + start = asyncio.Event() + release = asyncio.Event() + + async def slow_typing() -> None: + start.set() + await release.wait() + + typing_channel = _FakeChannel(channel_id=123) + typing_channel.typing_enter_hook = slow_typing + await channel._start_typing(typing_channel) + await asyncio.wait_for(start.wait(), timeout=1.0) + + async def _failing_send_outbound(msg: OutboundMessage) -> None: + raise ConnectionError("timeout") + + client.send_outbound = _failing_send_outbound # type: ignore[method-assign] + + with pytest.raises(ConnectionError, match="timeout"): + await channel.send(OutboundMessage(channel="discord", chat_id="123", content="hello")) + + release.set() + await asyncio.sleep(0) + + # Typing should have been cleaned up by the finally block + assert channel._typing_tasks == {} + + +@pytest.mark.asyncio +async def test_send_succeeds_normally() -> None: + """Successful sends should work without raising.""" + channel = DiscordChannel(DiscordConfig(enabled=True, allow_from=["*"]), MessageBus()) + client = _FakeDiscordClient(channel, intents=None) + channel._client = client + channel._running = True + + sent_messages: list[OutboundMessage] = [] + + async def _capture_send_outbound(msg: OutboundMessage) -> None: + sent_messages.append(msg) + + client.send_outbound = _capture_send_outbound # type: ignore[method-assign] + + msg = OutboundMessage(channel="discord", chat_id="123", content="hello world") + await channel.send(msg) + + assert len(sent_messages) == 1 + assert sent_messages[0].content == "hello world" + assert sent_messages[0].chat_id == "123" diff --git a/tests/channels/test_qq_channel.py b/tests/channels/test_qq_channel.py index 729442a13..417648adf 100644 --- a/tests/channels/test_qq_channel.py +++ b/tests/channels/test_qq_channel.py @@ -1,6 +1,7 @@ import tempfile from pathlib import Path from types import SimpleNamespace +from unittest.mock import AsyncMock, patch import pytest @@ -14,6 +15,8 @@ except ImportError: if not QQ_AVAILABLE: pytest.skip("QQ dependencies not installed (qq-botpy)", allow_module_level=True) +import aiohttp + from nanobot.bus.events import OutboundMessage from nanobot.bus.queue import MessageBus from nanobot.channels.qq import QQChannel, QQConfig @@ -170,3 +173,221 @@ async def test_read_media_bytes_missing_file() -> None: data, filename = await channel._read_media_bytes("/nonexistent/path/image.png") assert data is None assert filename is None + + +# ------------------------------------------------------- +# Tests for _send_media exception handling +# ------------------------------------------------------- + +def _make_channel_with_local_file(suffix: str = ".png", content: bytes = b"\x89PNG\r\n"): + """Create a QQChannel with a fake client and a temp file for media.""" + channel = QQChannel( + QQConfig(app_id="app", secret="secret", allow_from=["*"]), + MessageBus(), + ) + channel._client = _FakeClient() + channel._chat_type_cache["user1"] = "c2c" + + tmp = tempfile.NamedTemporaryFile(suffix=suffix, delete=False) + tmp.write(content) + tmp.close() + return channel, tmp.name + + +@pytest.mark.asyncio +async def test_send_media_network_error_propagates() -> None: + """aiohttp.ClientError (network/transport) should re-raise, not return False.""" + channel, tmp_path = _make_channel_with_local_file() + + # Make the base64 upload raise a network error + channel._client.api._http = SimpleNamespace() + channel._client.api._http.request = AsyncMock( + side_effect=aiohttp.ServerDisconnectedError("connection lost"), + ) + + with pytest.raises(aiohttp.ServerDisconnectedError): + await channel._send_media( + chat_id="user1", + media_ref=tmp_path, + msg_id="msg1", + is_group=False, + ) + + +@pytest.mark.asyncio +async def test_send_media_client_connector_error_propagates() -> None: + """aiohttp.ClientConnectorError (DNS/connection refused) should re-raise.""" + channel, tmp_path = _make_channel_with_local_file() + + from aiohttp.client_reqrep import ConnectionKey + conn_key = ConnectionKey("api.qq.com", 443, True, None, None, None, None) + connector_error = aiohttp.ClientConnectorError( + connection_key=conn_key, + os_error=OSError("Connection refused"), + ) + + channel._client.api._http = SimpleNamespace() + channel._client.api._http.request = AsyncMock( + side_effect=connector_error, + ) + + with pytest.raises(aiohttp.ClientConnectorError): + await channel._send_media( + chat_id="user1", + media_ref=tmp_path, + msg_id="msg1", + is_group=False, + ) + + +@pytest.mark.asyncio +async def test_send_media_oserror_propagates() -> None: + """OSError (low-level I/O) should re-raise for retry.""" + channel, tmp_path = _make_channel_with_local_file() + + channel._client.api._http = SimpleNamespace() + channel._client.api._http.request = AsyncMock( + side_effect=OSError("Network is unreachable"), + ) + + with pytest.raises(OSError): + await channel._send_media( + chat_id="user1", + media_ref=tmp_path, + msg_id="msg1", + is_group=False, + ) + + +@pytest.mark.asyncio +async def test_send_media_api_error_returns_false() -> None: + """API-level errors (botpy RuntimeError subclasses) should return False, not raise.""" + channel, tmp_path = _make_channel_with_local_file() + + # Simulate a botpy API error (e.g. ServerError is a RuntimeError subclass) + from botpy.errors import ServerError + + channel._client.api._http = SimpleNamespace() + channel._client.api._http.request = AsyncMock( + side_effect=ServerError("internal server error"), + ) + + result = await channel._send_media( + chat_id="user1", + media_ref=tmp_path, + msg_id="msg1", + is_group=False, + ) + assert result is False + + +@pytest.mark.asyncio +async def test_send_media_generic_runtime_error_returns_false() -> None: + """Generic RuntimeError (not network) should return False.""" + channel, tmp_path = _make_channel_with_local_file() + + channel._client.api._http = SimpleNamespace() + channel._client.api._http.request = AsyncMock( + side_effect=RuntimeError("some API error"), + ) + + result = await channel._send_media( + chat_id="user1", + media_ref=tmp_path, + msg_id="msg1", + is_group=False, + ) + assert result is False + + +@pytest.mark.asyncio +async def test_send_media_value_error_returns_false() -> None: + """ValueError (bad API response data) should return False.""" + channel, tmp_path = _make_channel_with_local_file() + + channel._client.api._http = SimpleNamespace() + channel._client.api._http.request = AsyncMock( + side_effect=ValueError("bad response data"), + ) + + result = await channel._send_media( + chat_id="user1", + media_ref=tmp_path, + msg_id="msg1", + is_group=False, + ) + assert result is False + + +@pytest.mark.asyncio +async def test_send_media_timeout_error_propagates() -> None: + """asyncio.TimeoutError inherits from Exception but not ClientError/OSError. + However, aiohttp.ServerTimeoutError IS a ClientError subclass, so that propagates. + For a plain TimeoutError (which is also OSError in Python 3.11+), it should propagate.""" + channel, tmp_path = _make_channel_with_local_file() + + channel._client.api._http = SimpleNamespace() + channel._client.api._http.request = AsyncMock( + side_effect=aiohttp.ServerTimeoutError("request timed out"), + ) + + with pytest.raises(aiohttp.ServerTimeoutError): + await channel._send_media( + chat_id="user1", + media_ref=tmp_path, + msg_id="msg1", + is_group=False, + ) + + +@pytest.mark.asyncio +async def test_send_fallback_text_on_api_error() -> None: + """When _send_media returns False (API error), send() should emit fallback text.""" + channel, tmp_path = _make_channel_with_local_file() + + from botpy.errors import ServerError + + channel._client.api._http = SimpleNamespace() + channel._client.api._http.request = AsyncMock( + side_effect=ServerError("internal server error"), + ) + + await channel.send( + OutboundMessage( + channel="qq", + chat_id="user1", + content="", + media=[tmp_path], + metadata={"message_id": "msg1"}, + ) + ) + + # Should have sent a fallback text message + assert len(channel._client.api.c2c_calls) == 1 + fallback_content = channel._client.api.c2c_calls[0]["content"] + assert "Attachment send failed" in fallback_content + + +@pytest.mark.asyncio +async def test_send_propagates_network_error_no_fallback() -> None: + """When _send_media raises a network error, send() should NOT silently fallback.""" + channel, tmp_path = _make_channel_with_local_file() + + channel._client.api._http = SimpleNamespace() + channel._client.api._http.request = AsyncMock( + side_effect=aiohttp.ServerDisconnectedError("connection lost"), + ) + + with pytest.raises(aiohttp.ServerDisconnectedError): + await channel.send( + OutboundMessage( + channel="qq", + chat_id="user1", + content="hello", + media=[tmp_path], + metadata={"message_id": "msg1"}, + ) + ) + + # No fallback text should have been sent + assert len(channel._client.api.c2c_calls) == 0 diff --git a/tests/channels/test_telegram_channel.py b/tests/channels/test_telegram_channel.py index 7dfb094f9..8d9431ba6 100644 --- a/tests/channels/test_telegram_channel.py +++ b/tests/channels/test_telegram_channel.py @@ -387,6 +387,84 @@ async def test_send_delta_stream_end_treats_not_modified_as_success() -> None: assert "123" not in channel._stream_bufs +@pytest.mark.asyncio +async def test_send_delta_stream_end_does_not_fallback_on_network_timeout() -> None: + """TimedOut during HTML edit should propagate, never fall back to plain text.""" + from telegram.error import TimedOut + + channel = TelegramChannel( + TelegramConfig(enabled=True, token="123:abc", allow_from=["*"]), + MessageBus(), + ) + channel._app = _FakeApp(lambda: None) + # _call_with_retry retries TimedOut up to 3 times, so the mock will be called + # multiple times – but all calls must be with parse_mode="HTML" (no plain fallback). + channel._app.bot.edit_message_text = AsyncMock(side_effect=TimedOut("network timeout")) + channel._stream_bufs["123"] = _StreamBuf(text="hello", message_id=7, last_edit=0.0) + + with pytest.raises(TimedOut, match="network timeout"): + await channel.send_delta("123", "", {"_stream_end": True}) + + # Every call to edit_message_text must have used parse_mode="HTML" — + # no plain-text fallback call should have been made. + for call in channel._app.bot.edit_message_text.call_args_list: + assert call.kwargs.get("parse_mode") == "HTML" + # Buffer should still be present (not cleaned up on error) + assert "123" in channel._stream_bufs + + +@pytest.mark.asyncio +async def test_send_delta_stream_end_does_not_fallback_on_network_error() -> None: + """NetworkError during HTML edit should propagate, never fall back to plain text.""" + from telegram.error import NetworkError + + channel = TelegramChannel( + TelegramConfig(enabled=True, token="123:abc", allow_from=["*"]), + MessageBus(), + ) + channel._app = _FakeApp(lambda: None) + channel._app.bot.edit_message_text = AsyncMock(side_effect=NetworkError("connection reset")) + channel._stream_bufs["123"] = _StreamBuf(text="hello", message_id=7, last_edit=0.0) + + with pytest.raises(NetworkError, match="connection reset"): + await channel.send_delta("123", "", {"_stream_end": True}) + + # Every call to edit_message_text must have used parse_mode="HTML" — + # no plain-text fallback call should have been made. + for call in channel._app.bot.edit_message_text.call_args_list: + assert call.kwargs.get("parse_mode") == "HTML" + # Buffer should still be present (not cleaned up on error) + assert "123" in channel._stream_bufs + + +@pytest.mark.asyncio +async def test_send_delta_stream_end_falls_back_on_bad_request() -> None: + """BadRequest (HTML parse error) should still trigger plain-text fallback.""" + from telegram.error import BadRequest + + channel = TelegramChannel( + TelegramConfig(enabled=True, token="123:abc", allow_from=["*"]), + MessageBus(), + ) + channel._app = _FakeApp(lambda: None) + + # First call (HTML) raises BadRequest, second call (plain) succeeds + channel._app.bot.edit_message_text = AsyncMock( + side_effect=[BadRequest("Can't parse entities"), None] + ) + channel._stream_bufs["123"] = _StreamBuf(text="hello ", message_id=7, last_edit=0.0) + + await channel.send_delta("123", "", {"_stream_end": True}) + + # edit_message_text should have been called twice: once for HTML, once for plain fallback + assert channel._app.bot.edit_message_text.call_count == 2 + # Second call should not use parse_mode="HTML" + second_call_kwargs = channel._app.bot.edit_message_text.call_args_list[1].kwargs + assert "parse_mode" not in second_call_kwargs or second_call_kwargs.get("parse_mode") is None + # Buffer should be cleaned up on success + assert "123" not in channel._stream_bufs + + @pytest.mark.asyncio async def test_send_delta_stream_end_splits_oversized_reply() -> None: """Final streamed reply exceeding Telegram limit is split into chunks.""" diff --git a/tests/channels/test_weixin_channel.py b/tests/channels/test_weixin_channel.py index 3a847411b..2b455fca6 100644 --- a/tests/channels/test_weixin_channel.py +++ b/tests/channels/test_weixin_channel.py @@ -1003,3 +1003,185 @@ async def test_download_media_item_non_image_requires_aes_key_even_with_full_url assert saved_path is None channel._client.get.assert_not_awaited() + + +# --------------------------------------------------------------------------- +# Tests for media-send error classification (network vs non-network errors) +# --------------------------------------------------------------------------- + + +def _make_outbound_msg(chat_id: str = "wx-user", content: str = "", media: list | None = None): + """Build a minimal OutboundMessage-like object for send() tests.""" + from nanobot.bus.events import OutboundMessage + + return OutboundMessage( + channel="weixin", + chat_id=chat_id, + content=content, + media=media or [], + metadata={}, + ) + + +@pytest.mark.asyncio +async def test_send_media_timeout_error_propagates_without_text_fallback() -> None: + """httpx.TimeoutException during media send must re-raise immediately, + NOT fall back to _send_text (which would also fail during network issues).""" + channel, _bus = _make_channel() + channel._client = object() + channel._token = "token" + channel._context_tokens["wx-user"] = "ctx-1" + channel._send_media_file = AsyncMock(side_effect=httpx.TimeoutException("timed out")) + channel._send_text = AsyncMock() + + msg = _make_outbound_msg(chat_id="wx-user", media=["/tmp/photo.jpg"]) + + with pytest.raises(httpx.TimeoutException, match="timed out"): + await channel.send(msg) + + # _send_text must NOT have been called as a fallback + channel._send_text.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_send_media_transport_error_propagates_without_text_fallback() -> None: + """httpx.TransportError during media send must re-raise immediately.""" + channel, _bus = _make_channel() + channel._client = object() + channel._token = "token" + channel._context_tokens["wx-user"] = "ctx-1" + channel._send_media_file = AsyncMock( + side_effect=httpx.TransportError("connection reset") + ) + channel._send_text = AsyncMock() + + msg = _make_outbound_msg(chat_id="wx-user", media=["/tmp/photo.jpg"]) + + with pytest.raises(httpx.TransportError, match="connection reset"): + await channel.send(msg) + + channel._send_text.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_send_media_5xx_http_status_error_propagates_without_text_fallback() -> None: + """httpx.HTTPStatusError with a 5xx status must re-raise immediately.""" + channel, _bus = _make_channel() + channel._client = object() + channel._token = "token" + channel._context_tokens["wx-user"] = "ctx-1" + + fake_response = httpx.Response( + status_code=503, + request=httpx.Request("POST", "https://example.test/upload"), + ) + channel._send_media_file = AsyncMock( + side_effect=httpx.HTTPStatusError( + "Service Unavailable", request=fake_response.request, response=fake_response + ) + ) + channel._send_text = AsyncMock() + + msg = _make_outbound_msg(chat_id="wx-user", media=["/tmp/photo.jpg"]) + + with pytest.raises(httpx.HTTPStatusError, match="Service Unavailable"): + await channel.send(msg) + + channel._send_text.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_send_media_4xx_http_status_error_falls_back_to_text() -> None: + """httpx.HTTPStatusError with a 4xx status should fall back to text, not re-raise.""" + channel, _bus = _make_channel() + channel._client = object() + channel._token = "token" + channel._context_tokens["wx-user"] = "ctx-1" + + fake_response = httpx.Response( + status_code=400, + request=httpx.Request("POST", "https://example.test/upload"), + ) + channel._send_media_file = AsyncMock( + side_effect=httpx.HTTPStatusError( + "Bad Request", request=fake_response.request, response=fake_response + ) + ) + channel._send_text = AsyncMock() + + msg = _make_outbound_msg(chat_id="wx-user", media=["/tmp/photo.jpg"]) + + # Should NOT raise — 4xx is a client error, non-retryable + await channel.send(msg) + + # _send_text should have been called with the fallback message + channel._send_text.assert_awaited_once_with( + "wx-user", "[Failed to send: photo.jpg]", "ctx-1" + ) + + +@pytest.mark.asyncio +async def test_send_media_file_not_found_falls_back_to_text() -> None: + """FileNotFoundError (a non-network error) should fall back to text.""" + channel, _bus = _make_channel() + channel._client = object() + channel._token = "token" + channel._context_tokens["wx-user"] = "ctx-1" + channel._send_media_file = AsyncMock( + side_effect=FileNotFoundError("Media file not found: /tmp/missing.jpg") + ) + channel._send_text = AsyncMock() + + msg = _make_outbound_msg(chat_id="wx-user", media=["/tmp/missing.jpg"]) + + # Should NOT raise + await channel.send(msg) + + channel._send_text.assert_awaited_once_with( + "wx-user", "[Failed to send: missing.jpg]", "ctx-1" + ) + + +@pytest.mark.asyncio +async def test_send_media_value_error_falls_back_to_text() -> None: + """ValueError (e.g. unsupported format) should fall back to text.""" + channel, _bus = _make_channel() + channel._client = object() + channel._token = "token" + channel._context_tokens["wx-user"] = "ctx-1" + channel._send_media_file = AsyncMock( + side_effect=ValueError("Unsupported media format") + ) + channel._send_text = AsyncMock() + + msg = _make_outbound_msg(chat_id="wx-user", media=["/tmp/file.xyz"]) + + # Should NOT raise + await channel.send(msg) + + channel._send_text.assert_awaited_once_with( + "wx-user", "[Failed to send: file.xyz]", "ctx-1" + ) + + +@pytest.mark.asyncio +async def test_send_media_network_error_does_not_double_api_calls() -> None: + """During network issues, media send should make exactly 1 API call attempt, + not 2 (media + text fallback). Verify total call count.""" + channel, _bus = _make_channel() + channel._client = object() + channel._token = "token" + channel._context_tokens["wx-user"] = "ctx-1" + channel._send_media_file = AsyncMock( + side_effect=httpx.ConnectError("connection refused") + ) + channel._send_text = AsyncMock() + + msg = _make_outbound_msg(chat_id="wx-user", content="hello", media=["/tmp/img.png"]) + + with pytest.raises(httpx.ConnectError): + await channel.send(msg) + + # _send_media_file called once, _send_text never called + channel._send_media_file.assert_awaited_once() + channel._send_text.assert_not_awaited()