diff --git a/backend/app/channels/feishu.py b/backend/app/channels/feishu.py index 82c35c6b3..6feb4f729 100644 --- a/backend/app/channels/feishu.py +++ b/backend/app/channels/feishu.py @@ -493,7 +493,17 @@ class FeishuChannel(Channel): return try: request = self._CreateMessageReactionRequest.builder().message_id(message_id).request_body(self._CreateMessageReactionRequestBody.builder().reaction_type(self._Emoji.builder().emoji_type(emoji_type).build()).build()).build() - await asyncio.to_thread(self._api_client.im.v1.message_reaction.create, request) + response = await asyncio.to_thread(self._api_client.im.v1.message_reaction.create, request) + if not response.success(): + logger.warning( + "[Feishu] reaction '%s' add failed for message %s: code=%s, msg=%s, log_id=%s", + emoji_type, + message_id, + response.code, + response.msg, + response.get_log_id(), + ) + return logger.info("[Feishu] reaction '%s' added to message %s", emoji_type, message_id) except Exception: logger.exception("[Feishu] failed to add reaction '%s' to message %s", emoji_type, message_id) @@ -506,6 +516,8 @@ class FeishuChannel(Channel): content = self._build_card_content(text) request = self._ReplyMessageRequest.builder().message_id(message_id).request_body(self._ReplyMessageRequestBody.builder().msg_type("interactive").content(content).build()).build() response = await asyncio.to_thread(self._api_client.im.v1.message.reply, request) + if not response.success(): + raise RuntimeError(f"Feishu card reply failed: code={response.code}, msg={response.msg}, log_id={response.get_log_id()}") response_data = getattr(response, "data", None) return getattr(response_data, "message_id", None) @@ -516,7 +528,9 @@ class FeishuChannel(Channel): content = self._build_card_content(text) request = self._CreateMessageRequest.builder().receive_id_type("chat_id").request_body(self._CreateMessageRequestBody.builder().receive_id(chat_id).msg_type("interactive").content(content).build()).build() - await asyncio.to_thread(self._api_client.im.v1.message.create, request) + response = await asyncio.to_thread(self._api_client.im.v1.message.create, request) + if not response.success(): + raise RuntimeError(f"Feishu card creation failed: code={response.code}, msg={response.msg}, log_id={response.get_log_id()}") async def _update_card(self, message_id: str, text: str) -> None: """Patch an existing card message in place.""" @@ -525,7 +539,9 @@ class FeishuChannel(Channel): content = self._build_card_content(text) request = self._PatchMessageRequest.builder().message_id(message_id).request_body(self._PatchMessageRequestBody.builder().content(content).build()).build() - await asyncio.to_thread(self._api_client.im.v1.message.patch, request) + response = await asyncio.to_thread(self._api_client.im.v1.message.patch, request) + if not response.success(): + raise RuntimeError(f"Feishu card update failed: code={response.code}, msg={response.msg}, log_id={response.get_log_id()}") def _track_background_task(self, task: asyncio.Task, *, name: str, msg_id: str) -> None: """Keep a strong reference to fire-and-forget tasks and surface errors.""" diff --git a/backend/tests/test_channels.py b/backend/tests/test_channels.py index 11329733b..6c8f8ca54 100644 --- a/backend/tests/test_channels.py +++ b/backend/tests/test_channels.py @@ -5457,6 +5457,267 @@ class TestFeishuSendFileSuccessChecks: _run(go()) +class TestFeishuCardSuccessChecks: + """Regression coverage: ``lark-oapi`` signals a *business-level* failure + (expired/invalid card, permission error, etc.) by returning a response + whose ``response.success()`` is ``False`` -- the SDK call itself does not + raise. This file's own ``_upload_image``/``_upload_file``/ + ``_receive_single_file`` already guard against this by checking + ``response.success()``; ``_reply_card``/``_create_card``/``_update_card``/ + ``_add_reaction`` did not, so a failed card send/update looked identical + to a successful one to every caller. + """ + + def test_reply_card_raises_on_business_failure_response(self): + from lark_oapi.api.im.v1 import ReplyMessageRequest, ReplyMessageRequestBody + + from app.channels.feishu import FeishuChannel + + async def go(): + bus = MessageBus() + channel = FeishuChannel(bus, config={}) + channel._api_client = MagicMock() + channel._ReplyMessageRequest = ReplyMessageRequest + channel._ReplyMessageRequestBody = ReplyMessageRequestBody + + failure_response = MagicMock() + failure_response.success.return_value = False + failure_response.code = 99991400 + failure_response.msg = "param invalid" + failure_response.get_log_id.return_value = "log-reply-1" + channel._api_client.im.v1.message.reply = MagicMock(return_value=failure_response) + + with pytest.raises(RuntimeError, match="99991400") as exc_info: + await channel._reply_card("om-source-msg", "hello") + assert "log-reply-1" in str(exc_info.value) + + _run(go()) + + def test_create_card_raises_on_business_failure_response(self): + from lark_oapi.api.im.v1 import CreateMessageRequest, CreateMessageRequestBody + + from app.channels.feishu import FeishuChannel + + async def go(): + bus = MessageBus() + channel = FeishuChannel(bus, config={}) + channel._api_client = MagicMock() + channel._CreateMessageRequest = CreateMessageRequest + channel._CreateMessageRequestBody = CreateMessageRequestBody + + failure_response = MagicMock() + failure_response.success.return_value = False + failure_response.code = 99991400 + failure_response.msg = "param invalid" + failure_response.get_log_id.return_value = "log-create-1" + channel._api_client.im.v1.message.create = MagicMock(return_value=failure_response) + + with pytest.raises(RuntimeError, match="99991400") as exc_info: + await channel._create_card("chat-1", "hello") + assert "log-create-1" in str(exc_info.value) + + _run(go()) + + def test_update_card_raises_on_business_failure_response(self): + from lark_oapi.api.im.v1 import PatchMessageRequest, PatchMessageRequestBody + + from app.channels.feishu import FeishuChannel + + async def go(): + bus = MessageBus() + channel = FeishuChannel(bus, config={}) + channel._api_client = MagicMock() + channel._PatchMessageRequest = PatchMessageRequest + channel._PatchMessageRequestBody = PatchMessageRequestBody + + failure_response = MagicMock() + failure_response.success.return_value = False + failure_response.code = 99991400 + failure_response.msg = "card has expired" + failure_response.get_log_id.return_value = "log-update-1" + channel._api_client.im.v1.message.patch = MagicMock(return_value=failure_response) + + with pytest.raises(RuntimeError, match="99991400") as exc_info: + await channel._update_card("om-running-card", "hello") + assert "log-update-1" in str(exc_info.value) + + _run(go()) + + def test_add_reaction_logs_warning_on_business_failure_without_raising(self, caplog): + from lark_oapi.api.im.v1 import ( + CreateMessageReactionRequest, + CreateMessageReactionRequestBody, + Emoji, + ) + + from app.channels.feishu import FeishuChannel + + async def go(): + bus = MessageBus() + channel = FeishuChannel(bus, config={}) + channel._api_client = MagicMock() + channel._CreateMessageReactionRequest = CreateMessageReactionRequest + channel._CreateMessageReactionRequestBody = CreateMessageReactionRequestBody + channel._Emoji = Emoji + + failure_response = MagicMock() + failure_response.success.return_value = False + failure_response.code = 99991400 + failure_response.msg = "reaction not allowed" + failure_response.get_log_id.return_value = "log-1" + channel._api_client.im.v1.message_reaction.create = MagicMock(return_value=failure_response) + + with caplog.at_level(logging.WARNING): + await channel._add_reaction("om-source-msg", "OK") + + assert "99991400" in caplog.text + + _run(go()) + + def test_final_streaming_update_falls_back_to_new_card_when_update_card_fails(self): + """``_send_card_message``'s ``try/except`` around ``_update_card`` + already falls back to a brand-new card reply for a final message -- + but that fallback could never fire while ``_update_card`` swallowed + business failures silently. Now that ``_update_card`` raises, the + fallback is reachable.""" + from app.channels.feishu import FeishuChannel + + async def go(): + bus = MessageBus() + channel = FeishuChannel(bus, config={}) + channel._api_client = MagicMock() + + channel._running_card_ids["om-source-msg"] = "om-running-card" + channel._update_card = AsyncMock(side_effect=RuntimeError("Feishu card update failed: code=99991400, msg=card expired")) + channel._reply_card = AsyncMock(return_value="om-fallback-card") + channel._add_reaction = AsyncMock() + + msg = OutboundMessage( + channel_name="feishu", + chat_id="chat-1", + thread_id="thread-1", + text="final answer", + is_final=True, + thread_ts="om-source-msg", + ) + + await channel._send_card_message(msg) + + channel._update_card.assert_awaited_once_with("om-running-card", "final answer") + channel._reply_card.assert_awaited_once_with("om-source-msg", "final answer") + assert "om-source-msg" not in channel._running_card_ids + + _run(go()) + + def test_non_final_streaming_update_failure_propagates_instead_of_silently_succeeding(self): + """A non-final ``_update_card`` failure must propagate out of + ``_send_card_message`` so ``send()``'s ``_send_with_retry`` sees it -- + previously it never would, since ``_update_card`` had no way to raise + on a business-level failure.""" + from app.channels.feishu import FeishuChannel + + async def go(): + bus = MessageBus() + channel = FeishuChannel(bus, config={}) + channel._api_client = MagicMock() + + channel._running_card_ids["om-source-msg"] = "om-running-card" + channel._update_card = AsyncMock(side_effect=RuntimeError("Feishu card update failed: code=99991400, msg=card expired")) + channel._reply_card = AsyncMock() + + msg = OutboundMessage( + channel_name="feishu", + chat_id="chat-1", + thread_id="thread-1", + text="partial answer", + is_final=False, + thread_ts="om-source-msg", + ) + + with pytest.raises(RuntimeError, match="99991400"): + await channel._send_card_message(msg) + + channel._reply_card.assert_not_awaited() + assert channel._running_card_ids["om-source-msg"] == "om-running-card" + + _run(go()) + + def test_send_retries_after_update_card_business_failure_then_succeeds(self, monkeypatch): + """End-to-end through ``send()``: a non-final ``_update_card`` + business failure must now engage ``_send_with_retry`` instead of the + caller believing the streaming update was delivered.""" + from app.channels.feishu import FeishuChannel + + async def go(): + bus = MessageBus() + channel = FeishuChannel(bus, config={}) + channel._api_client = MagicMock() + sleep = AsyncMock() + monkeypatch.setattr("app.channels.base.asyncio.sleep", sleep) + + channel._running_card_ids["om-source-msg"] = "om-running-card" + channel._update_card = AsyncMock( + side_effect=[ + RuntimeError("Feishu card update failed: code=99991400, msg=card expired"), + None, + ] + ) + + msg = OutboundMessage( + channel_name="feishu", + chat_id="chat-1", + thread_id="thread-1", + text="partial answer", + is_final=False, + thread_ts="om-source-msg", + ) + + await channel.send(msg, _max_retries=2) + + assert channel._update_card.await_count == 2 + sleep.assert_awaited_once_with(1) + + _run(go()) + + def test_send_retries_after_create_card_business_failure_then_succeeds(self, monkeypatch): + """End-to-end through ``send()`` for the no-``thread_ts`` path: a + business failure from ``_create_card`` (unwrapped at the tail of + ``_send_card_message``) must also engage ``_send_with_retry``, + mirroring ``test_send_retries_after_update_card_business_failure_then_succeeds`` + for the ``_update_card`` path above.""" + from app.channels.feishu import FeishuChannel + + async def go(): + bus = MessageBus() + channel = FeishuChannel(bus, config={}) + channel._api_client = MagicMock() + sleep = AsyncMock() + monkeypatch.setattr("app.channels.base.asyncio.sleep", sleep) + + channel._create_card = AsyncMock( + side_effect=[ + RuntimeError("Feishu card creation failed: code=99991400, msg=param invalid, log_id=log-1"), + None, + ] + ) + + msg = OutboundMessage( + channel_name="feishu", + chat_id="chat-1", + thread_id="thread-1", + text="new card message", + is_final=True, + thread_ts=None, + ) + + await channel.send(msg, _max_retries=2) + + assert channel._create_card.await_count == 2 + sleep.assert_awaited_once_with(1) + + _run(go()) + + class TestWeComChannel: def test_publish_ws_inbound_starts_stream_and_publishes_message(self, monkeypatch): from app.channels.wecom import WeComChannel