From 3de3fd51660ae297bc134d9e2d8bd11ddb0fb7a6 Mon Sep 17 00:00:00 2001 From: hataa <79907651+hata33@users.noreply.github.com> Date: Tue, 22 Sep 2026 21:59:53 +0800 Subject: [PATCH] fix(channels): log DingTalk inbound-file skips instead of vanishing silently (#5683) _receive_single_file dropped an attachment with zero log lines whenever _download_by_code returned None (or empty bytes) - each None reason already has its accurate line inside the download function (non-200 exchange, missing downloadUrl, oversize abort, transport failure), but nothing tied the skip to the file being received, and the empty-bytes case landed in the same silent branch. The caller now logs a neutral guard line naming the file - the same shape the WeChat channel's callers and the manager reader use since #5225. Both the None and empty-bytes paths are pinned by caplog tests; reverting to the silent branch turns them red. Full DingTalk suite 135 passed / 1 skipped. --- backend/app/channels/dingtalk.py | 8 +++++++ backend/tests/test_dingtalk_channel.py | 30 ++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/backend/app/channels/dingtalk.py b/backend/app/channels/dingtalk.py index 1c8ad48b1..ea19d857d 100644 --- a/backend/app/channels/dingtalk.py +++ b/backend/app/channels/dingtalk.py @@ -629,6 +629,14 @@ class DingTalkChannel(Channel): """ content = await self._download_by_code(download_code) if not content: + # Neutral on purpose: None covers several reasons (non-200 + # download exchange, missing downloadUrl, oversize abort, + # transport failure), each already logged with its accurate + # reason inside _download_by_code — the empty-bytes case lands + # here too. Logging here keeps the skip observable at the + # receive level instead of vanishing silently (the wechat + # channel's callers and the manager reader use the same shape). + logger.warning("[DingTalk] inbound file download returned no content, skipping: file=%s", filename or "(unnamed)") return "" paths = get_paths() diff --git a/backend/tests/test_dingtalk_channel.py b/backend/tests/test_dingtalk_channel.py index e6d55c3e8..6f4120879 100644 --- a/backend/tests/test_dingtalk_channel.py +++ b/backend/tests/test_dingtalk_channel.py @@ -2608,3 +2608,33 @@ class TestHandlerStashesRawData: assert DingTalkChannel._extract_files(msg) == [{"type": "file", "download_code": "dc_doc", "filename": "a.xlsx"}] _run(go()) + + +class TestDingTalkDownloadGuardLogging: + """A None (or empty) download result used to drop the attachment with + zero log lines at the receive level — the accurate reason lines inside + _download_by_code fired, but nothing tied them to the file being + received. The caller now logs a neutral guard line, mirroring the + wechat channel's callers and the manager reader.""" + + def test_none_download_logs_neutral_guard_line(self, caplog): + async def go(): + channel = DingTalkChannel(MessageBus(), config={}) + channel._download_by_code = AsyncMock(return_value=None) + with caplog.at_level(logging.WARNING, logger="app.channels.dingtalk"): + result = await channel._receive_single_file("dc1", "file", "report.pdf", "t1", user_id="default") + assert result == "" + + _run(go()) + assert any("inbound file download returned no content" in r.message and "report.pdf" in r.message for r in caplog.records) + + def test_empty_download_logs_neutral_guard_line(self, caplog): + async def go(): + channel = DingTalkChannel(MessageBus(), config={}) + channel._download_by_code = AsyncMock(return_value=b"") + with caplog.at_level(logging.WARNING, logger="app.channels.dingtalk"): + result = await channel._receive_single_file("dc2", "image", "photo.png", "t1", user_id="default") + assert result == "" + + _run(go()) + assert any("inbound file download returned no content" in r.message and "photo.png" in r.message for r in caplog.records)