fix(voyage): close response body stream when batch output JSONL parsing throws#98840
Conversation
…ng throws The batch output file download path creates a readline interface over a Readable.fromWeb() response body stream. If JSON.parse throws on a malformed JSONL line, the for-await loop exits via exception but the readline interface and underlying Readable stream were never explicitly closed or destroyed, leaving the HTTP response body stream dangling. Extract the stream reading into , a testable helper that wraps the iteration in a try-finally so both reader.close() and inputStream.destroy() are always called, matching the pattern established in openclaw#98493 for the same class of leak.
|
Codex review: needs maintainer review before merge. Reviewed July 1, 2026, 10:02 PM ET / 02:02 UTC. Summary PR surface: Source +24, Tests +93. Total +117 across 2 files. Reproducibility: yes. at source/proof level: current main and Review metrics: none identified. Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Next step before merge
Security Review detailsBest possible solution: Land the focused Voyage plugin cleanup once exact-head required checks complete, keeping broader output-reader convergence as a separate follow-up if maintainers want it. Do we have a high-confidence way to reproduce the issue? Yes at source/proof level: current main and Is this the best way to solve the issue? Yes. This is the narrow plugin-owned fix for the leak path; a shared parser refactor or OpenAI-style output-budget change would be broader than needed for this cleanup bug. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 313560d5b98e. Label changesLabel justifications:
Evidence reviewedPR surface: Source +24, Tests +93. Total +117 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
proof: sufficient
1ec42c9
into
openclaw:main
* origin/main: (1287 commits) fix(android): block loopback canvas navigation (openclaw#99874) fix(hooks): suppress unhandled stdout/stderr stream errors in gmail watcher (openclaw#100519) fix(memory-core): guard qmd mcporter JSON.parse against non-JSON stdout (openclaw#98381) fix(build): fall back to tsx for build TypeScript scripts (openclaw#91262) feat(skills): suggest saving detected reusable workflows by default (openclaw#95477) (openclaw#100692) docs(changelog): remove generated release-note entries feat(telegram): offer BotFather web app flow in setup help and docs (openclaw#100540) fix(ios): unify Talk and Settings row typography on one branded detail row (openclaw#100515) feat(gateway): add persisted crash-loop breaker and fatal-config exit contract refactor(macos): lock and unify PortGuardian tunnel record persistence so concurrent app instances cannot lose orphan records (openclaw#100601) fix: stop reconnecting on protocol mismatch (openclaw#98414) fix(maint): reuse recent hosted gates after rebase (openclaw#100663) fix(ui): reopen web terminals without stale content (openclaw#100665) fix(browser): diagnose empty WSL2 Chrome replies (openclaw#100590) fix(ios): chat snaps back to bottom when scrolling to top via status-bar tap (openclaw#100502) Treat already-compacted CLI compaction as no-op (openclaw#99136) docs(changelog): remove direct main fix entry fix(feishu): strip internal tool-trace banners from outbound text (openclaw#98705) fix(message): thread --limit through to CLI formatter and surface provider pagination hints (openclaw#99089) fix(voyage): close response body stream when batch output JSONL parsing throws (openclaw#98840) ... # Conflicts: # extensions/memory-wiki/package.json
…ng throws (openclaw#98840) The batch output file download path creates a readline interface over a Readable.fromWeb() response body stream. If JSON.parse throws on a malformed JSONL line, the for-await loop exits via exception but the readline interface and underlying Readable stream were never explicitly closed or destroyed, leaving the HTTP response body stream dangling. Extract the stream reading into , a testable helper that wraps the iteration in a try-finally so both reader.close() and inputStream.destroy() are always called, matching the pattern established in openclaw#98493 for the same class of leak.
…ng throws (openclaw#98840) The batch output file download path creates a readline interface over a Readable.fromWeb() response body stream. If JSON.parse throws on a malformed JSONL line, the for-await loop exits via exception but the readline interface and underlying Readable stream were never explicitly closed or destroyed, leaving the HTTP response body stream dangling. Extract the stream reading into , a testable helper that wraps the iteration in a try-finally so both reader.close() and inputStream.destroy() are always called, matching the pattern established in openclaw#98493 for the same class of leak.
1 participant
What Problem This Solves
The Voyage batch output file download path (
extensions/voyage/embedding-batch.ts) reads the HTTP response body viareadline.createInterface()overReadable.fromWeb(). IfJSON.parsethrows on a malformed JSONL line from the Voyage API, thefor awaitloop exits via exception — but the readline interface and underlyingReadablestream were never explicitly closed or destroyed, leaving the HTTP response body stream dangling.Why This Change Was Made
reader.close()alone does not destroy the underlying input stream (Node.jsInterface.close()only sets a flag and removes listeners; theReadableis not cleaned up). The fix wraps the iteration intry-finallyso bothreader.close()andinputStream.destroy()are always called.The stream reading is extracted into a
readBatchOutputContenthelper function, exported via the test-onlytestingobject, so the cleanup path has direct test coverage.User Impact
Low probability (malformed JSONL from Voyage is rare), but a genuine resource leak on every occurrence. Users running large-scale Voyage embedding batches are most likely to encounter this.
Evidence
Real behavior proof — a standalone Node.js script that reproduces the exact leak pattern and confirms the fix:
The old code path (no try-finally) leaves the underlying
ReadableStreamdangling afterJSON.parsethrows. The new code path (reader.close() + inputStream.destroy()in afinallyblock) properly cancels the stream.pnpm test extensions/voyage/embedding-batch.test.ts— 11 passedreadBatchOutputContentwith an HTTP Response containing malformed JSONL bodySide Effects
None. The refactored code paths are functionally identical — only the error-exit cleanup is added.
🤖 Generated with Claude Code