Skip to content

fix: preserve edge streaming responses on upstream failure - #7

Merged
sg-shag merged 2 commits into
mainfrom
fix/edge-streaming
Oct 19, 2026
Merged

sg-shag merged 2 commits into
mainfrom
fix/edge-streaming

Conversation

@gnezim

@gnezim gnezim commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Summary

An upstream read failure after TF sends HTTP headers currently appends a second HTTP response into the stream. The handler also retains the entire response with inspection disabled, and its 60-second timeout can expire before Olla's generated 120-second backend allowance.

upstream failure before response headers -> JSON 502
upstream failure after response headers  -> close incomplete stream, log failure
inspector disabled                      -> no response capture
inspector enabled                       -> capture at most 1 MiB, mark truncation
edge I/O timeout                        -> 180 seconds

Keep the existing handler and HTTPX client. Preserve the selected endpoint in failure accounting and clear active-request tracking in finally. Client output is never truncated by the inspector limit. No routing, retry or scheduler changes. Independent of #5 and the memory-guard PR.

Evidence

  • Before: regression tests exercising the actual HTTP handler over a socket pair observed two HTTP/1.0 status lines after read failures, and 1,310,880 captured bytes with inspection disabled or enabled.
    After: one HTTP response, incomplete streams close without a second response or fabricated completion, upstream streams close, failures retain endpoint accounting, and captures are absent or bounded to 1 MiB.
  • A real loopback HTTP server reproduced the timeout mismatch with scaled timings: 90 seconds of header latency represented by 450 ms failed under the old edge default and succeeds under the new default. It does not perform a 90-second cluster request.
  • 64 focused edge/CLI tests pass. make dev-lint and git diff --check pass.
  • make dev-check: 351 passed, 1 failed using an ignored copy of tfconfig.example.yaml. The existing test_cluster_status_reports_inference_health expects 13G, while the example config supplies 11G. The same failure was reproduced from unchanged main at b458a4e.

Merge Danger

Door: two-way

Revert to restore the former timeout and diagnostic-capture behavior.

Blast Radius: edge

The 180-second HTTPX timeout also applies to buffered inference requests and can hold stalled requests longer; it is an I/O timeout, not a total generation deadline. Streaming inspector entries may contain only a prefix and mark it with body_truncated: true. A stream already started can retain HTTP 200 on the wire while access accounting records the upstream failure as 502. No live gateway deployment or inference load test was performed.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ce57dd75-95c5-44bd-b7ea-e54177ff1530
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sg-shag
sg-shag merged commit 4676fcb into main Oct 19, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants