-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix: improve error handling and add guard check to deserializeMessage
#1225
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?
fix: improve error handling and add guard check to deserializeMessage
#1225
Conversation
commit: |
KKonstantinov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your work on this.
Leaving one comment with a question
src/shared/stdio.ts
Outdated
| } catch (error: unknown) { | ||
| // When Non JSONRPC message is received (parsing error or schema validation error), return null | ||
| if (error instanceof ZodError || error instanceof SyntaxError) { | ||
| return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understand the SynthaxError, however previously, the SDK did not return null if JSONRPCMessageSchema.parse threw, why the change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @KKonstantinov, thanks for the review 🙏
I agree - the SDK shouldn’t swallow JSONRPCMessageSchema.parse errors, since throwing in those cases is intentional and important for detecting real protocol violations.
My initial thought was to ignore all invalid JSON-RPC messages, but that would indeed mask real protocol issues.
I’ve updated the fix so that only non-JSON lines are skipped while invalid JSON-RPC messages continue to throw, and I also added an additional test to cover this use case 👍
a8d6474 to
a807cf7
Compare
|
UPDATE: |
This PR adds a guard check to the MCP TypeScript SDK’s stdio message deserialization logic to prevent JSON parsing errors when non-JSON RPC lines are received on the stdio stream.
Motivation and Context
The MCP SDK currently assumes that every line received over stdio is a valid JSON-RPC message.
However, there are scenarios where non-JSON or otherwise invalid JSON RPC data may be written to stdout
whether from the runtime environment, external tooling, or other processes that share the same output stream.
When these invalid messages reach the SDK, the deserializer attempts to parse them and throws a SyntaxError, which impacts overall stability.
This update adds a safeguard that gracefully skips any line that is not valid JSON, making the stdio transport more robust and tolerant of incidental or malformed output on the stream.
How Has This Been Tested?
I added a unit test covering this scenario to ensure that non-JSON RPC valid input is gracefully ignored by the stdio transport.
I'm going to build the MCP Inspector against the RC version of this fix and verify that the issue is no longer reproducible and that invalid JSON-RPC messages are handled safely without causing errors.
Breaking Changes
None.
This change is fully backward compatible
Types of changes
Checklist
Additional context
reference to the issue #700