Properly infer prefix for SSE messages #659
Merged
+87
−3
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.
I've been following the issue in #386 and see the solution merged in #540. However, it feels very wrong. An app should never have to know its mount path in advance in order to work properly. Furthermore, this results in a cumbersome DX where either users specify the mount paths when creating the FastMCP server, effectively preventing it from being used in any other context or users double-specify mount paths when mounting the SSE app into another Starlette application, which is confusing and awkward.
With that in mind, consider this example from the docs of #540. The GitHub and browser apps will simply break if they are mounted under any other path. This is not a good user-facing outcome.
Happily I don't think we need to settle, or pollute the FastMCP server with new user-facing configuration. The ASGI spec provides for pretty much this exact circumstance with a
root_path
attribute on theScope
object. By examining the root path when constructing the messages POST endpoint, we can ensure that it is always generated at the correct absolute path, without having to perform any complex parsing or even asking users to provide additional configuration.With this PR, the following works out of the box: