Uh oh!
There was an error while loading. Please reload this page.
Fix 307 Temporary Redirect when use streamable_http - #781
Conversation
chi2liu
commented
May 22, 2025
chi2liu
commented
May 22, 2025
when we set the default /mcp, the log will be, in some env will in some env will be hanging, because 307 Temporary Redirect to /byoa/mcp/mcp/, this is unreachable when we set the streamable_http to '/mcp/', it will be good, as the log: |
chi2liu
commented
May 22, 2025
chi2liu
commented
May 22, 2025
ihrpr
left a comment
There was a problem hiding this comment.
Thanks for identifying and reporting this redirect issue! However, I don't think changing the default
streamable_http_path is the right approach here.
The core issue: This change creates inconsistency across the codebase rather than solving the root problem.
- All client code constructs URLs with /mcp (no trailing slash) - see examples/clients/simple-auth-client/main.py:344
- All example servers mount at /mcp (no trailing slash) - see both simple-streamablehttp examples
- All tests use /mcp (no trailing slash) consistently - see tests/shared/test_streamable_http.py:238
- Only FastMCP would have a different default with this change
Problems with this approach:
- Breaking consistency: Makes FastMCP behave differently from all examples and existing usage patterns
- Incomplete fix: The examples and tests still use /mcp without trailing slash, so the inconsistency remains
- Wrong direction: The entire ecosystem already standardized on /mcp - we shouldn't change that
Alterative:
- Keep the default as /mcp and fix the ASGI mounting logic to handle redirects properly
- Add logic to FastMCP to handle both /mcp and /mcp/ transparently at the mount level
- Document the trailing slash behavior so users can choose the approach that works for their environment
The redirect issue you've identified is real and needs fixing, but changing the default path creates more problems than
it solves. Let's find a solution that maintains consistency while addressing the underlying ASGI routing behavior.
Thank you for your review and helpful suggestions.
In this way, whether the client, example, or test uses /mcp or /mcp/, it can be seamlessly compatible without modifying the existing code. The problem has been completely solved. |
Uh oh!
There was an error while loading. Please reload this page.
vectorstain
commented
May 27, 2025
@chi2liu please update the branch with master and check again tests results |
ihrpr
left a comment
There was a problem hiding this comment.
Thank you!
Please can you add tests to verify all works as expected?
…iu/python-sdk into fix-307-Temporary-Redirect
chi2liu
commented
May 29, 2025
I have modified it according to your suggestion. Could you please review it? @Kludex |
| starlette_app = self.streamable_http_app() | ||
| starlette_app.router.redirect_slashes = False |
There was a problem hiding this comment.
Hmmm... I think we can add redirect_slashes in the Starlette constructor on starlette's side.
But this part is fine.
There was a problem hiding this comment.
@Kludex So do we need to add this parameter to starlette now?
| async def handle_streamable_http(request: Request) -> Response: | ||
| await self.session_manager.handle_request( | ||
| request.scope, request.receive, request._send | ||
| ) | ||
| return Response() |
There was a problem hiding this comment.
This is not fine. Does handle_request needs to be an ASGI application?
| # Always register both /mcp and /mcp/ for full compatibility | ||
| _main_path = self.settings.streamable_http_path.removesuffix("/") | ||
| _alt_path = _main_path + "/" |
There was a problem hiding this comment.
I think the problem was the redirect, but if you have a 404 then it's not an issue anymore, and we shouldn't be needing this.
Uh oh!
There was an error while loading. Please reload this page.
* Refactor FastMCP routing to use Router and streamline request handling * Add async support to token validation test and enhance metadata snapshot assertions * Fix tests * Fix tests --------- Co-authored-by: Vincenzo Maria Calandra <vincenzomariacalandra@MacBook-Pro-di-Vincenzo.local> Co-authored-by: 633WHU <cliu_whu@yeah.net>
chi2liu
commented
May 30, 2025
Updated the latest code, please help review @Kludex@vectorstain@mroch@ihrpr |
Kludex
commented
May 30, 2025
You marked my comments as resolved, but you actually ignore them. |
chi2liu
commented
May 30, 2025
Restored, please check one by one |
tampiss
commented
Jun 9, 2025
🙏🏼 |
alenprodan
commented
Jun 24, 2025
Hi everyone! I'm experiencing the 307 redirect issue described in this PR. Thanks for your work on fixing this! |
vectorstain
commented
Jun 25, 2025
Hi @alenprodan, we are waiting for a feedback @ihrpr |
The default mount path of Streamable HTTP is /mcp, but in some environments, a 307 Temporary Redirect to /mcp/mcp/ will appear, causing the request to hang or fail. When the streamable_http path is set to /mcp/, everything works fine.
Code Analysis
Mount Path
In the server.py and simple-streamablehttp-stateless examples, the ASGI routes are Mount("/mcp", app=handle_streamable_http), that is, there is no trailing slash.
In the Settings class of server.py, the default value of streamable_http_path is "/mcp" (also without a slash).
Redirection Issue
Mount("/mcp", ...) route of Starlette/FastAPI, if /mcp is requested, will automatically 307 redirect to /mcp/ (with a slash).
But if the client requests /mcp/ and the server only mounts /mcp, a redirect chain of /mcp/mcp/ will appear in some environments, resulting in "unreachable".
This is related to the "strict slash matching" of ASGI routing.
Solution
Recommended practice: Always use /mcp/ as the mounting path (with a slash) and let the client request /mcp/, so there will be no redirection problem.
Or, when mounting /mcp, make sure the client only requests /mcp without a slash.
Motivation and Context
How Has This Been Tested?
Breaking Changes
Types of changes
Checklist
Additional context