From 48ed2181b8c6fbc515a7fa2010cd09025e8e327b Mon Sep 17 00:00:00 2001
From: Nicolas Pope <nwpope@utu.fi>
Date: Thu, 28 Feb 2019 12:50:20 +0200
Subject: [PATCH] Add exception handling and reporting in RPC code
---
net/include/ftl/net/protocol.hpp | 2 +-
net/src/dispatcher.cpp | 20 ++++--
net/src/net.cpp | 2 +-
net/test/protocol_unit.cpp | 105 ++++++++++++++++++++++++++++---
4 files changed, 113 insertions(+), 16 deletions(-)
diff --git a/net/include/ftl/net/protocol.hpp b/net/include/ftl/net/protocol.hpp
index fc4925e29..8679bb4a2 100644
--- a/net/include/ftl/net/protocol.hpp
+++ b/net/include/ftl/net/protocol.hpp
@@ -69,7 +69,7 @@ class Protocol {
static Protocol *find(const std::string &id);
- protected:
+ //protected:
void dispatchRPC(Socket &, const std::string &d);
void dispatchReturn(Socket &, const std::string &d);
void dispatchRaw(uint32_t service, Socket &);
diff --git a/net/src/dispatcher.cpp b/net/src/dispatcher.cpp
index 8bbe3a1b4..32239bd2b 100644
--- a/net/src/dispatcher.cpp
+++ b/net/src/dispatcher.cpp
@@ -55,14 +55,22 @@ void ftl::net::Dispatcher::dispatch_call(Socket &s, const msgpack::object &msg)
auto result = (it_func->second)(args); //->get();
response_t res_obj = std::make_tuple(1,id,msgpack::object(),result->get());
std::stringstream buf;
- msgpack::pack(buf, res_obj);
-
- //std::cout << " RESULT " << result.as<std::string>() << std::endl;
-
+ msgpack::pack(buf, res_obj);
s.send(FTL_PROTOCOL_RPCRETURN, buf.str());
- } catch (int e) {
+ } catch (const std::exception &e) {
+ //throw;
+ //LOG(ERROR) << "Exception when attempting to call RPC (" << e << ")";
+ response_t res_obj = std::make_tuple(1,id,msgpack::object(e.what()),msgpack::object());
+ std::stringstream buf;
+ msgpack::pack(buf, res_obj);
+ s.send(FTL_PROTOCOL_RPCRETURN, buf.str());
+ } catch (int e) {
//throw;
- LOG(ERROR) << "Exception when attempting to call RPC (" << e << ")";
+ //LOG(ERROR) << "Exception when attempting to call RPC (" << e << ")";
+ response_t res_obj = std::make_tuple(1,id,msgpack::object(e),msgpack::object());
+ std::stringstream buf;
+ msgpack::pack(buf, res_obj);
+ s.send(FTL_PROTOCOL_RPCRETURN, buf.str());
}
}
}
diff --git a/net/src/net.cpp b/net/src/net.cpp
index 0d2943f91..1ef7a8d36 100644
--- a/net/src/net.cpp
+++ b/net/src/net.cpp
@@ -11,7 +11,7 @@ using namespace std::chrono;
using ftl::net::Listener;
using ftl::net::Socket;
-static std::vector<shared_ptr<ftl::net::Socket>> sockets;
+ std::vector<shared_ptr<ftl::net::Socket>> sockets;
static std::vector<shared_ptr<ftl::net::Listener>> listeners;
static fd_set sfdread;
static fd_set sfderror;
diff --git a/net/test/protocol_unit.cpp b/net/test/protocol_unit.cpp
index 65fca0b0e..48773176b 100644
--- a/net/test/protocol_unit.cpp
+++ b/net/test/protocol_unit.cpp
@@ -1,18 +1,22 @@
#include "catch.hpp"
#include <ftl/net/protocol.hpp>
+#include <ftl/net/dispatcher.hpp>
using ftl::net::Protocol;
+using ftl::net::Dispatcher;
// --- Mock --------------------------------------------------------------------
#define _FTL_NET_SOCKET_HPP_ // Prevent include
+static std::string last_send;
+
namespace ftl {
namespace net {
class Socket {
public:
std::string getURI() { return "mock://"; }
- int send(int msg, const std::string &d) { return 0; }
+ int send(int msg, const std::string &d) { last_send = d; return 0; }
};
};
};
@@ -29,6 +33,13 @@ class MockProtocol : public Protocol {
// --- Support -----------------------------------------------------------------
+Dispatcher::response_t get_response() {
+ auto unpacked = msgpack::unpack(last_send.data(), last_send.size());
+ Dispatcher::response_t the_result;
+ unpacked.get().convert(the_result);
+ return the_result;
+}
+
// --- Files to test -----------------------------------------------------------
#include "../src/protocol.cpp"
@@ -64,11 +75,11 @@ TEST_CASE("Protocol::bind(int,...)", "[proto]") {
}
}
-TEST_CASE("Protocol::bind(string,...)", "[proto]") {
+SCENARIO("Protocol::bind(string,...)", "[proto]") {
MockProtocol p;
Socket s;
- SECTION("no argument bind with valid dispatch") {
+ GIVEN("no arguments and no return") {
bool called = false;
p.bind("test1", [&]() {
@@ -84,7 +95,7 @@ TEST_CASE("Protocol::bind(string,...)", "[proto]") {
REQUIRE( called );
}
- SECTION("multiple bindings") {
+ GIVEN("multiple no argument bindings") {
bool called1 = false;
bool called2 = false;
@@ -105,7 +116,7 @@ TEST_CASE("Protocol::bind(string,...)", "[proto]") {
REQUIRE( called2 );
}
- SECTION("one argument bind with valid dispatch") {
+ GIVEN("one argument") {
bool called = false;
p.bind("test1", [&](int a) {
@@ -122,7 +133,7 @@ TEST_CASE("Protocol::bind(string,...)", "[proto]") {
REQUIRE( called );
}
- SECTION("two argument bind fake dispatch") {
+ GIVEN("two arguments no return") {
bool called = false;
p.bind("test1", [&](int a, float b) {
@@ -140,7 +151,7 @@ TEST_CASE("Protocol::bind(string,...)", "[proto]") {
REQUIRE( called );
}
- SECTION("non-void bind no arguments") {
+ GIVEN("integer return, no arguments") {
bool called = false;
p.bind("test1", [&]() -> int {
@@ -156,7 +167,85 @@ TEST_CASE("Protocol::bind(string,...)", "[proto]") {
p.mock_dispatchRPC(s, buf.str());
REQUIRE( called );
- // TODO Require that a writev occurred with result value
+ auto [kind,id,err,res] = get_response();
+ REQUIRE( res.as<int>() == 55 );
+ REQUIRE( kind == 1 );
+ REQUIRE( id == 0 );
+ REQUIRE( err.type == 0 );
+ }
+
+ GIVEN("integer return and one argument") {
+ bool called = false;
+
+ p.bind("test1", [&](int a) -> int {
+ called = true;
+ return a+2;
+ });
+
+ auto args_obj = std::make_tuple(12);
+ auto call_obj = std::make_tuple(0,0,"test1",args_obj);
+ std::stringstream buf;
+ msgpack::pack(buf, call_obj);
+
+ p.mock_dispatchRPC(s, buf.str());
+ REQUIRE( called );
+
+ auto [kind,id,err,res] = get_response();
+ REQUIRE( res.as<int>() == 14 );
+ REQUIRE( kind == 1 );
+ REQUIRE( id == 0 );
+ REQUIRE( err.type == 0 );
+ }
+
+ GIVEN("an integer exception in bound function") {
+ bool called = false;
+
+ p.bind("test1", [&](int a) {
+ called = true;
+ throw -1;
+ });
+
+ auto args_obj = std::make_tuple(5);
+ auto call_obj = std::make_tuple(0,0,"test1",args_obj);
+ std::stringstream buf;
+ msgpack::pack(buf, call_obj);
+
+ p.mock_dispatchRPC(s, buf.str());
+ REQUIRE( called );
+
+ auto [kind,id,err,res] = get_response();
+ REQUIRE( res.type == 0 );
+ REQUIRE( kind == 1 );
+ REQUIRE( id == 0 );
+ REQUIRE( err.as<int>() == -1 );
+ }
+
+ GIVEN("a custom std::exception in bound function") {
+ bool called = false;
+
+ struct CustExcept : public std::exception {
+ const char *what() const noexcept { return "My exception"; }
+ };
+
+ p.bind("test1", [&](int a) {
+ called = true;
+ throw CustExcept();
+ });
+
+ auto args_obj = std::make_tuple(5);
+ auto call_obj = std::make_tuple(0,0,"test1",args_obj);
+ std::stringstream buf;
+ msgpack::pack(buf, call_obj);
+
+ p.mock_dispatchRPC(s, buf.str());
+ REQUIRE( called );
+
+ auto [kind,id,err,res] = get_response();
+ REQUIRE( res.type == 0 );
+ REQUIRE( kind == 1 );
+ REQUIRE( id == 0 );
+ REQUIRE( err.type == 5 );
+ REQUIRE( err.as<std::string>() == "My exception" );
}
}
--
GitLab