fix(llm-api-gateway): stop 502s leaking router errors under load by along-2017 · Pull Request #2270 · NVIDIA/nvcf

along-2017 · 2026-10-05T06:06:54Z

TL;DR

Under load, gateway 502s exposed the router's in-cluster URL, pod IPs and Go transport errors. Upstream transport errors now return a fixed 502 message, and proxied bodies go to the router with a fixed length instead of chunked.

Additional Details (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)

The original error stays as the 502's internal cause, so the request log and trace still record it.
The transport errors come from dead pooled connections after early router responses. The fixed length does not prevent them; the router fix is stargate: router closes HTTP/1 connections after early responses without Connection: close #2290.
With a fixed length, the router can reject a body over its size limit before it reads the body.

For the Reviewer

provider/stargate.go takes the length from the Content-Length header. All current callers set it to the buffered body length.

For QA (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)

New tests: a transport error becomes a 502 with no URL, IP or transport text; chat, responses and embeddings requests reach the upstream with a matching Content-Length. Both fail on main.
Local load test (32 concurrent, 30 s, with early router rejections): 48 leaked 502s before, 0 after. With stargate: router closes HTTP/1 connections after early responses without Connection: close #2290 as well, no 502s.
QA: after this PR and stargate: router closes HTTP/1 connections after early responses without Connection: close #2290 are deployed, run 32+ concurrent requests and confirm no 502s.

Issues

Fixes #2269

Checklist

I am familiar with the Contributing Guidelines.
I have signed off my commits for Developer Certificate of Origin (DCO) compliance.
New or existing tests cover these changes.
The documentation is up to date with these changes.

Proxied request bodies were wrapped in io.NopCloser, so Go sent them chunked. When the router answers before reading a chunked body (for example its overload 503), it then closes the connection without Connection: close. The gateway reused the dead connection and the next POST failed with broken pipe or "server closed idle connection". - Set the outbound ContentLength from the Content-Length header the handlers already compute, so bodies go out with a fixed length. - Return a fixed 502 message for upstream transport errors and keep the original error as the internal error, which the request log records. Signed-off-by: along <along@nvidia.com>

coderabbitai · 2026-10-05T06:07:19Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: NVIDIA/nvcf/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e8a79d2b-d158-4224-bcc0-bf00cd6571ec

📥 Commits

Reviewing files that changed from the base of the PR and between ce45c79 and 184837a.

📒 Files selected for processing (2)

src/invocation-plane-services/llm-api-gateway/api/upstream_framing_test.go
src/invocation-plane-services/llm-api-gateway/provider/stargate_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

src/invocation-plane-services/llm-api-gateway/provider/stargate_test.go
src/invocation-plane-services/llm-api-gateway/api/upstream_framing_test.go

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

📝 Walkthrough

Walkthrough

The gateway now forwards buffered requests with a fixed content length. For non-Echo errors, it returns a generic message and attaches the original error internally. Tests cover request framing and error handling.

Changes

Gateway upstream handling

Layer / File(s)	Summary
Fixed-length request forwarding `src/invocation-plane-services/llm-api-gateway/provider/stargate.go`, `src/invocation-plane-services/llm-api-gateway/provider/stargate_test.go`, `src/invocation-plane-services/llm-api-gateway/api/upstream_framing_test.go`, `src/invocation-plane-services/llm-api-gateway/api/BUILD.bazel`	`StargateProvider.Proxy` sets `ContentLength` when the request header contains a positive integer. Proxy and gateway tests check that upstream requests contain the full body, have a matching content length, and use no transfer encoding. The Bazel target includes the gateway framing test.
Generic upstream error responses `src/invocation-plane-services/llm-api-gateway/api/provider_error.go`, `src/invocation-plane-services/llm-api-gateway/api/provider_error_test.go`, `src/invocation-plane-services/llm-api-gateway/api/BUILD.bazel`	For non-Echo errors, `providerHTTPError` returns a generic message and attaches the original error internally. Tests cover direct and wrapped transport errors and an Echo 429 error. The Bazel target includes the error test.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: famousdirector

Merge Risk: ⚪ Minimal · up to 18483

The test-comment updates do not change gateway behavior, and no actionable merge risk was identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 6 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	For `#2269`, `providerHTTPError` returns a fixed 502 message and retains the original error with `SetInternal`; the PR summary states that completed-request logging records the cause with the request ID…
Out of Scope Changes check	✅ Passed	The error handling, request framing, tests, and Bazel test-source updates all support `#2269`. No unrelated changes are present in the reviewed summary or diffs.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title follows Conventional Commits with the required `fix` type and `llm-api-gateway` scope. It accurately describes the primary change: preventing internal router errors from appearing in client-…

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

…outer-content-length Signed-off-by: along <along@nvidia.com>

The router fix sets the request length from the Content-Length header each handler computes. Cover that link end to end: for chat, responses and embeddings, the router must receive no chunked body and a Content-Length that matches the bytes it reads. Refs #2269 Signed-off-by: along <along@nvidia.com>

