[WRONG BRANCH] fix(server): bound responses body-read timeout - #184
Draft
luvs01 wants to merge 1 commit into
Draft
[WRONG BRANCH] fix(server): bound responses body-read timeout#184luvs01 wants to merge 1 commit into
luvs01 wants to merge 1 commit into
Conversation
|
This pull request currently targets @luvs01 Please retarget this PR to Its title has been prefixed with This pull request is being kept as a draft automatically. Once every issue above is resolved, it will be marked ready for review again. |
github-actions
Bot
changed the title
📝 WalkthroughWalkthroughThe Responses handler now disables its request timeout after complete request-body parsing. A new callback connects this point to route timeout handling. Tests verify that incomplete bodies keep the timeout active and malformed JSON returns a client error. ChangesResponses timeout handling
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/server/responses/core.ts`:
- Line 1371: Update the combo request flow around handleComboResponses so
options.onRequestBodyRead?.() is invoked exactly once after validating the
original request body, before returning or dispatching child requests. Remove or
guard onRequestBodyRead when forwarding options to parsed child requests,
preserving the callback behavior for non-combo requests.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d6b48181-c92f-4bc4-9cde-5f8e927edf94
📒 Files selected for processing (3)
src/server/index.tssrc/server/responses/core.tstests/server-auth.test.ts
| } | ||
| return formatErrorResponse(400, "invalid_request_error", err instanceof Error ? err.message : String(err)); | ||
| } | ||
| options.onRequestBodyRead?.(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 20 \
'handleComboResponses|comboIdFromRawBody|comboAttempt|onRequestBodyRead' \
src/server/responses/core.ts
rg -n -C 10 \
'onRequestBodyRead|handleResponses\(' \
src/server/index.ts tests/server-auth.test.tsRepository: luvs01/opencodex
Length of output: 30248
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- handleComboResponses implementation ---'
sed -n '974,1235p' src/server/responses/core.ts
printf '%s\n' '--- callback gate definition and usages ---'
rg -n -C 20 'createChildPassthroughCallbackGate|onRequestBodyRead' src/server/responses/core.ts
printf '%s\n' '--- combo-related tests and callback assertions ---'
rg -n -C 12 'combo|onRequestBodyRead|timeout' tests src/server | head -n 500Repository: luvs01/opencodex
Length of output: 50372
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- handleResponses entry and callback propagation ---'
rg -n -C 18 '^export async function handleResponses|onRequestBodyRead' src/server/responses/core.ts
printf '%s\n' '--- focused combo test files ---'
git ls-files 'tests/*' | while IFS= read -r file; do
if rg -q 'handleComboResponses|comboAttempt|comboIdFromRawBody|combos:' "$file"; then
printf '%s\n' "$file"
fi
done
printf '%s\n' '--- callback-related test locations ---'
rg -n -C 8 'onRequestBodyRead' tests src/server/index.tsRepository: luvs01/opencodex
Length of output: 8382
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- timeout callback implementation ---'
rg -n -C 12 'function disableResponsesRequestTimeout|disableResponsesRequestTimeout' src/server
printf '%s\n' '--- combo failover test structure ---'
rg -n -C 8 'describe|test\(|failover|retry|targets' tests/server-combo-failover-e2e.test.ts | head -n 240
printf '%s\n' '--- read-only source verifier ---'
python3 - <<'PY'
from pathlib import Path
source = Path("src/server/responses/core.ts").read_text()
combo_branch = "if (comboId && Object.hasOwn(config.combos ?? {}, comboId)) {\n return handleComboResponses(req, body, comboId, config, logCtx, options);"
callback_call = "options.onRequestBodyRead?.();"
child_forward = "...options,"
child_guard = "comboAttempt: true,"
assert combo_branch in source
assert callback_call in source
assert source.index(combo_branch) < source.index(callback_call)
combo_start = source.index("export async function handleComboResponses(")
combo_end = source.index("\n\n\nfunction finalizeOwnedTranslatorBudget", combo_start)
combo = source[combo_start:combo_end]
assert child_forward in combo and child_guard in combo
assert callback_call not in combo
print("combo branch returns before the outer callback")
print("combo child forwards the parent options and invokes the shared callback in handleResponsesInner")
print("no combo-specific one-shot guard exists")
PYRepository: luvs01/opencodex
Length of output: 26032
🏁 Script executed:
#!/usr/bin/env bash
set -u
python3 - <<'PY'
from pathlib import Path
source = Path("src/server/responses/core.ts").read_text()
combo_branch = """if (comboId && Object.hasOwn(config.combos ?? {}, comboId)) {
return handleComboResponses(req, body, comboId, config, logCtx, options);"""
callback_call = "options.onRequestBodyRead?.();"
combo_start = source.index("export async function handleComboResponses(")
combo_end = source.index("\n\n\nfunction finalizeOwnedTranslatorBudget", combo_start)
combo = source[combo_start:combo_end]
checks = {
"combo branch returns before outer callback": source.index(combo_branch) < source.index(callback_call),
"combo forwards parent options to child": "...options," in combo,
"child marks recursive call as combo attempt": "comboAttempt: true," in combo,
"combo handler has no direct callback call": callback_call not in combo,
}
for name, value in checks.items():
print(f"{name}: {value}")
assert all(checks.values())
print("Conclusion: each successfully parsed child reaches the shared callback, so a two-target failover can invoke it twice.")
PYRepository: luvs01/opencodex
Length of output: 464
Invoke onRequestBodyRead once for the original combo request. In src/server/responses/core.ts:1320-1323, the combo path returns before line 1371. handleComboResponses forwards ...options to each child at lines 1097-1114, so each parsed child invokes the callback. A two-target failover can invoke it twice. Call it once after validating the original body, then omit it from child options or guard it as one-shot.
🤖 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/server/responses/core.ts` at line 1371, Update the combo request flow
around handleComboResponses so options.onRequestBodyRead?.() is invoked exactly
once after validating the original request body, before returning or dispatching
child requests. Remove or guard onRequestBodyRead when forwarding options to
parsed child requests, preserving the callback behavior for non-combo requests.
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.
Motivation
Description
onRequestBodyRead?: () => voidhook toHandleResponsesOptionsand invoke it only after the inbound JSON body is fully read and validated inhandleResponsesInner(src/server/responses/core.ts).disableResponsesRequestTimeout(req, requestServer)immediately on route match and instead passonRequestBodyRead: () => disableResponsesRequestTimeout(req, requestServer)from the/v1/responsesroute insrc/server/index.tsso the timeout is disabled only after the body parse succeeds.tests/server-auth.test.ts).fix(server): bound responses body read timeout(three files changed:src/server/index.ts,src/server/responses/core.ts,tests/server-auth.test.ts).Testing
bun run test -- tests/server-auth.test.ts -t 'responses handler keeps|responses timeout helper'and the targeted tests passed.bun run typecheckand it succeeded.bun run privacy:scanand it succeeded.bun run testwas attempted earlier in the environment but unrelated permission/race tests and environment timeouts were observed; those failures are not caused by this change and were not required to validate the focused regression.Codex Task
Summary by CodeRabbit