fix(providers): bound transit response buffering - #224
Closed
hyrious wants to merge 1 commit into
Closed
Conversation
Summary by CodeRabbit
WalkthroughProvider file and media download paths now use 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
Comment |
Collaborator
Author
|
按维护者安排，改为本地合并整理，关闭此独立加固 PR。 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/providers/elevenlabs/executors.ts (1)
544-552: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse an
interfacefor this object contract.
ElevenlabsRequestInputis object-shaped rather than a union or mapped composition.Proposed fix
-type ElevenlabsRequestInput = { +interface ElevenlabsRequestInput { method?: "DELETE" | "GET" | "POST"; baseUrl?: string; path: string; query?: Record<string, string | string[] | undefined>; body?: Record<string, unknown>; mode?: "validate" | "execute"; maxResponseBytes?: number; -}; +}As per coding guidelines, “Prefer
interfacefor object-shaped contracts; usetypefor unions and mapped or utility compositions.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/elevenlabs/executors.ts` around lines 544 - 552, Change ElevenlabsRequestInput from a type alias to an interface while preserving all existing optional properties and their types.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/providers/agenty/runtime.ts`:
- Around line 585-590: Update the oversized-response handling in
src/core/request.ts around lines 83-91 to cancel response.body before throwing
the early Content-Length 413 error, and add a regression test covering this
path. The affected response consumers at src/providers/agenty/runtime.ts lines
585-590, src/providers/feishu_app_bot/executors.ts lines 1145-1151,
src/providers/fuxin/executors.ts lines 357-369, src/providers/gemini/runtime.ts
lines 674-678, src/providers/gladia/runtime.ts lines 180-185,
src/providers/googledrive/executors.ts lines 511-516, and
src/providers/klangio/runtime.ts lines 222-235 require no direct changes; they
are corrected by the shared request-layer fix.
In `@src/providers/elevenlabs/executors.ts`:
- Around line 345-347: Move the context.transitFiles precondition checks before
the corresponding context.fetcher calls in the text_to_speech,
create_sound_effect, and history-audio execution paths. Update all three sites
in src/providers/elevenlabs/executors.ts:345-347, :433-435, and :466-468,
placing each check before the TTS POST, sound-effect POST, and history-audio GET
respectively while preserving the existing ProviderRequestError behavior.
---
Nitpick comments:
In `@src/providers/elevenlabs/executors.ts`:
- Around line 544-552: Change ElevenlabsRequestInput from a type alias to an
interface while preserving all existing optional properties and their types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2fd662a3-4e9c-477b-b23c-4de3c5106aed
📒 Files selected for processing (16)
src/providers/agenty/runtime.tssrc/providers/docsend_2_pdf/executors.tssrc/providers/elevenlabs/executors.tssrc/providers/feishu_app_bot/executors.tssrc/providers/fuxin/executors.tssrc/providers/gemini/runtime.tssrc/providers/gladia/runtime.tssrc/providers/googledrive/executors.tssrc/providers/klangio/runtime.tssrc/providers/kraken_io/executors.tssrc/providers/remove_bg/executors.tssrc/providers/screenshotone/runtime.tssrc/providers/stabilityai/executors.tssrc/providers/tinypng/runtime.tssrc/providers/vimeo/runtime.tssrc/providers/youtube/runtime.ts
Comment on lines
+585
to
+590
| const bytes = await readBoundedResponseBytes(response, { | ||
| maxBytes: context.transitFiles.maxBytes, | ||
| fieldName: "Agenty file output", | ||
| createError: (message) => new ProviderRequestError(413, message), | ||
| }); | ||
| const upload = await context.transitFiles.create(new File([Uint8Array.from(bytes)], name, { type: mimeType })); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== locate helper and call sites ==\n'
rg -n "readBoundedResponseBytes|assertMaxBytes|response.body\\?\\.cancel|Content-Length|maxBytes" src/providers src -g'*.ts'
printf '\n== helper definition context ==\n'
helper_file=$(rg -l "function readBoundedResponseBytes|const readBoundedResponseBytes|async function readBoundedResponseBytes" src -g'*.ts' | head -n 1)
echo "helper_file=$helper_file"
if [ -n "${helper_file:-}" ]; then
nl -ba "$helper_file" | sed -n '1,260p'
fi
printf '\n== tests referencing bounded bytes helper ==\n'
rg -n "readBoundedResponseBytes|Content-Length|413|cancel\\(" src test tests -g'*.ts' -g'*.tsx'Repository: oomol-lab/open-connector
Length of output: 49634
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== direct helper file search ==\n'
fd -a "readBoundedResponseBytes" src -t fRepository: oomol-lab/open-connector
Length of output: 195
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path("src/core/request.ts")
lines = path.read_text().splitlines()
for start, end in [(60, 130), (470, 520)]:
print(f"\n== {path} lines {start}-{end} ==")
for i in range(start, min(end, len(lines)) + 1):
print(f"{i:4d}: {lines[i-1]}")
PY
printf '\n== tests mentioning readBoundedResponseBytes or Content-Length ==\n'
python3 - <<'PY'
from pathlib import Path
import re
for path in Path("src").rglob("*.test.ts"):
text = path.read_text(errors="ignore")
if "readBoundedResponseBytes" in text or "Content-Length" in text or "content-length" in text:
print(path)
PYRepository: oomol-lab/open-connector
Length of output: 4119
Cancel the body before the early Content-Length 413
src/core/request.ts#L83-L91 rejects oversized responses before touching response.body, so the underlying request can stay open. Cancel the body before throwing and add a regression test for this path.
📍 Affects 7 files
src/providers/agenty/runtime.ts#L585-L590(this comment)src/providers/feishu_app_bot/executors.ts#L1145-L1151src/providers/fuxin/executors.ts#L357-L369src/providers/gemini/runtime.ts#L674-L678src/providers/gladia/runtime.ts#L180-L185src/providers/googledrive/executors.ts#L511-L516src/providers/klangio/runtime.ts#L222-L235
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/providers/agenty/runtime.ts` around lines 585 - 590, Update the
oversized-response handling in src/core/request.ts around lines 83-91 to cancel
response.body before throwing the early Content-Length 413 error, and add a
regression test covering this path. The affected response consumers at
src/providers/agenty/runtime.ts lines 585-590,
src/providers/feishu_app_bot/executors.ts lines 1145-1151,
src/providers/fuxin/executors.ts lines 357-369, src/providers/gemini/runtime.ts
lines 674-678, src/providers/gladia/runtime.ts lines 180-185,
src/providers/googledrive/executors.ts lines 511-516, and
src/providers/klangio/runtime.ts lines 222-235 require no direct changes; they
are corrected by the shared request-layer fix.
Comment on lines
+345
to
+347
| if (!context.transitFiles) { | ||
| throw new ProviderRequestError(500, "text_to_speech requires transit file storage"); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate transit storage before making the upstream request.
The check is too late: text_to_speech and create_sound_effect can consume ElevenLabs credits before inevitably failing with a local 500. Move each precondition ahead of its context.fetcher call; this also avoids an unnecessary history-audio request.
src/providers/elevenlabs/executors.ts#L345-L347: move the check before the TTS POST at Line 328.src/providers/elevenlabs/executors.ts#L433-L435: move the check before the sound-effect POST at Line 417.src/providers/elevenlabs/executors.ts#L466-L468: move the check before the history-audio GET at Line 456.
📍 Affects 1 file
src/providers/elevenlabs/executors.ts#L345-L347(this comment)src/providers/elevenlabs/executors.ts#L433-L435src/providers/elevenlabs/executors.ts#L466-L468
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/providers/elevenlabs/executors.ts` around lines 345 - 347, Move the
context.transitFiles precondition checks before the corresponding
context.fetcher calls in the text_to_speech, create_sound_effect, and
history-audio execution paths. Update all three sites in
src/providers/elevenlabs/executors.ts:345-347, :433-435, and :466-468, placing
each check before the TTS POST, sound-effect POST, and history-audio GET
respectively while preserving the existing ProviderRequestError behavior.
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
1 participant
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.
概要
批次 provider 自审发现，多个文件输出 action 会先无界读取完整 provider 响应，再由
transitFiles.create检查大小。超大图片、音频、视频或导出文件因此可能在限制生效前放大进程内存。本 PR 将 16 个 provider 的 response → transit 路径改为复用现有
readBoundedResponseBytes，以transitFiles.maxBytes在读取阶段早停并返回 413：remove.bg 与 ElevenLabs 的 JSON/base64 文件分支按 base64 膨胀量限制 JSON 响应，并在解码后再次检查实际文件大小。仅返回 inline base64、读取本地 transit file 或已有独立 bounded reader 的路径保持不变。
边界
src/core/request.ts已有 bounded reader验证
npm run fix-checknpm run buildnpx vitest run src/core/request.test.ts src/providers/provider-runtime.test.ts（2 files / 36 tests）git diff --check origin/main..HEAD未运行全量测试；本 PR 未改共享 helper，涉及 provider 没有对应的 OSS provider-local tests。