Skip to content

Commit 096b4f6

Browse files
committed
[rpc] Add call error support by returning error code and message
1 parent 87e1cc2 commit 096b4f6

File tree

7 files changed

+191
-44
lines changed

7 files changed

+191
-44
lines changed

src/messages/GenericMessageSender.h

Lines changed: 75 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@ enum class CallResult
3636
/** @brief Message will be sent later */
3737
Delayed,
3838
/** @brief Message cannot be send or no response has been received */
39-
Failed
39+
Failed,
40+
/** @brief A call error message has been received */
41+
Error
4042
};
4143

4244
/** @brief Generic message sender with C++ data type to JSON conversion */
@@ -79,6 +81,46 @@ class GenericMessageSender
7981
ResponseType& response,
8082
IRequestFifo* request_fifo = nullptr,
8183
unsigned int connector_id = 0)
84+
{
85+
std::string error;
86+
std::string message;
87+
return call(action, request, response, error, message, request_fifo, connector_id);
88+
}
89+
90+
/**
91+
* @brief Execute a call request
92+
* @param action RPC action for the request
93+
* @param request Request payload
94+
* @param response Response payload
95+
* @param error Error (Empty if not a CallError)
96+
* @param message Error message (Empty if not a CallError)
97+
* @return Result of the call request (See CallResult documentation)
98+
*/
99+
template <typename RequestType, typename ResponseType>
100+
CallResult call(const std::string& action, const RequestType& request, ResponseType& response, std::string& error, std::string& message)
101+
{
102+
return call(action, request, response, error, message, nullptr, 0);
103+
}
104+
105+
/**
106+
* @brief Execute a call request
107+
* @param action RPC action for the request
108+
* @param request Request payload
109+
* @param response Response payload
110+
* @param error Error (Empty if not a CallError)
111+
* @param message Error message (Empty if not a CallError)
112+
* @param request_fifo Pointer to the request FIFO to use when messages cannot be sent.
113+
* @param connector_id Id of the connector associated to the request.
114+
* @return Result of the call request (See CallResult documentation)
115+
*/
116+
template <typename RequestType, typename ResponseType>
117+
CallResult call(const std::string& action,
118+
const RequestType& request,
119+
ResponseType& response,
120+
std::string& error,
121+
std::string& message,
122+
IRequestFifo* request_fifo,
123+
unsigned int connector_id)
82124
{
83125
CallResult ret = CallResult::Failed;
84126

@@ -99,15 +141,23 @@ class GenericMessageSender
99141
// Execute call
100142
rapidjson::Document rpc_frame;
101143
rapidjson::Value resp;
102-
if (m_rpc.call(action, payload, rpc_frame, resp, m_timeout))
144+
if (m_rpc.call(action, payload, rpc_frame, resp, error, message, m_timeout))
103145
{
104-
// Convert response
105-
const char* error_code = nullptr;
106-
std::string error_message;
107-
resp_converter->setAllocator(&rpc_frame.GetAllocator());
108-
if (resp_converter->fromJson(resp, response, error_code, error_message))
146+
// Check error
147+
if (error.empty())
148+
{
149+
// Convert response
150+
const char* error_code = nullptr;
151+
std::string error_message;
152+
resp_converter->setAllocator(&rpc_frame.GetAllocator());
153+
if (resp_converter->fromJson(resp, response, error_code, error_message))
154+
{
155+
ret = CallResult::Ok;
156+
}
157+
}
158+
else
109159
{
110-
ret = CallResult::Ok;
160+
ret = CallResult::Error;
111161
}
112162
}
113163
else
@@ -151,15 +201,25 @@ class GenericMessageSender
151201
// Execute call
152202
rapidjson::Document rpc_frame;
153203
rapidjson::Value resp;
154-
if (m_rpc.call(action, request, rpc_frame, resp, m_timeout))
204+
std::string error;
205+
std::string message;
206+
if (m_rpc.call(action, request, rpc_frame, resp, error, message, m_timeout))
155207
{
156-
// Convert response
157-
const char* error_code = nullptr;
158-
std::string error_message;
159-
resp_converter->setAllocator(&rpc_frame.GetAllocator());
160-
if (resp_converter->fromJson(resp, response, error_code, error_message))
208+
// Check error
209+
if (error.empty())
210+
{
211+
// Convert response
212+
const char* error_code = nullptr;
213+
std::string error_message;
214+
resp_converter->setAllocator(&rpc_frame.GetAllocator());
215+
if (resp_converter->fromJson(resp, response, error_code, error_message))
216+
{
217+
ret = CallResult::Ok;
218+
}
219+
}
220+
else
161221
{
162-
ret = CallResult::Ok;
222+
ret = CallResult::Error;
163223
}
164224
}
165225
}

src/rpc/IRpc.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,17 @@ class IRpc
5252
* @param payload JSON payload for the action
5353
* @param rpc_frame Full JSON response received
5454
* @param response JSON response received
55+
* @param error Error code (empty if no error)
56+
* @param message Error message (empty if no error)
5557
* @param timeout Response timeout
5658
* @return true if a response has been received, false otherwise
5759
*/
5860
virtual bool call(const std::string& action,
5961
const rapidjson::Document& payload,
6062
rapidjson::Document& rpc_frame,
6163
rapidjson::Value& response,
64+
std::string& error,
65+
std::string& message,
6266
std::chrono::milliseconds timeout = std::chrono::seconds(2)) = 0;
6367

6468
/**

src/rpc/RpcBase.cpp

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,14 @@ RpcBase::~RpcBase()
4545
stop();
4646
}
4747

48-
/** @copydoc bool IRpc::call(const std::string&, const rapidjson::Document&, rapidjson::Document&, rapidjson::Value&, std::chrono::milliseconds) */
48+
/** @copydoc bool IRpc::call(const std::string&, const rapidjson::Document&, rapidjson::Document&, rapidjson::Value&,
49+
* std::string&, std::string&, std::chrono::milliseconds) */
4950
bool RpcBase::call(const std::string& action,
5051
const rapidjson::Document& payload,
5152
rapidjson::Document& rpc_frame,
5253
rapidjson::Value& response,
54+
std::string& error,
55+
std::string& message,
5356
std::chrono::milliseconds timeout)
5457
{
5558
bool ret = false;
@@ -113,6 +116,16 @@ bool RpcBase::call(const std::string& action,
113116
{
114117
rpc_frame.Swap(rpc_message->rpc_frame);
115118
response.Swap(rpc_message->payload);
119+
error.clear();
120+
message.clear();
121+
if (!rpc_message->error.IsNull())
122+
{
123+
error = rpc_message->error.GetString();
124+
}
125+
if (!rpc_message->message.IsNull())
126+
{
127+
message = rpc_message->message.GetString();
128+
}
116129
delete rpc_message;
117130
}
118131
else
@@ -324,19 +337,21 @@ bool RpcBase::decodeCallResult(const std::string& unique_id, rapidjson::Document
324337
}
325338

326339
/** @brief Decode a CALLERROR message */
327-
bool RpcBase::decodeCallError(const std::string& unique_id,
328-
rapidjson::Document& rpc_frame,
329-
const rapidjson::Value& error,
330-
const rapidjson::Value& message,
331-
const rapidjson::Value& payload)
340+
bool RpcBase::decodeCallError(const std::string& unique_id,
341+
rapidjson::Document& rpc_frame,
342+
rapidjson::Value& error,
343+
rapidjson::Value& message,
344+
rapidjson::Value& payload)
332345
{
333346
bool ret = false;
334347

335348
// Check types
336349
if (error.IsString() && message.IsString() && payload.IsObject())
337350
{
338-
(void)unique_id;
339-
(void)rpc_frame;
351+
// Add error to the queue
352+
RpcMessage* msg = new RpcMessage(unique_id, rpc_frame, payload, &error, &message);
353+
m_results_queue.push(msg);
354+
340355
ret = true;
341356
}
342357

src/rpc/RpcBase.h

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,14 @@ class RpcBase : public IRpc
4444

4545
// IRpc interface
4646

47-
/** @copydoc bool IRpc::call(const std::string&, const rapidjson::Document&, rapidjson::Document&, rapidjson::Value&, std::chrono::milliseconds) */
47+
/** @copydoc bool IRpc::call(const std::string&, const rapidjson::Document&, rapidjson::Document&, rapidjson::Value&,
48+
* std::string&, std::string&, std::chrono::milliseconds) */
4849
bool call(const std::string& action,
4950
const rapidjson::Document& payload,
5051
rapidjson::Document& rpc_frame,
5152
rapidjson::Value& response,
53+
std::string& error,
54+
std::string& message,
5255
std::chrono::milliseconds timeout = std::chrono::seconds(2)) override;
5356

5457
/** @copydoc void IRpc::registerListener(IListener&) */
@@ -88,19 +91,33 @@ class RpcBase : public IRpc
8891
struct RpcMessage
8992
{
9093
RpcMessage(const std::string& _unique_id, const char* _action, rapidjson::Document& _rpc_frame, rapidjson::Value& _payload)
91-
: unique_id(_unique_id), action(_action), rpc_frame(std::move(_rpc_frame)), payload()
94+
: unique_id(_unique_id), action(_action), rpc_frame(std::move(_rpc_frame)), payload(), error(), message()
9295
{
9396
payload.Swap(_payload);
9497
}
95-
RpcMessage(const std::string& _unique_id, rapidjson::Document& _rpc_frame, rapidjson::Value& _payload)
96-
: unique_id(_unique_id), action(), rpc_frame(std::move(_rpc_frame)), payload()
98+
RpcMessage(const std::string& _unique_id,
99+
rapidjson::Document& _rpc_frame,
100+
rapidjson::Value& _payload,
101+
rapidjson::Value* _error = nullptr,
102+
rapidjson::Value* _message = nullptr)
103+
: unique_id(_unique_id), action(), rpc_frame(std::move(_rpc_frame)), payload(), error(), message()
97104
{
98105
payload.Swap(_payload);
106+
if (_error)
107+
{
108+
error.Swap(*_error);
109+
}
110+
if (_message)
111+
{
112+
message.Swap(*_message);
113+
}
99114
}
100115
const std::string unique_id;
101116
const std::string action;
102117
rapidjson::Document rpc_frame;
103118
rapidjson::Value payload;
119+
rapidjson::Value error;
120+
rapidjson::Value message;
104121
};
105122

106123
/** @brief RPC listener */
@@ -131,11 +148,11 @@ class RpcBase : public IRpc
131148
bool decodeCallResult(const std::string& unique_id, rapidjson::Document& rpc_frame, rapidjson::Value& payload);
132149

133150
/** @brief Decode a CALLERROR message */
134-
bool decodeCallError(const std::string& unique_id,
135-
rapidjson::Document& rpc_frame,
136-
const rapidjson::Value& error,
137-
const rapidjson::Value& message,
138-
const rapidjson::Value& payload);
151+
bool decodeCallError(const std::string& unique_id,
152+
rapidjson::Document& rpc_frame,
153+
rapidjson::Value& error,
154+
rapidjson::Value& message,
155+
rapidjson::Value& payload);
139156

140157
/** @brief Send a CALLERROR message */
141158
void sendCallError(const std::string& unique_id, const char* error, const std::string& message);

tests/rpc/test_rpc.cpp

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ static constexpr const char* EXPECTED_CALL_MESSAGE_1 = "[2, \"1\", \"Heart
166166
static constexpr const char* EXPECTED_CALL_MESSAGE_2 = "[2, \"2\", \"Heartbeat\", {\"id\":4}]";
167167
static constexpr const char* EXPECTED_CALLRESULT_MESSAGE_1 = "[3, \"1\", {\"name\":\"bob\"}]";
168168
static constexpr const char* EXPECTED_CALLRESULT_MESSAGE_2 = "[3, \"2\", {\"name\":\"bob\"}]";
169-
static constexpr const char* EXPECTED_CALLERROR_MESSAGE_1 = "[4, \"1\", \"NotImplemented\", \"This is an error!\", {}]";
169+
static constexpr const char* EXPECTED_CALLERROR_MESSAGE_0 = "[4, \"0\", \"NotImplemented\", \"This is an error!\", {}]";
170170

171171
TEST_SUITE("CALL messages")
172172
{
@@ -184,14 +184,16 @@ TEST_SUITE("CALL messages")
184184

185185
rapidjson::Document rpc_frame;
186186
rapidjson::Value response;
187+
std::string error;
188+
std::string message;
187189
rapidjson::StringBuffer buffer;
188190
rapidjson::Writer<rapidjson::StringBuffer> writer(buffer);
189191

190-
CHECK_FALSE(client.call(ACTION, payload, rpc_frame, response, std::chrono::milliseconds(0)));
192+
CHECK_FALSE(client.call(ACTION, payload, rpc_frame, response, error, message, std::chrono::milliseconds(0)));
191193
CHECK(websocket.sendCalled());
192194
CHECK_EQ(strcmp(reinterpret_cast<const char*>(websocket.sentData()), EXPECTED_CALL_MESSAGE_0), 0);
193195

194-
CHECK_FALSE(client.call(ACTION, payload, rpc_frame, response, std::chrono::milliseconds(0)));
196+
CHECK_FALSE(client.call(ACTION, payload, rpc_frame, response, error, message, std::chrono::milliseconds(0)));
195197
CHECK_EQ(strcmp(reinterpret_cast<const char*>(websocket.sentData()), EXPECTED_CALL_MESSAGE_1), 0);
196198

197199
std::thread response_thread(
@@ -200,13 +202,41 @@ TEST_SUITE("CALL messages")
200202
std::this_thread::sleep_for(std::chrono::milliseconds(25u));
201203
websocket.notifyDataReceived(EXPECTED_CALLRESULT_MESSAGE_2, strlen(EXPECTED_CALLRESULT_MESSAGE_2));
202204
});
203-
CHECK(client.call(ACTION, payload, rpc_frame, response, std::chrono::milliseconds(50)));
205+
CHECK(client.call(ACTION, payload, rpc_frame, response, error, message, std::chrono::milliseconds(50)));
204206
CHECK_EQ(strcmp(reinterpret_cast<const char*>(websocket.sentData()), EXPECTED_CALL_MESSAGE_2), 0);
205207
response.Accept(writer);
206208
CHECK_EQ(strcmp(buffer.GetString(), CALLRESULT_PAYLOAD), 0);
207209
response_thread.join();
208210
}
209211

212+
TEST_CASE("Call error")
213+
{
214+
RpcClientListener listener;
215+
WebsocketClientStub websocket;
216+
RpcClient client(websocket, WS_PROTOCOL);
217+
client.registerListener(listener);
218+
client.registerClientListener(listener);
219+
websocket.setConnected();
220+
221+
rapidjson::Document payload;
222+
rapidjson::Document rpc_frame;
223+
rapidjson::Value response;
224+
std::string error;
225+
std::string message;
226+
227+
std::thread response_thread(
228+
[&websocket]
229+
{
230+
std::this_thread::sleep_for(std::chrono::milliseconds(25u));
231+
websocket.notifyDataReceived(EXPECTED_CALLERROR_MESSAGE_0, strlen(EXPECTED_CALLERROR_MESSAGE_0));
232+
});
233+
CHECK(client.call(ACTION, payload, rpc_frame, response, error, message, std::chrono::milliseconds(50)));
234+
CHECK_EQ(error, "NotImplemented");
235+
CHECK_EQ(message, "This is an error!");
236+
CHECK(response.IsObject());
237+
response_thread.join();
238+
}
239+
210240
TEST_CASE("Timeout")
211241
{
212242
RpcClientListener listener;
@@ -219,16 +249,18 @@ TEST_SUITE("CALL messages")
219249
rapidjson::Document payload;
220250
rapidjson::Document rpc_frame;
221251
rapidjson::Value response;
252+
std::string error;
253+
std::string message;
222254

223255
auto start = std::chrono::steady_clock::now();
224-
CHECK_FALSE(client.call(ACTION, payload, rpc_frame, response, std::chrono::milliseconds(0)));
256+
CHECK_FALSE(client.call(ACTION, payload, rpc_frame, response, error, message, std::chrono::milliseconds(0)));
225257
auto end = std::chrono::steady_clock::now();
226258
std::chrono::duration<double> diff = end - start;
227259
CHECK_LT(diff, std::chrono::milliseconds(5u));
228260
CHECK(websocket.sendCalled());
229261

230262
start = std::chrono::steady_clock::now();
231-
CHECK_FALSE(client.call(ACTION, payload, rpc_frame, response, std::chrono::milliseconds(100)));
263+
CHECK_FALSE(client.call(ACTION, payload, rpc_frame, response, error, message, std::chrono::milliseconds(100)));
232264
end = std::chrono::steady_clock::now();
233265
diff = end - start;
234266
CHECK_GT(diff, std::chrono::milliseconds(99u));
@@ -240,7 +272,7 @@ TEST_SUITE("CALL messages")
240272
std::this_thread::sleep_for(std::chrono::milliseconds(100u));
241273
websocket.notifyDataReceived(EXPECTED_CALLRESULT_MESSAGE_2, strlen(EXPECTED_CALLRESULT_MESSAGE_2));
242274
});
243-
CHECK_FALSE(client.call(ACTION, payload, rpc_frame, response, std::chrono::milliseconds(50)));
275+
CHECK_FALSE(client.call(ACTION, payload, rpc_frame, response, error, message, std::chrono::milliseconds(50)));
244276
response_thread.join();
245277
}
246278

@@ -277,11 +309,11 @@ TEST_SUITE("CALL messages")
277309
listener.received_error = true;
278310
listener.error_code = IRpc::RPC_ERROR_NOT_IMPLEMENTED;
279311
listener.error_message = CALLERROR_PAYLOAD;
280-
websocket.notifyDataReceived(EXPECTED_CALL_MESSAGE_1, strlen(EXPECTED_CALL_MESSAGE_1));
312+
websocket.notifyDataReceived(EXPECTED_CALL_MESSAGE_0, strlen(EXPECTED_CALL_MESSAGE_0));
281313
std::this_thread::sleep_for(std::chrono::milliseconds(50u));
282314
CHECK_EQ(listener.action, ACTION);
283315
CHECK_EQ(listener.payload, CALL_PAYLOAD);
284316
CHECK(websocket.sendCalled());
285-
CHECK_EQ(strcmp(reinterpret_cast<const char*>(websocket.sentData()), EXPECTED_CALLERROR_MESSAGE_1), 0);
317+
CHECK_EQ(strcmp(reinterpret_cast<const char*>(websocket.sentData()), EXPECTED_CALLERROR_MESSAGE_0), 0);
286318
}
287319
}

0 commit comments

Comments
 (0)