fix(responses): keep caller cancellations out of upstream failure logs (carry of #3515) - #3541
Conversation
Co-authored-by: VXNCXNX <93332837+VXNCXNX@users.noreply.github.com>
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughNative passthrough now detects request cancellation earlier, preserves upstream terminal outcomes during bounded draining, and distinguishes client cancellation from upstream failure. Tests and reference documentation cover the resulting status, logging, usage, and account-pool behavior. ChangesNative passthrough outcome handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Native passthrough requests now record caller cancellations as 499 without penalizing provider accounts, while genuine upstream resets remain 502 failures. The paired regression coverage supports the intended distinction, with no remaining merge-readiness risk identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
리뷰 · 우선순위 74 / 80이 PR은 호출자가 native Responses 스트리밍 요청을 중간에 끊었을 때, 그 끊김이 업스트림 실패(502)처럼 기록되고 계정 풀 페널티까지 먹는 버그를 고치는 유지자 캐리입니다. 원본은 #3515이고, 그 exact head 왜 중요한지 한 줄로 말하면, 클라이언트가 탭을 닫거나 AbortController로 요청을 취소하는 일은 정상 동작인데, 그걸 업스트림 리셋과 같은 실패로 세면 풀 헬스( 원본 PR CI는 exact head에서 풀 매트릭스가 초록이었고, 이 캐리 쪽은 작성 시점 기준 hygiene·react-doctor·storage policy·일부 npm-global/테스트 샤드가 이미 통과 중이며 나머지 매트릭스는 아직 pending입니다. 라인 - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
Ingwannu
left a comment
There was a problem hiding this comment.
Revalidated the carry at exact head 696847cd4f11d2657e4aceda4c30708fe1ab53b0. It preserves the already-reviewed cancellation boundary and paired regressions at the current test path: caller abort remains 499 client_cancel with no pool penalty, while a genuine upstream reset remains a 502 failure.
All jobs in Cross-platform CI run 33926437244 are green, and the exact-head CodeRabbit review has no actionable finding. The remaining pending target-check duplicate has an already-successful exact-head counterpart and the PR targets dev. Approved.
Summary
When the caller aborts a native Responses request, the relay logged it as an upstream 502 and penalized the provider pool. This carry classifies caller aborts as 499 in
src/server/relay.ts/src/server/responses/core.tswhile a genuine upstream reset still logs 502. The original PR carries an APPROVED review from @Ingwannu on its exact head4f09faf5d.Supersedes #3515 (maintainer carry: GitHub reports the original CONFLICTING after the
tests/<domain>/migration, whilegit merge-treeis clean; the carry is the PR head merged withorigin/dev, no source edits).Stack (wp1 merge train, independent layers — each targets
dev):Unit:
devlog/_plan/260905_open_work_closeout/(010, 011).Verification
bun run typecheck— exit 0 on the carry head.bun test tests/server/server-auth.test.ts— tests/server/server-auth.test.ts — 105 pass / 0 fail (paired 499/502 regressions).3515's exact head was full-matrix green; exact-head CI on this branch is the merge gate.Checklist
devCo-authored-bytrailerCo-authored-by: VXNCXNX 93332837+VXNCXNX@users.noreply.github.com
Summary by CodeRabbit
client_cancelreason.