Skip to content

fix(chat): don't penalize accounts or fail over on client disconnect (stream replay / re-login / non-stream)#225

Merged
dwgx merged 1 commit into
dwgx:masterfrom
warelik:fix/abort-hygiene
Jul 25, 2026
Merged

fix(chat): don't penalize accounts or fail over on client disconnect (stream replay / re-login / non-stream)#225
dwgx merged 1 commit into
dwgx:masterfrom
warelik:fix/abort-hygiene

Conversation

@warelik

@warelik warelik commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

中文 TL;DR

客户端中途断开时,原来可能给账号记错误、继续 failover(把配额打到已断的 socket)、
往关闭的流里写数据。本 PR 把已有的 abort 处理扩展到 same-token replay / re-login /
非流式路径:不惩罚账号、不 failover、不写死 socket,非流式返回 499。

Summary

Hardening. Extends the existing "client disconnect stops account-hopping" behavior
to the paths it didn't yet cover. Complements #221 (both reduce needless upstream
load on the DEVIN_CONNECT path).

Fix

  • isAbortError(err) (name / code / message) unifies abort detection.
  • Stream attempt + transient-replay + re-login-retry catches → { kind: 'abort' },
    finalize with an AbortError (no error-budget penalty), break the failover loop.
  • [DONE] guarded by !signal.aborted && !res.writableEnded (no write to a dead socket).
  • Non-stream: pre-attempt + abort result → 499 client_disconnect.

Test plan

  • test/devin-connect-stream-lifecycle.test.js +1: same-token replay + disconnect →
    replays once, no account hop, no [DONE], no error frame to the gone client
  • Full suite: npm test2762 pass / 0 fail

Branch is on current master (v3.5.0), applies cleanly.

…_CONNECT paths

A client disconnect mid-attempt could penalize the account, fail over to fresh
accounts (burning quota to a dead socket), and write to a closed stream. Extend
the existing abort handling to the same-token replay, re-login retry, and
non-stream paths.

- src/handlers/chat.js: add isAbortError(err) helper; return {kind:'abort'} (no
  penalty) from the stream attempt, transient-replay, and re-login-retry catches;
  break the failover loop on abort; guard [DONE] against a closed socket; 499
  Client Closed Request on the non-stream path
- test/devin-connect-stream-lifecycle.test.js: +1 — aborts cleanly during a
  same-token transient replay when the client disconnects
@dwgx
dwgx merged commit 1783545 into dwgx:master Jul 25, 2026
5 checks passed
dwgx added a commit that referenced this pull request Jul 25, 2026
Conflict resolution: #225 (abort branch) and #226 (session commit on ok) both extend the attemptStream result switch. Kept both — commit the pair only on kind:'ok'; a kind:'abort' turn never completed, so committing it would poison the pair chain with an unmatched half-turn.
dwgx added a commit that referenced this pull request Jul 25, 2026
- contributors.json: 新增 6 条(#224 S / #225 A+ / #226 S / #227 A / #228 B+ / #229 A)
- README 中英: warelik 段落补本轮六个 PR 的技术叙述
@dwgx

dwgx commented Jul 25, 2026

Copy link
Copy Markdown
Owner

已合并,谢谢 🙏 isAbortError 同时认 err.nameerr.code/aborted/i 这个细节是这个 PR 的关键 —— undici 和 Node 自己的 AbortController 在错误形态上确实不一致,只认一种就会漏掉一半场景,而漏掉的那一半正好是最常见的用户 Ctrl-C。

覆盖面做得很完整:流式的首次尝试 / transient replay / re-login retry 三条 catch,加非流式主路径与 re-login retry,一条不落。两个防御细节尤其到位 —— replay 启动前重查 aborted(不对死 socket 发起重放)、[DONE] 写入前检查(避免卡在已关连接),这两处不写的话,前面的判断会被后面的写操作绕过去。非流式返回 499 也比静默 200 诚实。

对多用户 / 网关前置的部署这是实打实的配额止血 —— 之前每个断连都在罚 error budget + 跨账号 failover,等于用户按一次 Ctrl-C 就烧掉好几个账号的健康度。

合并时与 #226attemptStream 的结果分支上有一处冲突(你两个 PR 都扩展了那个 switch),我保留了两边语义并做了一个判断:kind: 'abort' 分支提交 session pair —— 那一轮从未完成,提交会用一个永远匹配不上的半轮污染 pair chain。已写进 merge commit 说明,如果你觉得该走另一种处理欢迎提。

已加入致谢名单,评级 A+。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants