Commit 6bc89bc1 authored by Johan Ström's avatar Johan Ström

agorpc: Fix proper JSONRPC error response

This was broken quite a while back (9aafd93e)

Also update test_agorpc.py a bit
parent 7762b2f4
......@@ -209,8 +209,23 @@ void AgoRpc::jsonrpc_message(JsonRpcReqRep* reqRep, boost::unique_lock<boost::mu
response = agoConnection->sendRequest(subject.asString(), content, timeout);
}
AGO_TRACE() << "JsonRPC Response: " << response.getResponse();
responseRoot.swap(response.getResponse());
AGO_TRACE() << "RPC Response: " << response.getResponse();
Json::Value& remoteResponse(response.getResponse());
if(response.isError()) {
// Need to add "code" to make this a proper JsonRPC error.
responseRoot["error"] = Json::Value(Json::objectValue);
responseRoot["error"]["code"] = AGO_JSONRPC_COMMAND_ERROR;
responseRoot["error"]["message"] = remoteResponse["error"]["message"];
if(remoteResponse["error"].isMember("data")) {
responseRoot["error"]["data"].swap(remoteResponse["error"]["data"]);
}
// This is not permitted in jsonrpc spec..
responseRoot["error"]["identifier"] = remoteResponse["error"]["identifier"];
} else {
// Swap the result 1:1 from backend
responseRoot["result"].swap(remoteResponse["result"]);
}
}
......@@ -337,7 +352,7 @@ bool AgoRpc::handleJsonRpcRequest(JsonRpcReqRep *reqRep, const Json::Value &requ
// If ID is missing, this is a notification and no response should be sent (unless error).
if(!request.isMember("id")) {
//... but we do not have any methods which are notifications..
jsonrpcErrorResponse(responseRoot, JSONRPC_INVALID_REQUEST, "Invalid request, 'id' missing");
return jsonrpcErrorResponse(responseRoot, JSONRPC_INVALID_REQUEST, "Invalid request, 'id' missing");
}
responseRoot["id"] = request["id"];
......
......@@ -75,7 +75,7 @@ class RPCTest(unittest.TestCase):
self.assertIn('command', msg.content)
self.assertEquals('some-command', msg.content['command'])
self.assertEquals(1234, msg.content['int-param'])
return {'_newresponse':True, 'result':{'int':4321, 'string':'test'}}
return {'_newresponse':True, 'result':{'int':4321, 'identifier': 'success', 'string':'test'}}
qpr = DummyQpidResponder(self.qpid_session, mock)
try:
......@@ -92,16 +92,15 @@ class RPCTest(unittest.TestCase):
self.assertIn('command', msg.content)
self.assertEquals('some-err-command', msg.content['command'])
self.assertEquals(12345, msg.content['int-param'])
return {'_newresponse':True, 'error':{'int':4321, 'string':'test'}}
return {'_newresponse':True, 'error':{'data':{'int':4321}, 'identifier': 'success', 'message':'test error message'}}
qpr = DummyQpidResponder(self.qpid_session, mock)
try:
qpr.start()
rep = self.bus_message('', {'command':'some-err-command', 'int-param':12345, 'UT-EXP':True}, rpc_error_code=RPC_COMMAND_ERROR)
# rep is error from response; contains message, code and data.
self.assertEquals('Command returned error', rep['message'])
self.assertEquals('test error message', rep['message'])
self.assertEquals(4321, rep['data']['int'])
self.assertEquals('test', rep['data']['string'])
finally:
qpr.shutdown()
......@@ -109,9 +108,10 @@ class RPCTest(unittest.TestCase):
self.jsonrpc_request('message', None, rpc_error_code = RPC_INVALID_PARAMS)
s = time.time()
# Must take at least 0.5s
ret = self.bus_message('', {'command':'non-existent'}, rpc_error_code = RPC_MESSAGE_ERROR, timeout=0.5)
ret = self.bus_message('', {'command':'non-existent'}, rpc_error_code = RPC_COMMAND_ERROR, timeout=0.5)
e = time.time()
self.assertEquals("error.no.reply", ret['message'])
self.assertEquals("error.no.reply", ret['identifier'])
self.assertEquals("Timeout", ret['message'])
self.assertGreaterEqual(e-s, 0.5)
self.assertLessEqual(e-s, 1.0) # but quite near 0.5..
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment