Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#7301 closed defect (fixed)

JSON RPC (dojox):: Returning true on error response from server (not 200 OK)

Reported by: Goran Miskovic Owned by: Dustin Machi
Priority: high Milestone: 1.2
Component: RPC Version: 1.1.1
Keywords: JSON RPC, error handling Cc: alex
Blocked By: Blocking:

Description

According to JSON-RPC over HTTP specification http://groups.google.com/group/json-rpc/web/json-rpc-over-http section 3.6 Response Codes and JSON-RPC 2.0 (former 1.2) Specification proposal http://groups.google.com/group/json-rpc/web/json-rpc-1-2-proposal section 5 Response (Procedure Return) server must return appropriate HTTP status (message) in case of error.

If server returns any status other than 200 (or 204 in case of notification) JsonRPC call would return true instead of error object.

PROBLEM

In dojox.rpc.JsonRPC in method deserialize on line 47 returned object is deserialized and new object is checked on the next line if it has property error and than new Error object would be created and returned. Here is the problem: returned object in case that server responded with non 200 status will never have property error. To make it worst there will be no property result as well which would lead to returning true on line 53.

deserialize: function(results){
	var obj = dojox.rpc.resolveJson(results);
	if (obj.error) {
		var e = new Error(obj.error.message);
		e._rpcErrorObject = obj.error;
		return e;
	}
	return obj.result || true;
}

Why?

Because class _resHandle dojo.js on line 7339 in case of non 200 respond status would give new error object to dfd.errback and not original JSON string that when deserialized would have properties error and result.

var _resHandle = function(/*Deferred*/dfd){
	var xhr = dfd.ioArgs.xhr;
	if(_d._isDocumentOk(xhr)){
		dfd.callback(dfd);
	}else{
		var err = new Error("Unable to load " + dfd.ioArgs.url + " status:" + xhr.status);
		err.status = xhr.status;
		err.responseText = xhr.responseText;
		dfd.errback(err);
	}
}

Possible reslolution

As far as I can see there are two options:

  1. Instead of creating error object in _resHandle give xhr.responseText to dfd.errbaack instead
  2. Change method deserialize to properly handle errors.

I would vote for the second option. Method deserialize could read:

deserialize: function(results){
	if ('Error' == results.name) {
		var error dojox.rpc.resolveJson(results.responseText);
		var e = new Error(error.message);
		e._rpcErrorObject =error;
		return e;
	}
	if (obj.result) {
		return obj.result;
	} else {
		return new Error("No one would expect that this will happen. :)")
	}
}

There is similar problem in dojo.rpc.JsonService? that I have reported some time ago: http://trac.dojotoolkit.org/ticket/6765

Change History (2)

comment:1 Changed 11 years ago by Kris Zyp

Resolution: fixed
Status: newclosed

(In [14793]) fixes #7301, should handle errors with HTTP error status codes as well as JSON-RPC errors with 200 status codes

comment:2 Changed 11 years ago by dylan

Milestone: tbd1.2
Note: See TracTickets for help on using tickets.