-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Added support for raw request injection in RequestContext. #380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Added support for raw request injection in RequestContext. #380
Conversation
All reactions
-
👍 31 reactions -
🎉 14 reactions
@dsp-ant, I am not sure how the reviewing process works here. I was not able to add reviewers to this PR. Will somebody eventually be assigned as reviewer to this PR? |
All reactions
Sorry, something went wrong.
pls merge this it's really essential to develop professional remote mcp servers! |
All reactions
-
👍 14 reactions
Sorry, something went wrong.
Thank you @georg-ort! I am hoping that someone will review it and approve it so that I may merge the PR. |
All reactions
Sorry, something went wrong.
Well I have to thank you @ylassoued! I'm currently using your fork in order to progress since I do need authentication context in my tool calls or our product wouldn't work. |
All reactions
Sorry, something went wrong.
Oh brilliant. Thank you @georg-ort for supporting this. I am glad somebody if finding it useful :-). |
All reactions
Sorry, something went wrong.
Thank you so much for this, @ylassoued! This looks incredibly useful across a variety of contexts. I actually ran into the exact same situation where I needed authentication context in my tool calls and came across your PR. Really appreciate you putting this together. Fingers crossed it gets reviewed and merged soon! |
All reactions
-
👍 1 reaction
Sorry, something went wrong.
Thank you very much @emilioflc. My pleasure :-). |
All reactions
Sorry, something went wrong.
is this good to merge? this pull request is very helpful to me. |
All reactions
-
👍 8 reactions
Sorry, something went wrong.
@dsp-ant Please take some time to check whether this can be merged? |
All reactions
-
👍 6 reactions
Sorry, something went wrong.
Hey, we sort of would like to get this too as we ended up implementing the same thing |
All reactions
-
👍 2 reactions
Sorry, something went wrong.
I'd just spent some time poking around trying to figure out if this could be done with the sdk as is so I'm happy to see there's already a PR for it, I hope it can be merged in because this functionality would make my life a lot easier! Thanks for putting the work in @ylassoued |
All reactions
-
👍 2 reactions
Sorry, something went wrong.
Thank you for the contribution! I'm using your branch locally and see the headers I need. Would love to see this merged. |
All reactions
-
👍 3 reactions
Sorry, something went wrong.
Can we please get a review on this PR? I would love to see it merged. Thank you. |
All reactions
-
👍 2 reactions
Sorry, something went wrong.
When are you planning to launch? We urgently need this feature |
All reactions
-
👍 9 reactions
Sorry, something went wrong.
Please merge. This feature is essential for all remote MCPs |
All reactions
-
👍 3 reactions
Sorry, something went wrong.
Seems good, even implements generics for requests different than Starlette. |
All reactions
-
👍 2 reactions
Sorry, something went wrong.
hi @ylassoued, This pr looks like it injects the header of the first /sse request for all subsequent /messages requests, but if the user needs the header of subsequent requests what do they need to do, such as tools/call and tools/list requests, is there any way for the user to get the header information? I want to carry tracer information in the header of each subsequent /messages related http request and get the related tracer information in tools. |
All reactions
Sorry, something went wrong.
I looked at the client-related code and found that there is now no parameter allowing the user to carry a header in subsequent requests 😭 |
All reactions
Sorry, something went wrong.
Please merge!!! It is really useful for tools authorization!! |
All reactions
Sorry, something went wrong.
I imagined that I'd associate a header value with the |
All reactions
-
😕 1 reaction
Sorry, something went wrong.
All subsequent requests (e.g. tools/call and tools/list) are a separate http request, which should allow the developer to customise the header of each request, but the Python client doesn't have this ability right now |
All reactions
Sorry, something went wrong.
Hi @wenxuwan, @samirbajaj, apologies for the delay in getting back to you. I have to admit, I have not fully understood your concern with carrying the request headers for all subsequent /messages requests. I assumed that when you initialised a client with headers, then it would inject the headers in every request. Am I wrong in assuming so? |
All reactions
Sorry, something went wrong.
hi @ylassoued ,thanks for the reply, what I mean is that apart from the header information in the initialisation phase, I would like to be able to inject some other custom header information, such as the tracer information, in subsequent requests (the trace_id and span_id should be different for each subsequent http request) |
All reactions
Sorry, something went wrong.
Thanks @wenxuwan! Any chance you could write some sample code to illustrate the use case? For example, by writing how ideally the user can make use of the header in the subsequent calls. |
All reactions
Sorry, something went wrong.
I think the basic idea is that an MCP client could send different data (be it headers / different header values or whatever else) for the requests it submits for your tools / resources / prompts. Those headers could contain information unique to each request, like a trace_id as @wenxuwan mentioned. These could in turn be used on the MCP server side for observability (e.g use Grafana, CloudWatch etc to monitor your applications and data processes). Unless I'm missing something, the solution in this PR doesn't account for that fact, and reuses the same initial request that was submitted when an MCP client connected to the /sse endpoint, which is also a bit misleading since you're basically caching it in the request_context, which is supposed to pertain to the actual current request. |
All reactions
-
👍 3 reactions
Sorry, something went wrong.
hi @ylassoued , It's not easy to write a demo because the current python sdk doesn't have this capability, but as @quitrk said, each http request should have its own request_context, and currently your implementation is that all subsequent requests share the request_context of the /sse request, which may solve some of the problems (like the This may solve some of the problems (e.g., authentication of tools), but it's not really right. We should carry the authentication information in the request_context of subsequent requests instead of sharing the request_context of the /sse request. |
All reactions
Sorry, something went wrong.
OK, I see what you mean. Yes, I confirm that in this implementation, it is the initial request that is injected in the context. This was sufficient to solve the authentication issue as you mentioned it already. But I do understand your concern, and I believe now that this implementation is misleading. Instead of injecting the request, I should have simply injected the session-wide initial request headers. But, as you suggested, it would be cleaner to inject the current request for each call instead of injecting the initial one. I'll have to dig into the code again to figure this out... Thanks @wenxuwan, @samirbajaj, and @quitrk! |
All reactions
-
😄 1 reaction
Sorry, something went wrong.
hi @ylassoued ,Currently, all subsequent requests except /sse are handled inside mcp.server.sse.SseServerTransport.handle_post_message, which transforms the request and sends it via writer.send(message) to the mcp.shared.session.BaseSession._receive_loop. This is where the problem occurs, handle_post_message converts the http request to a message while processing the request, resulting in the header information of the http request being lost 😣. Therefore, we can try to stuff the request_context into the message at the time of conversion and make sure that subsequent conversions will carry the request_context. |
All reactions
-
👍 3 reactions
Sorry, something went wrong.
How are we supposed to pass in the correct headers from the mcp client |
All reactions
Sorry, something went wrong.
This is super useful -- since I'm sure the low-level server is in flux with things like Streamable HTTP coming into focus, I've added support for accessing the current Starlette request in the standalone FastMCP repo, without modifying the low-level MCP server. Would welcome thoughts for enhancement there: jlowin/fastmcp#302 |
All reactions
-
❤️ 1 reaction
Sorry, something went wrong.
related #216 |
All reactions
Sorry, something went wrong.
to reiterate, suppose i have a chatbot application which has an mcp client connecting to some remote mcp server. Suppose there are multiple users using this application, each user has an ID to identify themselves and the response from the server at any time depends on the which user has made a request. I don't want to give the user ID in the tool schema since its not upto the chatbot to keep track of user IDs, all i need is a way to pass custom metadata per tool call so that the server returns information pertaining to a particular user. This PR doesn't help with that since the headers are finalised while initialising the client and cannot be changed after app startup |
All reactions
Sorry, something went wrong.
Successfully merging this pull request may close these issues.
Pass entire request object to handlers; add raw request to MCP base Request
closes #195
Added support for injecting the raw request into the
RequestContext
.Motivation and Context
In real-life applications, authentication may rely on request headers. In the case where the MCP server needs to act on behalf of the client to retrieve resources from a remote server or DB, thus requiring to authenticate on behalf of the client, getting the request headers is essential.
With the solution implemented in this PR, you may access the raw request from any end point as follows:
How Has This Been Tested?
Breaking Changes
The only breaking change is in typing. As a matter of fact the following generic classes have now an additional type argument
RequestT
(raw request type):class RequestContext(Generic[SessionT, LifespanContextT, RequestT])
class Server(Generic[LifespanResultT, RequestT])
Types of changes
Checklist
Additional context