fix(installer): pause response stream in all paths to prevent data loss#1860
fix(installer): pause response stream in all paths to prevent data loss#1860jeoor wants to merge 3 commits into
Conversation
Fixes Hmbown#1858 The downloadText function was losing early data chunks due to a race condition with httpRequest's stall timer listener. Root cause: - httpRequest attaches a data listener for stall detection, which puts the stream in flowing mode immediately - By the time downloadText attaches its own data listener, some data events have already fired and been lost - This causes partial checksum manifest lines to be parsed, resulting in 'Invalid checksum manifest line' errors Fix: - Pause the response stream immediately in httpRequest before the stall timer listener can put it in flowing mode - Resume the stream in downloadText after attaching the data listener The stall timer still works correctly because: - armStallTimer() is called when response is received (initial arm) - The stall timer's data listener still fires after resume, re-arming the timer on each chunk
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a race condition in the httpRequest function where early data chunks were being lost because the stall timer's listener triggered flowing mode before the caller's data listener was attached. The fix involves pausing the response stream immediately and resuming it once the caller is ready. Feedback indicates that this fix should also be applied to the proxy request paths within the same function to ensure comprehensive coverage for users behind proxies.
| // Pause the stream immediately to prevent data events from firing | ||
| // before the caller attaches their data listener. Without this, | ||
| // the stall timer's data listener puts the stream in flowing mode | ||
| // and early chunks are lost, causing checksum manifest parsing to fail | ||
| // with "Invalid checksum manifest line" errors. | ||
| response.pause(); |
There was a problem hiding this comment.
The fix for the race condition (pausing the response stream) is currently only applied to the direct connection path. It should also be applied to the two proxy request paths in this function (see lines 658 and 721 in the full file) to ensure the fix is comprehensive for all users, including those behind a proxy. Without this, the stall timer's data listener will still put the stream into flowing mode prematurely in those cases, leading to the same potential data loss for proxy users.
…loss Apply the same race condition fix from the direct connection path to both proxy request paths (HTTP proxy and HTTPS CONNECT tunnel). Without this, the stall timer's data listener puts the stream into flowing mode before the caller attaches their data listener, causing early chunks to be lost and checksum manifest parsing to fail with 'Invalid checksum manifest line'.
jeoor
changed the title
…c, receipt truncation fix - SWE-bench: codewhale swebench run/export writes prediction JSONL from working-tree diff, with untracked-file inclusion via git add -N - CLI: --workspace / -C global flag forwards to TUI for file ops - CLI: codewhale exec --auto semantics clarified in help text - Markdown: table pipes inside inline code no longer create phantom columns (split_table_cells with backtick-awareness) - Receipt: floor_char_boundary prevents multibyte UTF-8 slice panic - Contributors: Ling (LING71671 Hmbown#1839 Hmbown#1911), Ben Younes (ousamabenyounes Hmbown#1938), jeoor npm fix (Hmbown#1860) credited across all 3 READMEs - ja-JP README: 19 contributors synced to parity with EN/zh-CN (80 each) - Docs: SWEBENCH.md, RECURSIVE_SELF_IMPROVEMENT.md, MODES.md exec clarification - Sub-agent footer: Alt+V hint now says 'details' not 'raw'
|
谢谢 @jeoor，这个修复已经通过 #1988 合入 main，对应提交是 b94ccd3。已发布的实现会在所有下载响应路径里尽早 pause stream，并在 downloadText 挂好读取逻辑后再 resume，避免 checksum manifest 早期数据块丢失。\n\n现在代码已经在 main 上，所以我关闭这个 PR。v0.8.43/v0.8.44 的 release notes 和 README Thanks 都已经补上对你的署名。之前 harvest commit 没有写精确的 'Harvested from PR #1860 by @jeoor' trailer，导致自动关闭流程没有在合入时处理，这是我们的流程问题，抱歉。 |
2 participants
问题
npm 全局安装在 Windows（及其他平台）上失败，报错：
每次运行错误信息不同（如
k-macos-arm64、-macos-arm64、seek-macos-arm64），显示校验和清单的部分文件名。根本原因
httpRequest和downloadText之间的竞态条件：httpRequest附加 stall timer 的datalistener，立即将流切换到 flowing 模式downloadText获取响应并附加自己的datalistener 时，部分 data 事件已触发修复
在所有响应路径中立即暂停流：
这样无论用户是直连还是通过代理，都能防止数据丢失。
stall timer 仍然正常工作，因为
armStallTimer()在收到响应时调用（初始计时），data listener 在 resume 后重新计时。Fixes #1858