Updates SDK error codes to use JSON-RPC server error range #103
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The SDK's custom error codes were previously using arbitrary negative numbers (-1, -2) which could conflict with application-specific error codes. This change: - Updates ConnectionClosed from -1 to -32000 - Updates RequestTimeout from -2 to -32001 - Keeps standard JSON-RPC error codes unchanged - Adds documentation explaining the server error range usage This modification ensures compliance with the JSON-RPC 2.0 specification, which reserves -32000 to -32099 for implementation-defined server errors. This change allows applications to freely use other error code ranges without potential conflicts with SDK-generated errors. Issue: modelcontextprotocol#85
…est timeout and connection closed scenarios
|
Hi, @jspahrsummers let me know if any changes are required in this PR. |
jspahrsummers
requested changes
Jan 2, 2025
src/types.ts
Outdated
| @@ -100,13 +100,16 @@ export const JSONRPCResponseSchema = z | |||
| .strict(); | |||
|
|
|||
| /** | |||
| * An incomplete set of error codes that may appear in JSON-RPC responses. | |||
| */ | |||
| * @author : Sumitesh Naithani | |||
There was a problem hiding this comment.
Please remove this by-line. Your contribution will definitely be in the Git commit log and repository history—we just don't want to end up having annotations like this everywhere in the code. 🙏
src/types.ts
Outdated
Comment on lines
104
to
106
| * @link : https://docs.trafficserver.apache.org/en/latest/developer-guide/jsonrpc/jsonrpc-node-errors.en.html#standard-errors | ||
| * @description : An incomplete set of error codes that may appear in JSON-RPC responses. | ||
| * @note : SDK-specific errors should use the server error range (-32000 to -32099), as per JSON-RPC 2.0 specification. |
There was a problem hiding this comment.
We're not using a documentation generator that will make use of these kind of @ annotations right now. Can you please reformat as a normal documentation comment?
src/shared/protocol.test.ts
Outdated
| result: z.string(), | ||
| }); | ||
| await protocol.request(request, mockSchema, { | ||
| timeout: 100, |
There was a problem hiding this comment.
We don't want to perform any waiting in tests, however small, as this will degrade overall testing times:
Suggested change
| timeout: 100, | |
| timeout: 0, |
…ol test timeout to 0
jspahrsummers
approved these changes
Jan 3, 2025
MediaInfluences
pushed a commit
to MediaInfluences/typescript-sdk
that referenced
this pull request
Apr 3, 2025
…/fix-error-codes Updates SDK error codes to use JSON-RPC server error range
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Updates SDK error codes to use the JSON-RPC server error range (-32000 to -32099) instead of arbitrary negative numbers.
Motivation and Context
The SDK was using arbitrary negative numbers (-1, -2) for custom error codes, which could potentially conflict with application-specific error codes. By restricting SDK errors to the JSON-RPC server error range (-32000 to -32099), we ensure compliance with the specification and leave other ranges available for application usage. This change makes the error handling more standardized and prevents potential conflicts.
How Has This Been Tested?
Breaking Changes
This may be a breaking change as it modifies error codes that applications might be checking against. Applications that explicitly check for -1 or -2 error codes will need to update their checks to use the new values (-32000 and -32001 respectively).
Types of changes
Checklist
Additional context
This change aligns with JSON-RPC 2.0 specification's recommendation for implementation-defined server errors. The specification reserves codes -32000 to -32099 for this purpose, making it a natural fit for SDK-generated errors. I've chosen to start at -32000 and -32001, leaving plenty of room for additional SDK error codes in the future while maintaining a clear separation from application-specific error codes.