fix(dingtalk): preserve richText formatting and set HTTP client timeout
richText messages only kept rich_text_list items whose type was "text", so bold/italic/inlineCode/pre segments were silently dropped; a message made entirely of formatted segments produced empty content and fell through to the "unsupported message type: richText" warning. Now any item carrying text is kept (matching the SDK's own get_text_list, which keys off the "text" field) and its type is mapped to Markdown so the formatting survives into the agent's context. Text and downloadCode within a rich-text item are handled independently (the SDK treats them as separate via get_text_list / get_image_list), so an item carrying both a caption and an attachment no longer drops the file. The shared httpx.AsyncClient was created without a timeout, so all requests (including large file/image downloads) used httpx's 5s default and hit ConnectTimeout/ReadTimeout on uploads. Set an explicit httpx.Timeout (connect=10s, read/write=30s). Adds regression tests for formatted-segment preservation, the all-formatted no-drop case, the text+downloadCode item, and the client timeout configuration. Closes #4497
This commit is contained in:
Max Hsu
Xubin Ren
@@ -94,11 +94,23 @@ class NanobotDingTalkHandler(CallbackHandler):
|
||||
for item in rich_list:
|
||||
if not isinstance(item, dict):
|
||||
continue
|
||||
if item.get("type") == "text":
|
||||
t = item.get("text", "").strip()
|
||||
if t:
|
||||
content = (content + " " + t).strip() if content else t
|
||||
elif item.get("downloadCode"):
|
||||
# A rich-text item may carry text and/or a downloadCode; the
|
||||
# DingTalk SDK treats them independently, so handle both.
|
||||
t = item.get("text", "").strip()
|
||||
if t:
|
||||
fmt = item.get("type", "")
|
||||
if fmt == "bold":
|
||||
formatted = f"**{t}**"
|
||||
elif fmt == "italic":
|
||||
formatted = f"*{t}*"
|
||||
elif fmt == "inlineCode":
|
||||
formatted = f"`{t}`"
|
||||
elif fmt == "pre":
|
||||
formatted = f"```\n{t}\n```"
|
||||
else:
|
||||
formatted = t
|
||||
content = (content + " " + formatted).strip() if content else formatted
|
||||
if item.get("downloadCode"):
|
||||
dc = item["downloadCode"]
|
||||
fname = item.get("fileName") or "file"
|
||||
sender_uid = chatbot_msg.sender_staff_id or chatbot_msg.sender_id or "unknown"
|
||||
@@ -214,7 +226,9 @@ class DingTalkChannel(BaseChannel):
|
||||
return
|
||||
|
||||
self._running = True
|
||||
self._http = httpx.AsyncClient()
|
||||
self._http = httpx.AsyncClient(
|
||||
timeout=httpx.Timeout(10.0, connect=10.0, read=30.0, write=30.0, pool=10.0)
|
||||
)
|
||||
|
||||
self.logger.info(
|
||||
"Initializing Stream Client with Client ID: {}...",
|
||||
|
||||
@@ -259,6 +259,150 @@ async def test_handler_processes_file_message(monkeypatch) -> None:
|
||||
assert "/tmp/nanobot_dingtalk/user1/report.xlsx" in msg.content
|
||||
|
||||
|
||||
def _rich_text_message(rich_text_list):
|
||||
class _FakeRichTextChatbotMessage:
|
||||
text = None
|
||||
extensions = {}
|
||||
image_content = None
|
||||
rich_text_content = SimpleNamespace(rich_text_list=rich_text_list)
|
||||
sender_staff_id = "user1"
|
||||
sender_id = "fallback-user"
|
||||
sender_nick = "Alice"
|
||||
message_type = "richText"
|
||||
|
||||
@staticmethod
|
||||
def from_dict(_data):
|
||||
return _FakeRichTextChatbotMessage()
|
||||
|
||||
return _FakeRichTextChatbotMessage
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handler_richtext_keeps_formatted_segments(monkeypatch) -> None:
|
||||
"""richText segments with non-'text' types (bold/italic/code/pre) must be kept
|
||||
and mapped to Markdown, not dropped (issue #4497)."""
|
||||
bus = MessageBus()
|
||||
channel = DingTalkChannel(
|
||||
DingTalkConfig(client_id="app", client_secret="secret", allow_from=["user1"]),
|
||||
bus,
|
||||
)
|
||||
handler = NanobotDingTalkHandler(channel)
|
||||
|
||||
fake_msg = _rich_text_message([
|
||||
{"type": "bold", "text": "Title"},
|
||||
{"type": "text", "text": "plain"},
|
||||
{"type": "italic", "text": "em"},
|
||||
{"type": "inlineCode", "text": "x = 1"},
|
||||
{"type": "pre", "text": "block"},
|
||||
])
|
||||
monkeypatch.setattr(dingtalk_module, "ChatbotMessage", fake_msg)
|
||||
monkeypatch.setattr(dingtalk_module, "AckMessage", SimpleNamespace(STATUS_OK="OK"))
|
||||
|
||||
status, body = await handler.process(
|
||||
SimpleNamespace(data={"conversationType": "1", "text": {"content": ""}})
|
||||
)
|
||||
msg = await asyncio.wait_for(bus.consume_inbound(), timeout=2.0)
|
||||
|
||||
assert (status, body) == ("OK", "OK")
|
||||
assert msg.content == "**Title** plain *em* `x = 1` ```\nblock\n```"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handler_richtext_all_formatted_not_dropped(monkeypatch) -> None:
|
||||
"""A richText message made only of formatted segments must not end up with empty
|
||||
content and fall through to the 'unsupported message type' path (issue #4497)."""
|
||||
bus = MessageBus()
|
||||
channel = DingTalkChannel(
|
||||
DingTalkConfig(client_id="app", client_secret="secret", allow_from=["user1"]),
|
||||
bus,
|
||||
)
|
||||
handler = NanobotDingTalkHandler(channel)
|
||||
|
||||
fake_msg = _rich_text_message([{"type": "bold", "text": "Important"}])
|
||||
monkeypatch.setattr(dingtalk_module, "ChatbotMessage", fake_msg)
|
||||
monkeypatch.setattr(dingtalk_module, "AckMessage", SimpleNamespace(STATUS_OK="OK"))
|
||||
|
||||
status, body = await handler.process(
|
||||
SimpleNamespace(data={"conversationType": "1", "text": {"content": ""}})
|
||||
)
|
||||
# Before the fix this message produced empty content and never reached the bus,
|
||||
# so consume_inbound would block here.
|
||||
msg = await asyncio.wait_for(bus.consume_inbound(), timeout=2.0)
|
||||
|
||||
assert (status, body) == ("OK", "OK")
|
||||
assert msg.content == "**Important**"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handler_richtext_item_with_text_and_download(monkeypatch) -> None:
|
||||
"""A rich-text item carrying both text and a downloadCode must yield both the
|
||||
text and the downloaded file, not drop the attachment (issue #4497)."""
|
||||
bus = MessageBus()
|
||||
channel = DingTalkChannel(
|
||||
DingTalkConfig(client_id="app", client_secret="secret", allow_from=["user1"]),
|
||||
bus,
|
||||
)
|
||||
handler = NanobotDingTalkHandler(channel)
|
||||
|
||||
fake_msg = _rich_text_message([
|
||||
{"text": "see attached", "downloadCode": "abc123", "fileName": "report.xlsx"},
|
||||
])
|
||||
|
||||
async def fake_download(download_code, filename, sender_id):
|
||||
return f"/tmp/nanobot_dingtalk/{sender_id}/{filename}"
|
||||
|
||||
monkeypatch.setattr(dingtalk_module, "ChatbotMessage", fake_msg)
|
||||
monkeypatch.setattr(dingtalk_module, "AckMessage", SimpleNamespace(STATUS_OK="OK"))
|
||||
monkeypatch.setattr(channel, "_download_dingtalk_file", fake_download)
|
||||
|
||||
status, body = await handler.process(
|
||||
SimpleNamespace(data={"conversationType": "1", "text": {"content": ""}})
|
||||
)
|
||||
await asyncio.gather(*list(channel._background_tasks))
|
||||
msg = await asyncio.wait_for(bus.consume_inbound(), timeout=2.0)
|
||||
|
||||
assert (status, body) == ("OK", "OK")
|
||||
assert "see attached" in msg.content
|
||||
assert "/tmp/nanobot_dingtalk/user1/report.xlsx" in msg.content
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_start_configures_http_timeout(monkeypatch) -> None:
|
||||
"""The shared httpx client must be created with an explicit timeout so file/image
|
||||
downloads don't hit httpx's 5s default and ConnectTimeout (issue #4497)."""
|
||||
channel = DingTalkChannel(
|
||||
DingTalkConfig(client_id="app", client_secret="secret", allow_from=["*"]),
|
||||
MessageBus(),
|
||||
)
|
||||
|
||||
class _FakeStreamClient:
|
||||
def __init__(self, _credential):
|
||||
pass
|
||||
|
||||
def register_callback_handler(self, _topic, _handler):
|
||||
pass
|
||||
|
||||
async def start(self):
|
||||
# Exit the reconnect loop after one iteration.
|
||||
channel._running = False
|
||||
|
||||
monkeypatch.setattr(dingtalk_module, "DINGTALK_AVAILABLE", True)
|
||||
monkeypatch.setattr(dingtalk_module, "Credential", lambda *a, **k: object())
|
||||
monkeypatch.setattr(dingtalk_module, "DingTalkStreamClient", _FakeStreamClient)
|
||||
monkeypatch.setattr(dingtalk_module, "ChatbotMessage", SimpleNamespace(TOPIC="topic"))
|
||||
|
||||
await channel.start()
|
||||
|
||||
assert channel._http is not None
|
||||
timeout = channel._http.timeout
|
||||
assert timeout.connect == 10.0
|
||||
assert timeout.read == 30.0
|
||||
assert timeout.write == 30.0
|
||||
assert timeout.pool == 10.0
|
||||
|
||||
await channel.stop()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_download_dingtalk_file(tmp_path, monkeypatch) -> None:
|
||||
"""Test the two-step file download flow (get URL then download content)."""
|
||||
|
||||
Reference in New Issue
Block a user