modelcontextprotocol / modelcontextprotocol/typescript-sdk
Promise/async handling causes unhandled rejections
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 13.4k
- Forks
- 2.2k
- Avg merge
- 3d 15h
- Merged PRs (30d)
- 4
Description
Describe the bug
If an error occurs while sending an error response in a request handler() it remains unhandled if there's no onerror handler on the protocol (which is the default)
This means that nothing is returned to the client and the promise rejection goes unhandled. On Cloudflare Workers, this will result in an error like this: A hanging Promise was canceled. This happens when the worker runtime is waiting for a Promise from JavaScript to resolve, but has detected that the Promise cannot possibly ever resolve because all code and events related to the Promise's I/O context have already finished., and on Node.js, I suspect this would result in an unhandledRejection event though I haven't confirmed this – it may just be that the request stays open and eventually times out.
The problem is it's very hard to know exactly what's happened (or even that adding an onerror handler may help)
Expected behavior
Currently the protocol fires off Promises without awaiting them. Messages are processed in a loop without waiting, which means they all get processed in parallel, with no limits or waiting for execution. And the _onrequest handler creates a Promise without awaiting or returning it.
This kind of event handling becomes difficult to manage unless all event handlers ultimately resolve (including error handling). In the case of unhandled rejections, it's often not clear where the rejection happened, if it's even logged / managed at all – the error could just disappear into the ether if there's no error handler.
It also means it's very easy for errors to be handled multiple times, and if each handler writes a message on the transport, you then end up with Cannot write headers after they are sent to the client.
Ideally, install default error handlers that can handle an error occurring anywhere and won't leave unhandled promises/rejections. Having a default handler that at least does console.error would ensure that users should see the error, even if it's not handled gracefully.
Longer term I'd love to see this refactored to use async/await instead of events – it should be much easier to reason about, much easier to handle concurrency and flow control (eg message processing), and much easier to ensure errors or responses aren't sent over the wire multiple times.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading the request handler in src/shared/protocol.ts around lines 374-411 and the message-processing loop in src/server/streamableHttp.ts around lines 429-431. Trace how rejected Promises and errors flow when no onerror handler is configured. Done means failures are surfaced or handled without unhandled rejections, missing client responses, or duplicate transport writes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- api, backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100