Skip to content
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

fix: update the jsonrpc.Response struct to adhere to the JSONRPC spec #130

Merged
merged 4 commits into from
Jan 13, 2025

Conversation

commoddity
Copy link
Contributor

@commoddity commoddity commented Jan 10, 2025

Summary

Fixes the jsonrpc.Response struct used for qos to adhere to the JSON-RPC spec, as well as update its .Validate method.

Issue

There were some small discrepancies between the official JSON-RPC spec and our jsonrpc.Response struct.

Specifically the error object was always being included even without an actual error, which is prohibited by the JSON-RPC spec.

As well the presence of either neither or both of result and error was not being validated.

Source: https://www.jsonrpc.org/specification#response_object

5 Response object
When a rpc call is made, the Server MUST reply with a Response, except for in the case of Notifications. The Response is expressed as a single JSON Object, with the following members:

jsonrpc
A String specifying the version of the JSON-RPC protocol. MUST be exactly "2.0".

result
This member is REQUIRED on success.
This member MUST NOT exist if there was an error invoking the method.
The value of this member is determined by the method invoked on the Server.

error
This member is REQUIRED on error.
This member MUST NOT exist if there was no error triggered during invocation.
The value for this member MUST be an Object as defined in section 5.1.

...

Either the result member or error member MUST be included, but both members MUST NOT be included.

Type of change

Select one or more from the following:

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Documentation
  • Other (specify)

Testing

  • All Tests: make test_all
  • Documentation: make docusaurus_start; only needed if you make doc changes

Sanity Checklist

  • I have tested my changes using the available tooling
  • I have commented my code
  • I have performed a self-review of my own code; both comments & source code
  • I create and reference any new tickets, if applicable
  • I have left TODOs throughout the codebase, if applicable

@commoddity commoddity added enhancement New feature or request qos Intended to improve quality of service labels Jan 10, 2025
@commoddity commoddity added this to the PATH Quality Of Life milestone Jan 10, 2025
@commoddity commoddity self-assigned this Jan 10, 2025
@commoddity commoddity merged commit 7608628 into main Jan 13, 2025
9 checks passed
@commoddity commoddity deleted the adhere-to-json-rpc-spec branch January 13, 2025 19:39
commoddity added a commit that referenced this pull request Jan 13, 2025
## Summary

#130 contained a small bug
re: the `jsonrpc.Response` struct's `ID` and `Version` fields, which
were missing the type definitions. This PR fixes that bug.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request qos Intended to improve quality of service
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants