Skip to content

mcp: better handle 405 on GET requests#1980

Open
nacx wants to merge 3 commits intoenvoyproxy:mainfrom
nacx:mcp-get-open
Open

mcp: better handle 405 on GET requests#1980
nacx wants to merge 3 commits intoenvoyproxy:mainfrom
nacx:mcp-get-open

Conversation

@nacx
Copy link
Copy Markdown
Member

@nacx nacx commented Mar 21, 2026

Description

Changes how the GET requests are managed when some backends return 405.

The main challenge is that the GET implementation always returns a 202, no matter what the upstream backends return. When all upstream backends return a 405, that translates into an immediate 202 completed, and clients reconnect, causing the constant reconnection loop.

Ideally, when all backends return 405, we should return a 405 as well, but this is challenging: it would require for us to not return the response status until we have received a 405 from all backends, and if some backend does not return and it's just waiting to start streaming events, we may not be able to return the right response code anything until we've received the first event, which may be waiting too long.

This PR addresses it in a different way, by always returning a 202 and keeping the stream open between the client and the gateway, even when all backends return 405, or they return EOF after stopping sending events. This prevents the client reconnection loop and keeps the stream open to send gateway-related notifications, such as tool changes. Even though upstream backends don't have more events to report, the gateway itself may still use it to send notifications, so I think it is OK to keep it open and move the control to the client, and let clients decide when to close the stream or reopen it.

Related Issues/PRs (if applicable)

Fixes #1924
Fixes #1925

Special notes for reviewers (if applicable)

N/A

@nacx
Copy link
Copy Markdown
Member Author

nacx commented Mar 21, 2026

@mathetake WDYT about this approach?

nacx added 2 commits March 21, 2026 21:31
Signed-off-by: Ignasi Barrera <nacx@apache.org>
Signed-off-by: Ignasi Barrera <nacx@apache.org>
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.37%. Comparing base (2d35d43) to head (ee65bd4).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1980      +/-   ##
==========================================
+ Coverage   84.33%   84.37%   +0.03%     
==========================================
  Files         130      130              
  Lines       17987    17988       +1     
==========================================
+ Hits        15170    15177       +7     
+ Misses       1873     1868       -5     
+ Partials      944      943       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nacx
Copy link
Copy Markdown
Member Author

nacx commented Mar 21, 2026

/retest

@nacx nacx marked this pull request as ready for review March 29, 2026 08:35
@nacx nacx requested a review from a team as a code owner March 29, 2026 08:35
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Mar 29, 2026
@nacx nacx requested review from johnugeorge and mathetake March 29, 2026 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gateway requires persistent SSE stream Instability with one backend affects all backends

2 participants