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

Methods that return nothing must contain result field #102

Closed
oblique opened this issue Jul 8, 2023 · 3 comments · Fixed by #107
Closed

Methods that return nothing must contain result field #102

oblique opened this issue Jul 8, 2023 · 3 comments · Fixed by #107

Comments

@oblique
Copy link

oblique commented Jul 8, 2023

We were trying to call a method that returns nothing but the library that we use in the client rejects to deserialize the response.

After some investigation we found the following in JSON RPC 2.0 spec:

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.

So on success the result field must exists.

The following unit tests are wrong:

go-jsonrpc/rpc_test.go

Lines 130 to 133 in 17a3577

t.Run("inc", tc(`{"jsonrpc": "2.0", "method": "SimpleServerHandler.Inc", "params": [], "id": 1}`, `{"jsonrpc":"2.0","id":1}`, 1, 200))
t.Run("inc-null", tc(`{"jsonrpc": "2.0", "method": "SimpleServerHandler.Inc", "params": null, "id": 1}`, `{"jsonrpc":"2.0","id":1}`, 1, 200))
t.Run("inc-noparam", tc(`{"jsonrpc": "2.0", "method": "SimpleServerHandler.Inc", "id": 2}`, `{"jsonrpc":"2.0","id":2}`, 1, 200))
t.Run("add", tc(`{"jsonrpc": "2.0", "method": "SimpleServerHandler.Add", "params": [10], "id": 4}`, `{"jsonrpc":"2.0","id":4}`, 10, 200))

@oblique
Copy link
Author

oblique commented Jul 9, 2023

This is what other servers return on a successful call:

{
    "jsonrpc": "2.0",
    "result": null,
    "id": 1
}

@Wondertan
Copy link

@oblique, so to make this spec compliant, we would change unit tests to return "result": null, in the expected response, correct?

@zvolin
Copy link

zvolin commented Sep 8, 2023

yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants
@oblique @Wondertan @zvolin and others