Repository navigation
Fix silent outbound send failures (timeouts and gateway rejections reported as delivered) - #80
Open
LiuZiqi-dual wants to merge 1 commit into
Open
LiuZiqi-dual wants to merge 1 commit into
LiuZiqi-dual wants to merge 1 commit into
Conversation
sendMessage had two paths that reported success while the message was
never delivered, defeating deliverReply's retry loop:
1. apiPost swallowed AbortError and returned {ret: 0} as a fake success.
That behavior is correct for the getupdates long-poll (timeout = no
new updates) but wrong for every other endpoint: a sendmessage POST
that timed out was treated as delivered. The empty-success fallback
is now opt-in via {timeoutAsEmpty: true}, used only by getUpdates;
all other endpoints throw on timeout so retries kick in.
2. Only the HTTP status was checked. A gateway rejection carried in an
HTTP-200 body (non-zero ret/errcode: expired context_token, rate
limit, unreachable user) was also treated as delivered. sendMessage
now validates the response body and throws on a business error.
Additionally, send failures were only reported through trackException,
which is a no-op when telemetry is disabled — so a dropped reply left
no trace in the logs. Text-segment, image-send, and final-drop failures
are now also logged via this.log so operators can see them on stdout.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
We hit a production incident where the bridge logged
Agent done, reply N charsfor every turn while the WeChat user received nothing — with zero errors anywhere in the logs. The incident itself turned out to be undiagnosable after the fact, precisely because the outbound send path can neither detect nor report most delivery-failure modes. This PR fixes the two concrete gaps that makedeliverReply's retry loop unreachable for whole classes of failures, and makes every send failure visible on stdout:1. A send timeout is reported as success.
apiPostturnsAbortErrorintoreturn { ret: 0, msgs: [] }. That fallback is correct for thegetupdateslong-poll (timeout just means "no new updates"), but every other endpoint shares the same function — so asendmessagePOST that hangs until the 15s timeout is treated as delivered. The caller sees a clean resolve,sent = true, and the retry loop never fires.2. Business-level rejections are reported as success. Only
res.okis checked. When the gateway returns HTTP 200 with a non-zeroret/errcodein the body (expiredcontext_token, rate limit, unreachable user), the send is also treated as delivered. This is the send-side validation gap reported in #51 — the poll side (monitor.js) already checksret/errcode, the send side has no equivalent.Compounding both: send failures are only reported via
trackException, which is a no-op when telemetry is disabled (WECHAT_ACP_TELEMETRY=0) — so even a hard failure that does throw (HTTP 4xx/5xx, connection reset, retries exhausted) leaves no trace on stdout.Both bugs are reproducible against a mock gateway: with the current code,
sendMessageresolves as success both when the gateway hangs past the timeout and when it returnsHTTP 200 {"ret":-13}; with this PR it throws in both cases.To be explicit about scope: #51 also documents a deeper failure mode (HTTP 200 +
ret: 0accepted by the gateway but never routed to the user's phone) that no client-side check can catch. This PR does not claim to fix that; it makes every gateway-visible failure loud and retryable, which narrows any remaining silent loss down to that server-side case.Fix
apiPostgains an opt-in{ timeoutAsEmpty: true }flag; onlygetUpdatesuses it. All other endpoints now throwTimeout after Nms: POST <endpoint>on abort, so the existing retry-with-stable-client_idlogic indeliverReplyactually engages (the gateway de-duplicates byclient_id, so retries stay safe).sendMessagevalidates the response body via a newassertBusinessOkhelper and throws on non-zeroret/errcode.this.log(...)so they are visible on stdout even with telemetry off.Callers of the other affected endpoints already tolerate throws:
getTypingTicket/sendTypingIndicatorwrap in try/catch,cancelTypingIndicatorcall sites use.catch(() => {}), andgetUploadUrlfailures feed the existing image retry loop.Tests
Added
tests/api.test.tscovering: send timeout rejects, HTTP-200 + non-zeroretrejects, HTTP-200 + non-zeroerrcoderejects, clean success resolves, body without status fields resolves, andgetupdateslong-poll timeout still resolves to an empty batch.npm test: 224 tests, 223 pass, 1 skipped (pre-existing), 0 fail.中文说明
问题:我们在生产环境遇到一次事故:桥接日志每轮都记录
Agent done, reply N chars、全程零报错,但微信用户什么都没收到。事后复盘发现这次事故根本无法诊断——因为出站发送路径对大多数投递失败模式既检测不到、也不留痕。本 PR 修复了让deliverReply重试逻辑形同虚设的两个具体缺陷,并让所有发送失败在 stdout 可见:apiPost把AbortError吞成{ret: 0}返回。这对getupdates长轮询是对的(超时=没新消息),但所有接口共用这个函数——sendmessage挂起 15 秒超时后会被当成"已送达",上层sent = true,重试永远不触发。ret/errcode非零(context_token 失效、限流、用户不可达)时,同样被当成"已送达"。这正是 sendMessage returns HTTP 200 but message never reaches user (silent delivery failure) #51 报告的发送侧校验缺口——轮询侧monitor.js有ret/errcode校验,发送侧没有。trackException,遥测关闭(WECHAT_ACP_TELEMETRY=0)时是空操作——即使真的抛了错(HTTP 4xx/5xx、连接重置、重试耗尽),stdout 也一行不留。两个缺陷均可用 mock 网关复现:现有代码在"网关挂起超时"和"HTTP 200 +
{"ret":-13}"两种场景下sendMessage都正常返回;本 PR 后两种场景都会抛错并进入重试。范围说明:#51 还记录了一种更深的失败模式(HTTP 200 +
ret: 0,网关受理但始终不投递到用户手机),这类服务端静默丢弃是客户端校验无法捕获的,本 PR 不声称能修复它;本 PR 的作用是让所有网关可见的失败都变得响亮且可重试,从而把剩余的静默丢失收窄到那种纯服务端场景。修复:
apiPost新增{ timeoutAsEmpty: true }选项、仅getUpdates使用,其余接口超时抛错以触发既有的稳定client_id重试(网关按client_id去重,重试安全);sendMessage新增assertBusinessOk校验返回体;文本分段、图片发送、最终丢弃三处失败均通过this.log(...)落 stdout。其余受影响接口的调用方均已具备 try/catch 或.catch()兜底。测试:新增
tests/api.test.ts共 6 例(超时抛错、ret非零抛错、errcode非零抛错、正常成功、无状态字段成功、长轮询超时语义不变)。npm test:224 例,223 通过,1 例既有跳过,0 失败。🤖 Generated with Claude Code