fix: evict dead tunnel routes so offline tunnels 404 instead of 502 - #5930
Open
vishalg0wda wants to merge 1 commit into
Open
fix: evict dead tunnel routes so offline tunnels 404 instead of 502#5930vishalg0wda wants to merge 1 commit into
vishalg0wda wants to merge 1 commit into
Conversation
|
Contributor
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/internal/remotemcp/proxy/proxy.go">
<violation number="1" location="server/internal/remotemcp/proxy/proxy.go:1008">
P2: When the replacement gateway returns a retryable 502, `forwardRequestWithRetry` bypasses `UpstreamResponseRetryer`, so tunnel response errors are returned without eviction or failover. Route the replacement response through the existing response-retry handling, while preserving the intended retry limit.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Thegreatsura
pushed a commit
to Thegreatsura/gram
that referenced
this pull request
Sep 3, 2026
…api#5935) ## Summary Introduces a single-writer `routeReconciler`: one per-pod goroutine owns every mutating route-store call (publish, connection snapshots, unpublish, global revoke). Handlers mark a tunnel dirty and nudge; the reconciler reads live registry state at write time, batch-refreshes all live tunnels on one ticker (replacing the per-session refresher goroutines), gates publishes on `IsActive`, and prunes tracking for cleanly-removed tunnels. Because all writes are serialized through one goroutine, graceful shutdown becomes the reconciler's final act: on SIGTERM the registry stops admitting agent connects (checked again under the lock at registration), the reconciler performs one bounded batched cleanup of every route this pod may own, the forward server drains in-flight requests, agent sessions close concurrently, and `main` blocks until the sequence completes — all within the existing 25s budget. Publish-after-cleanup races are impossible by construction rather than coordinated away. Tests are black-box at the gateway boundary (real WebSocket agent + yamux sessions against an in-memory store): post-drain store emptiness with no writes after drain returns, 503 on connect during drain, in-flight forwards completing through shutdown, TTL refresh cadence, revocation across multiple owners, and a concurrent-churn convergence property under the race detector. ## Motivation The gateway exited without unpublishing its routes, so every rollout left pod-IP routes in Redis for up to the 30s TTL while the pod was already gone. gram-server dialed the dead IP and each affected tunneled request hung ~30s before surfacing a 502, producing an error burst on every deploy. Draining removes the stale-route window; agents re-home to surviving pods in ~0.5–2s using their existing reconnect backoff. Complements speakeasy-api#5888 (fast dial timeout) and speakeasy-api#5930 (dead-route eviction on dial failure), which cover unplanned pod death. closes AIM-166
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.
AIS-662
Summary
proxy.Proxygains aForwardErrorRetryerhook, invoked once when the upstream forward fails with a transport error before any response headers arrive (mirrorsUpstreamResponseRetryer, which only sees gateway-responded errors).tunnelrouting.DeadDialRetryerimplements it for tunnels: on a dead-peer dial failure it unpublishes the dead route, retries another candidate if one exists (the dial failed, so the request never left gram-server and replay is safe for any method), and otherwise surfaces the sameCodeNotFoundthat route selection uses for tunnels with no live route.tunnelManager.buildProxy, so every non-pinned tunnel path (serve endpoint, public tunnels, meta proxy, consent enumeration) gets it.Motivation
#5888 made dead pinned sessions fail fast into a 404, but a stale route (a gateway pod that died without unpublishing) still 502s every non-pinned request for the rest of its 30s TTL, one 3-second dial timeout at a time. Retrying MCP clients rack up enough of those to page the platform 5xx monitors, and on custom domains the traffic is indistinguishable from Gram-origin errors at the ingress (the rewrite to
/mcp/<slug>happens inside gram-server), so no monitor-side exclusion can catch it. Evicting the route on the first failed dial turns a dead tunnel into a customer-visible 404 instead of a platform 502 burst.Summary by cubic
Evicts dead tunnel gateway routes on the first failed dial so offline tunnels return 404 instead of a burst of 502s. Fixes AIS-662.
Bug Fixes
ForwardErrorRetryerhook toproxy.Proxyfor transport errors that occur before any response headers arrive.DeadDialRetryerunpublishes the dead route and fails over to another candidate, or returnsCodeNotFoundwhen none remain.buildProxy; replaying after a dial failure is safe because the request never left gram-server.Written for commit 7b75e7f. Summary will update on new commits.