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

Added default values for error.error and error.message #51

Closed
wants to merge 2 commits into from
Closed

Added default values for error.error and error.message #51

wants to merge 2 commits into from

Conversation

TatriX
Copy link

@TatriX TatriX commented Jul 7, 2015

When tivoka used with http://golang.org/pkg/net/rpc/jsonrpc/ there is no "error" or "message" in server response, so we get and undefined index exception. Fixed it by providing some default values.

@@ -140,8 +140,8 @@ public function setHeaders($raw_headers) {
public function interpretResponse($json_struct) {
//server error?
if(($error = self::interpretError($this->spec, $json_struct, $this->id)) !== FALSE) {
$this->error = $error['error']['code'];
$this->errorMessage = $error['error']['message'];
$this->error = (isset($error['error']['error'])) ? $error['error']['code'] : 'error';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, you mean "code" here:

$this->error = (isset($error['error']['code'])) ? $error['error']['code'] : 'error';

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, of course. My bad.

@rafalwrzeszcz
Copy link
Collaborator

I'm against this - http://www.jsonrpc.org/specification#error_object states clearly that response has to have these properties. If Go extension doesn't return it, it's Go extension error.

@TatriX
Copy link
Author

TatriX commented Jul 7, 2015

Well its a go's standard library implementation. It uses 1.0 spec, and I cannot find anything about error object there: http://json-rpc.org/wiki/specification

@fiddur
Copy link
Collaborator

fiddur commented Aug 18, 2015

Hmm, this PR is included in the other PR as well. As I commented there, null would be a better default for representing "no value", if one is really wanted.

But, since 1.0 does not specify the content of the error member, perhaps it is better to not try to extract code, message and data, but just give access to the error as it is.

@TatriX TatriX closed this Jan 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants