graze / guzzle-jsonrpc

JSON-RPC 2.0 client for Guzzle
https://packagist.org/packages/graze/guzzle-jsonrpc
MIT License
93 stars 61 forks source link

JSONRPC 2.0 compatibility fix: result in response can be null #8

Closed d-ulyanov closed 10 years ago

d-ulyanov commented 10 years ago

Please, see http://www.simple-is-better.org/rpc/#differences-between-1-0-and-2-0

adlawson commented 10 years ago

From the link you gave me, I see no reference to an optional results parameter.

There is, however, a mention of an optional params parameter in the request:

Request: params may be omitted

and there's a mention of either a result or an error parameter in the response:

Response: contains only result OR error (but not both)

Could you elaborate on your interpretation of the spec and the differences described in the link above?

d-ulyanov commented 10 years ago
Array (
    [jsonrpc] => "2.0"
    [result] => null
    [id] => 1
)

It is a correct response, but now \Graze\Guzzle\JsonRpc\Message\Request::send returns ErrorResponse object

adlawson commented 10 years ago

Okay, sounds like a pretty reasonable argument :+1: Could you add some tests to cover the changes you've made?

Thanks.

d-ulyanov commented 10 years ago

Done.

Could you merge it and push new tag? We are using Graze and that fix is very important for us :) Thank you.

adlawson commented 10 years ago

Travis seems to be taking a long time to run the tests. You've only included a test for Request but missed out BatchRequest.

Once I've got tests for both of your changes and the Travis builds work fine, I'll merge and tag 0.4.2.

d-ulyanov commented 10 years ago

Oh, sorry, I'll add tests for BatchRequest

d-ulyanov commented 10 years ago

Done