fix(channel/qq): deliver HITL approval prompts reliably (#166)
* fix(channel/qq): deliver HITL approval prompts reliably
QQ approval prompts were silently dropped when the markdown send hit
a QQ server-side error (e.g. template not configured, content audit)
because the fallback path only matched TypeError / specific string
patterns, and the plain-text retry reused the already-consumed
msg_seq which QQ then rejects as duplicate.
- Consume a fresh msg_seq for the plain-text fallback send
- Recognize QQ server error codes (304014/304023/304003/40034059)
and CN fragments ("模版"/"审核") as markdown-fallback triggers
- Promote send failure logs from debug to warning/error with
chat_id/msg_id/seq so real-world errors can be diagnosed
- Extend test_qq_channel with a server-error-code fallback case
* style(channel/qq): apply ruff formatter to approval-delivery fix
* Fix
This commit is contained in:
@@ -200,12 +200,56 @@ class QQChannel(Channel):
|
||||
return
|
||||
except Exception as exc:
|
||||
if not self._should_fallback_to_plain_text(exc):
|
||||
logger.error(
|
||||
"QQ markdown send failed with non-fallbackable error "
|
||||
"(chat_id=%s, msg_id=%s, seq=%s): %r",
|
||||
chat_id,
|
||||
msg_id,
|
||||
seq,
|
||||
exc,
|
||||
)
|
||||
raise
|
||||
self._record_markdown_fallback(chat_id, raw_text, exc)
|
||||
logger.debug("QQ markdown send failed, falling back to plain text: %s", exc)
|
||||
logger.warning(
|
||||
"QQ markdown send failed, falling back to plain text "
|
||||
"(chat_id=%s, msg_id=%s, seq=%s): %r",
|
||||
chat_id,
|
||||
msg_id,
|
||||
seq,
|
||||
exc,
|
||||
)
|
||||
|
||||
# QQ may have already consumed `seq` server-side even on failure.
|
||||
# Reusing it for the plain retry triggers "duplicate msg_seq", so
|
||||
# always advance to a fresh seq before the fallback send.
|
||||
fallback_seq = self._next_msg_seq(msg_id)
|
||||
plain_text = self._plain_formatter.format(raw_text)
|
||||
await self._post_plain_message(chat_id, plain_text, msg_type, msg_id, seq)
|
||||
try:
|
||||
await self._post_plain_message(
|
||||
chat_id, plain_text, msg_type, msg_id, fallback_seq
|
||||
)
|
||||
except Exception as plain_exc:
|
||||
logger.error(
|
||||
"QQ plain fallback also failed (chat_id=%s, msg_id=%s, seq=%s): %r",
|
||||
chat_id,
|
||||
msg_id,
|
||||
fallback_seq,
|
||||
plain_exc,
|
||||
)
|
||||
raise
|
||||
|
||||
# QQ server-side error codes / fragments that indicate the markdown
|
||||
# request itself is invalid (template not configured, format rejected,
|
||||
# content audit, etc.). Seeing any of these means we should retry with
|
||||
# plain text rather than re-raise.
|
||||
_QQ_MARKDOWN_ERROR_MARKERS: ClassVar[tuple[str, ...]] = (
|
||||
"304014", # markdown template not configured
|
||||
"304003", # invalid markdown params
|
||||
"40034059", # generic send message failed (often markdown-related)
|
||||
"模板", # CN: template (standard form)
|
||||
"模版", # CN: template (variant form)
|
||||
"审核", # CN: audit
|
||||
)
|
||||
|
||||
def _should_fallback_to_plain_text(self, exc: Exception) -> bool:
|
||||
"""Return True only for markdown compatibility/validation failures."""
|
||||
@@ -214,17 +258,20 @@ class QQChannel(Channel):
|
||||
|
||||
msg = str(exc).lower()
|
||||
compatibility_tokens = ("unsupported", "unexpected", "unknown", "invalid")
|
||||
return (
|
||||
"unexpected keyword argument" in msg
|
||||
or (
|
||||
"markdown" in msg
|
||||
and any(token in msg for token in compatibility_tokens)
|
||||
)
|
||||
or (
|
||||
"msg_type" in msg
|
||||
and any(token in msg for token in compatibility_tokens)
|
||||
)
|
||||
)
|
||||
|
||||
if "unexpected keyword argument" in msg:
|
||||
return True
|
||||
if "markdown" in msg and any(token in msg for token in compatibility_tokens):
|
||||
return True
|
||||
if "msg_type" in msg and any(token in msg for token in compatibility_tokens):
|
||||
return True
|
||||
|
||||
# QQ-specific server error codes returned by qq-botpy as strings.
|
||||
raw = str(exc)
|
||||
for marker in self._QQ_MARKDOWN_ERROR_MARKERS:
|
||||
if marker in raw or marker.lower() in msg:
|
||||
return True
|
||||
return False
|
||||
|
||||
def _record_markdown_fallback(
|
||||
self,
|
||||
|
||||
@@ -71,7 +71,10 @@ class TestQQChannelSend:
|
||||
assert second["msg_type"] == 0
|
||||
assert second["content"] == "Title\n\n• item"
|
||||
assert second["msg_id"] == "evt_2"
|
||||
assert second["msg_seq"] == 1
|
||||
# Plain fallback consumes a fresh msg_seq — QQ may have already
|
||||
# counted the failed markdown attempt, so re-using seq would
|
||||
# trigger "duplicate msg_seq".
|
||||
assert second["msg_seq"] == 2
|
||||
|
||||
def test_send_does_not_fallback_on_transport_error(self):
|
||||
channel = self._make_ready_channel()
|
||||
@@ -99,3 +102,67 @@ class TestQQChannelSend:
|
||||
sent = channel._client.api.post_c2c_message.await_args.kwargs
|
||||
assert sent["msg_type"] == 2
|
||||
assert "content" not in sent
|
||||
|
||||
def test_send_does_not_fallback_when_transport_error_mentions_markdown(self):
|
||||
"""A transport-layer error whose message incidentally contains the word
|
||||
"markdown" must NOT be reclassified as a markdown compatibility failure,
|
||||
otherwise genuine send failures get silently swallowed as plain-text."""
|
||||
channel = self._make_ready_channel()
|
||||
|
||||
async def _send_once(coro_factory, max_retries=3):
|
||||
return await coro_factory()
|
||||
|
||||
channel._send_with_retry = _send_once
|
||||
channel._client.api.post_c2c_message = AsyncMock(
|
||||
side_effect=ConnectionError(
|
||||
"failed to post markdown message: ConnectionReset"
|
||||
)
|
||||
)
|
||||
msg = OutboundMessage(
|
||||
channel="qq",
|
||||
chat_id="openid",
|
||||
content="## Title",
|
||||
metadata={
|
||||
"chat_id": "openid",
|
||||
"event_id": "evt_transport",
|
||||
"msg_type": "c2c",
|
||||
},
|
||||
)
|
||||
|
||||
assert _run(channel.send(msg)) is False
|
||||
channel._client.api.post_c2c_message.assert_awaited_once()
|
||||
|
||||
def test_send_falls_back_on_qq_server_error_code(self):
|
||||
"""QQ server-side markdown errors (e.g. 304014 template not configured)
|
||||
should trigger plain-text fallback with a fresh msg_seq."""
|
||||
channel = self._make_ready_channel()
|
||||
channel._client.api.post_c2c_message = AsyncMock(
|
||||
side_effect=[
|
||||
RuntimeError(
|
||||
'{"code": 304014, "message": "markdown template not configured"}'
|
||||
),
|
||||
None,
|
||||
]
|
||||
)
|
||||
msg = OutboundMessage(
|
||||
channel="qq",
|
||||
chat_id="openid",
|
||||
content="## Title",
|
||||
metadata={
|
||||
"chat_id": "openid",
|
||||
"event_id": "evt_4",
|
||||
"msg_type": "c2c",
|
||||
},
|
||||
)
|
||||
|
||||
assert _run(channel.send(msg)) is True
|
||||
|
||||
assert channel._client.api.post_c2c_message.await_count == 2
|
||||
first = channel._client.api.post_c2c_message.await_args_list[0].kwargs
|
||||
second = channel._client.api.post_c2c_message.await_args_list[1].kwargs
|
||||
|
||||
assert first["msg_type"] == 2
|
||||
assert first["msg_seq"] == 1
|
||||
assert second["msg_type"] == 0
|
||||
# Fresh seq on fallback — avoids QQ "duplicate msg_seq" rejection.
|
||||
assert second["msg_seq"] == 2
|
||||
|
||||
Reference in New Issue
Block a user