A fixed length lets the router reject oversized bodies before reading them. It does not stop dead pooled connections after early router responses; that fix is tracked in #2290. Refs: #2269 Signed-off-by: along <along@nvidia.com>

Refs: #2269 Signed-off-by: along <along@nvidia.com>

…outer-content-length

FamousDirector

No P0/P1 found. Both Proxy callers (proxy_handler.go, responses_handler.go) set Content-Length to the buffered body length, so the new fixed-length framing cannot mismatch; the internal transport error is still logged via the middleware's httpErr.Internal path. api and provider tests pass with -race on 2a49f69.

Non-blocking: providerHTTPError is also used by chat_handler, so non-transport provider messages (e.g. "stargate stream ended without chat completion chunks") are now returned as the generic message too; that seems fine but is a broader change than the title suggests. An explicit ContentLength field on ProxyRequest would be less implicit than parsing the header, if you touch this again.

ProxyRequest now carries ContentLength, so Proxy no longer parses the Content-Length header to choose fixed-length framing. Refs: #2269 Signed-off-by: along <along@nvidia.com>

along-2017 · 2026-10-05T22:12:21Z

@FamousDirector thanks for the review.

Generic message for non-transport errors: intended. On the chat path, any provider error that is not an HTTP error from Stargate now returns upstream request failed, for example stargate stream ended without chat completion chunks or decode stargate stream chunk: .... The client cannot act on these, and the full error is still in the completion log and the trace. The status codes are unchanged: these were 502 before and stay 502.
Explicit ContentLength on ProxyRequest: done in 8691677. Proxy now reads the field instead of parsing the header, and both callers set it from the buffered body.

balajinvda · 2026-10-05T22:31:12Z

🎉 This PR is included in src/invocation-plane-services/llm-api-gateway/v0.14.9 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

balajinvda · 2026-10-06T06:41:55Z

This PR is included in version 1.29.4.

The release is available on GitHub release.

balajinvda · 2026-10-06T20:34:23Z

This PR is included in version 1.69.1.

The release is available on GitHub release.

balajinvda · 2026-10-06T20:35:37Z

This PR is included in version 1.25.1.

The release is available on GitHub release.

balajinvda · 2026-10-06T20:36:46Z

This PR is included in version 1.14.2.

The release is available on GitHub release.

balajinvda · 2026-10-06T20:38:12Z

This PR is included in version 1.10.1.

The release is available on GitHub release.

balajinvda · 2026-10-06T20:39:19Z

This PR is included in version 0.5.1.

The release is available on GitHub release.

balajinvda · 2026-10-06T20:40:49Z

This PR is included in version 0.9.1.

The release is available on GitHub release.

along-2017 requested a review from a team as a code owner October 5, 2026 06:06

along-2017 requested a review from shobham-nv October 5, 2026 06:06

along-2017 added 4 commits October 4, 2026 23:24

Merge remote-tracking branch 'origin/main' into fix/llm-api-gateway/r…

a6a049d

…outer-content-length Signed-off-by: along <along@nvidia.com>

test(llm-api-gateway): correct reason in body framing test comments

184837a

Refs: #2269 Signed-off-by: along <along@nvidia.com>

along-2017 self-assigned this Oct 5, 2026

along-2017 requested review from FamousDirector and Max-NV October 5, 2026 19:38

Merge remote-tracking branch 'origin/main' into fix/llm-api-gateway/r…

2a49f69

…outer-content-length

FamousDirector approved these changes Oct 5, 2026

View reviewed changes

Max-NV approved these changes Oct 5, 2026

View reviewed changes

refactor(llm-api-gateway): pass proxy body length as a field

8691677

ProxyRequest now carries ContentLength, so Proxy no longer parses the Content-Length header to choose fixed-length framing. Refs: #2269 Signed-off-by: along <along@nvidia.com>

along-2017 enabled auto-merge October 5, 2026 22:14

along-2017 added this pull request to the merge queue Oct 5, 2026

Merged via the queue into main with commit 13ec516 Oct 5, 2026
24 checks passed

along-2017 deleted the fix/llm-api-gateway/router-content-length branch October 5, 2026 22:29

balajinvda added the released label Oct 5, 2026

Conversation

along-2017 commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TL;DR

Additional Details (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)

For the Reviewer

For QA (optional for docs, build, test, refactor, ci, chore, style, and revert PRs)

Issues

Checklist

Uh oh!

coderabbitai Bot commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

FamousDirector left a comment

Choose a reason for hiding this comment

Uh oh!

along-2017 commented Oct 5, 2026

Uh oh!

Uh oh!

balajinvda commented Oct 5, 2026

Uh oh!

balajinvda commented Oct 6, 2026

Uh oh!

balajinvda commented Oct 6, 2026

Uh oh!

balajinvda commented Oct 6, 2026

Uh oh!

balajinvda commented Oct 6, 2026

Uh oh!

balajinvda commented Oct 6, 2026

Uh oh!

balajinvda commented Oct 6, 2026

Uh oh!

balajinvda commented Oct 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

along-2017 commented Oct 5, 2026 •

edited

Loading

coderabbitai Bot commented Oct 5, 2026 •

edited

Loading