fix(feishu): check response.success() on send_file's reply/create calls (#4335)

willem-bd's review on PR #4234 (Feishu card response.success() checks)
flagged send_file (around lines 313-318) as having the same unchecked-
response pattern: after _upload_image/_upload_file (which already raise on
a business-level failure), the resulting file/image message.reply or
message.create call's response was never checked, so a failed send logged
"file sent" and returned True exactly like a real success.

Adds the same response.success() check used by _reply_card/_create_card/
_update_card, raising (caught by the existing try/except, which already
logs and returns False) so the caller can no longer mistake a silent
business-level failure for a delivered file/image.
This commit is contained in:
Daoyuan Li 2026-07-21 03:37:10 -07:00 committed by GitHub
parent e283341c55
commit 2bb22643ad
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 148 additions and 2 deletions

View File

@ -312,10 +312,12 @@ class FeishuChannel(Channel):
if msg.thread_ts:
request = self._ReplyMessageRequest.builder().message_id(msg.thread_ts).request_body(self._ReplyMessageRequestBody.builder().msg_type(msg_type).content(content).build()).build()
await asyncio.to_thread(self._api_client.im.v1.message.reply, request)
response = await asyncio.to_thread(self._api_client.im.v1.message.reply, request)
else:
request = self._CreateMessageRequest.builder().receive_id_type("chat_id").request_body(self._CreateMessageRequestBody.builder().receive_id(msg.chat_id).msg_type(msg_type).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 file send failed: code={response.code}, msg={response.msg}, log_id={response.get_log_id()}")
logger.info("[Feishu] file sent: %s (type=%s)", attachment.filename, msg_type)
return True

View File

@ -5311,6 +5311,150 @@ class TestFeishuChannel:
_run(go())
class TestFeishuSendFileSuccessChecks:
"""``send_file`` uploads via ``_upload_image``/``_upload_file`` (which already
raise on a ``response.success() is False`` business failure), then sends the
resulting file/image message with a raw ``message.reply``/``message.create``
call whose response was never checked. lark-oapi signals that same kind of
business-level failure (invalid receiver, permission error, etc.) by
returning ``success()=False`` without raising, so a failed file/image send
logged "file sent" and returned ``True`` exactly like a real success.
"""
def test_send_file_returns_false_on_reply_business_failure(self, tmp_path):
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
channel._upload_image = AsyncMock(return_value="img-key-1")
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-send-file-1"
channel._api_client.im.v1.message.reply = MagicMock(return_value=failure_response)
path = tmp_path / "image.png"
path.write_bytes(b"png")
attachment = ResolvedAttachment(
virtual_path="/mnt/user-data/outputs/image.png",
actual_path=path,
filename="image.png",
mime_type="image/png",
size=path.stat().st_size,
is_image=True,
)
msg = OutboundMessage(
channel_name="feishu",
chat_id="chat-1",
thread_id="thread-1",
text="",
is_final=True,
thread_ts="om-source-msg",
)
result = await channel.send_file(msg, attachment)
assert result is False
_run(go())
def test_send_file_returns_false_on_create_business_failure(self, tmp_path):
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
channel._upload_file = AsyncMock(return_value="file-key-1")
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-send-file-2"
channel._api_client.im.v1.message.create = MagicMock(return_value=failure_response)
path = tmp_path / "report.pdf"
path.write_bytes(b"pdf")
attachment = ResolvedAttachment(
virtual_path="/mnt/user-data/outputs/report.pdf",
actual_path=path,
filename="report.pdf",
mime_type="application/pdf",
size=path.stat().st_size,
is_image=False,
)
msg = OutboundMessage(
channel_name="feishu",
chat_id="chat-1",
thread_id="thread-1",
text="",
is_final=True,
thread_ts=None,
)
result = await channel.send_file(msg, attachment)
assert result is False
_run(go())
def test_send_file_returns_true_on_reply_business_success(self, tmp_path):
"""Control case: a genuinely successful response still returns True."""
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
channel._upload_image = AsyncMock(return_value="img-key-1")
success_response = MagicMock()
success_response.success.return_value = True
channel._api_client.im.v1.message.reply = MagicMock(return_value=success_response)
path = tmp_path / "image.png"
path.write_bytes(b"png")
attachment = ResolvedAttachment(
virtual_path="/mnt/user-data/outputs/image.png",
actual_path=path,
filename="image.png",
mime_type="image/png",
size=path.stat().st_size,
is_image=True,
)
msg = OutboundMessage(
channel_name="feishu",
chat_id="chat-1",
thread_id="thread-1",
text="",
is_final=True,
thread_ts="om-source-msg",
)
result = await channel.send_file(msg, attachment)
assert result is True
_run(go())
class TestWeComChannel:
def test_publish_ws_inbound_starts_stream_and_publishes_message(self, monkeypatch):
from app.channels.wecom import WeComChannel