fix(agentex-ui): abort the upstream request when the BFF client disconnects - #383
Open
erichwoo-scale wants to merge 2 commits into
Open
fix(agentex-ui): abort the upstream request when the BFF client disconnects#383erichwoo-scale wants to merge 2 commits into
erichwoo-scale wants to merge 2 commits into
Conversation
…nnects The `/api/agentex` catch-all proxy streams SSE through unbuffered but passes no signal to `fetch`, so when the browser goes away — closed tab, navigation, or the 300s Istio route timeout — undici keeps the upstream request open. Each orphan leaves an agentex handler blocked in a Redis XREAD, pinning one connection from that pod's pool until it is exhausted and agentex can no longer serve requests. There is no timeout on the call either, so an orphan never expires. Pass `req.signal`, which App Router aborts on client disconnect, and answer the resulting fetch rejection with a 499 so a departed client is not reported as an upstream failure. A fixed timeout would be wrong here — it would cut legitimate long-lived SSE streams. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses review feedback: `req.signal.aborted` alone can mask a genuine transport failure that happens to coincide with a client disconnect, since the flag is already set by the time the catch runs. Match on `error === req.signal.reason` instead. `fetch` rejects with the signal's abort reason itself, so identity separates the two cases without consulting the flag at all. Matching on `error.name === 'AbortError'` would not work here: Next's abort reason is a `ResponseAborted`, and the class name is minified in a production build, so both name checks fail and every disconnect would rethrow as a 500. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
anitaliuu
approved these changes
Jul 29, 2026
deepthi-rao-scale
approved these changes
Jul 29, 2026
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
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.
Problem
The
/api/agentexcatch-all BFF proxy streams SSE through unbuffered but passes nosignaltofetch. When the browser goes away — closed tab, navigation, or the 300s Istio route timeout on the UI'svirtualService— undici keeps the upstream request open. There is no timeout on the call either, so an orphan never expires, and the browser'sEventSourcethen reconnects and opens a fresh one.Each orphan leaves an
agentexhandler blocked in a RedisXREAD, pinning one connection from that pod's redis-py pool (REDIS_MAX_CONNECTIONS, default 200). They accumulate until the pools are exhausted, at which pointagentexfails to serve new requests and only a restart recovers it.Reported on Mayo Nonprod (Slack thread). At saturation Redis showed 1,094
connected_clientswith 943blocked_clients, while each of the twoagentex-uipods held ~570 established sockets to theagentexClusterIP and served one inbound client apiece. Restarting onlyagentex-uidropped backend Redis connections from ~1,094 to 279 without touchingagentex, confirming the UI as the holder. Introduced in 0f07dc7 (#350).Fix
Pass
req.signal. App Router aborts it on client disconnect, which propagates through undici and destroys the upstream request — and makes the existing Istio route timeout an effective ceiling on upstream lifetime. A fixed timeout would be wrong here, since it would cut legitimate long-lived SSE streams.The abort also rejects the in-flight
fetch, so that case is answered with a 499 instead of propagating as a 500 — a departed client isn't an upstream failure. A genuine transport error still propagates exactly as before.Verification
Built this branch, pointed
AGENTEX_API_URLat a connection-counting SSE upstream, opened 5 streaming clients and killed them:mainshapeNo
ResponseAbortedstack traces in the server log, server healthy afterward.The main risk of adding a signal is truncating a response that is still streaming after the handler returns, so that was checked explicitly on a production build:
POSTwithduplex: 'half'echoed back intacttsc --noEmit,eslint --max-warnings=0, andprettier --checkare clean.Follow-ups for the Agents team (not this PR)
Two
agentexbackend issues from the same thread remain open:cleanup_streamin thefinallydeletes a task-wide stream (streams_use_case.py:194). The topic is keyed by task id alone, so it is shared by every viewer. Worth landing with or right after this PR: today thatfinallyalmost never runs because disconnects don't propagate, and this change makes it run on every tab close. If something looks like a regression from this PR, it is that.XREADevery ~2.1s. The naive fix is unsafe: the client's retry loop (custom-subscribe-task-state.tsx,while (!signal?.aborted)) treats a clean server-side stream end as "reconnect immediately" — no backoff, no error-counter increment — so ending the stream server-side alone would produce a reconnect storm. Needs a coordinated client change.Separately and unrelated to the leak:
AGENTEX_API_URLfalls back silently tohttp://localhost:5003, so a rename on either side yields a UI that can't reach the backend with no configuration error.🤖 Generated with Claude Code
Greptile Summary
This PR fixes a connection-pool exhaustion bug where client disconnects (tab close, navigation, Istio timeout) left orphaned upstream SSE connections open indefinitely. The fix passes
req.signalto the upstreamfetchso that undici tears down the upstream request when the browser goes away, and wraps the fetch in a try/catch to return HTTP 499 for clean disconnects rather than propagating a 500.signal: req.signalis added to thefetchoptions, making the Istio route timeout an effective ceiling on upstream lifetime and eliminating the Redis connection leak described in the incident.error === req.signal.reason) rather thanerror.name === 'AbortError', correctly targeting Next.js'sResponseAbortedabort reason while letting genuine transport errors propagate unchanged.Confidence Score: 5/5
Safe to merge — a targeted, well-tested fix that eliminates the orphaned upstream connection leak with no changes to the happy path.
The change is small and surgical: one new try/catch block, one added fetch option. The abort discrimination via identity comparison correctly handles Next.js's ResponseAborted abort reason without relying on error names. The PR description documents explicit verification that streaming bodies are not truncated and that 0 upstream connections survive after all clients disconnect.
Files Needing Attention: No files require special attention.
Important Files Changed
Reviews (2): Last reviewed commit: "fix(agentex-ui): match the abort by reje..." | Re-trigger Greptile