Skip to content

Make Redis truly optional for StreamableHTTPServerTransport #1

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

Closed
patricksevat opened this issue May 13, 2025 · 7 comments
Closed

Make Redis truly optional for StreamableHTTPServerTransport #1

patricksevat opened this issue May 13, 2025 · 7 comments

Comments

@patricksevat
Copy link
Contributor

patricksevat commented May 13, 2025

First of all thanks for creating this package, it's great that you'll be supporting MCP as NextJS Route Handlers!

I'm currently working on a simple StreamableHTTPServerTransport MCP server, but out of the box it doesn't work due to the awaiting on redisPromise : https://github.com/vercel/mcp-adapter/blob/main/src/next/mcp-api-handler.ts#L202

If I comment that line out, it works as expected.

I'm wondering if the whole redis logic could perhaps be extracted away into an own function and only be called within the if-blocks for url.pathname === sseEndpoint and url.pathname === sseMessageEndpoint. The readme lists redis as being optional.

I'd be happy to open a PR with this change

@quuu
Copy link
Collaborator

quuu commented May 14, 2025

hey @patricksevat , thanks a lot for the PR!

I just put up this draft of a slightly different approach: #3

I was planning on deprecating support for SSE going forward (since anthropic has already deprecated it), and as a result removing the reliance on Redis

It looks pretty aligned with your change and i'll get your opinion on it when i flush it out more!

@patricksevat
Copy link
Contributor Author

Hey @quuu , thanks for letting me know. I can work with my fork for now.
I'd be happy to help test once your PR is ready

@quuu
Copy link
Collaborator

quuu commented May 15, 2025

@patricksevat hey patrick, after further thought -- i like your incremental approach more for now while there's a longer term migration for my change!

would you like to re-open your PR? tested your change locally and it was a great improvement

otherwise I don't mind applying your patch

ill update the docs accordingly after merging in your redis change :)

@patricksevat
Copy link
Contributor Author

@quuu cool, reopened!

@quuu
Copy link
Collaborator

quuu commented May 15, 2025

@patricksevat merged, will update this thread when I publish the next version, should be out by later today/ tomorrow after i re- set up the github action to do the release

@quuu
Copy link
Collaborator

quuu commented May 16, 2025

Still need to update docs, but this fix is live as of 0.4.1!

@quuu quuu closed this as completed May 16, 2025
@patricksevat
Copy link
Contributor Author

@quuu awesome, I updated the version, works like expected!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants