From 185de4e06c5e42949a2d26399002aeca92428e91 Mon Sep 17 00:00:00 2001
From: Karl Oczadly <kocz@live.co.uk>
Date: Fri, 11 Sep 2020 12:58:52 +0100
Subject: [PATCH] Fix error with `work_cancel` RPC request (#2916)

* Add fix for 'work_cancel' RPC request returning error when completed successfully, and implement nano::error_rpc::empty_response error code to catch future errors.

* Add missing write_response() call in test.
---
 nano/core_test/fakes/work_peer.hpp | 16 ++++++++++++----
 nano/lib/errors.cpp                |  2 ++
 nano/lib/errors.hpp                |  1 +
 nano/node/json_handler.cpp         | 10 ++++++++--
 nano/rpc_test/rpc.cpp              |  2 ++
 5 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/nano/core_test/fakes/work_peer.hpp b/nano/core_test/fakes/work_peer.hpp
index 2f54c09d..6fe29e4a 100644
--- a/nano/core_test/fakes/work_peer.hpp
+++ b/nano/core_test/fakes/work_peer.hpp
@@ -33,7 +33,6 @@ enum class work_peer_type
 class work_peer_connection : public std::enable_shared_from_this<work_peer_connection>
 {
 	const std::string generic_error = "Unable to parse JSON";
-	const std::string empty_response = "Empty response";
 
 public:
 	work_peer_connection (asio::io_context & ioc_a, work_peer_type const type_a, nano::work_version const version_a, nano::work_pool & pool_a, std::function<void(bool const)> on_generation_a, std::function<void()> on_cancel_a) :
@@ -126,6 +125,17 @@ private:
 		beast::ostream (response.body ()) << ostream.str ();
 	}
 
+	void handle_cancel ()
+	{
+		on_cancel ();
+		ptree::ptree message_l;
+		message_l.put ("success", "");
+		std::stringstream ostream;
+		ptree::write_json (ostream, message_l);
+		beast::ostream (response.body ()) << ostream.str ();
+		write_response ();
+	}
+
 	void handle_generate (nano::block_hash const & hash_a)
 	{
 		if (type == work_peer_type::good)
@@ -176,9 +186,7 @@ private:
 		}
 		else if (action_text == "work_cancel")
 		{
-			error (empty_response);
-			on_cancel ();
-			write_response ();
+			handle_cancel ();
 		}
 		else
 		{
diff --git a/nano/lib/errors.cpp b/nano/lib/errors.cpp
index 29aac08d..b4bedbf4 100644
--- a/nano/lib/errors.cpp
+++ b/nano/lib/errors.cpp
@@ -127,6 +127,8 @@ std::string nano::error_rpc_messages::message (int ev) const
 	{
 		case nano::error_rpc::generic:
 			return "Unknown error";
+		case nano::error_rpc::empty_response:
+			return "Empty response";
 		case nano::error_rpc::bad_destination:
 			return "Bad destination account";
 		case nano::error_rpc::bad_difficulty_format:
diff --git a/nano/lib/errors.hpp b/nano/lib/errors.hpp
index 6a78034d..4a495430 100644
--- a/nano/lib/errors.hpp
+++ b/nano/lib/errors.hpp
@@ -76,6 +76,7 @@ enum class error_blocks
 enum class error_rpc
 {
 	generic = 1,
+	empty_response,
 	bad_destination,
 	bad_difficulty_format,
 	bad_key,
diff --git a/nano/node/json_handler.cpp b/nano/node/json_handler.cpp
index 0eff29f5..9a286bc8 100644
--- a/nano/node/json_handler.cpp
+++ b/nano/node/json_handler.cpp
@@ -148,10 +148,15 @@ void nano::json_handler::process_request (bool unsafe_a)
 
 void nano::json_handler::response_errors ()
 {
-	if (ec || response_l.empty ())
+	if (!ec && response_l.empty ())
+	{
+		// Return an error code if no response data was given
+		ec = nano::error_rpc::empty_response;
+	}
+	if (ec)
 	{
 		boost::property_tree::ptree response_error;
-		response_error.put ("error", ec ? ec.message () : "Empty response");
+		response_error.put ("error", ec.message ());
 		std::stringstream ostream;
 		boost::property_tree::write_json (ostream, response_error);
 		response (ostream.str ());
@@ -4861,6 +4866,7 @@ void nano::json_handler::work_cancel ()
 	if (!ec)
 	{
 		node.observers.work_cancel.notify (hash);
+		response_l.put ("success", "");
 	}
 	response_errors ();
 }
diff --git a/nano/rpc_test/rpc.cpp b/nano/rpc_test/rpc.cpp
index 51a5e659..bb1a26ca 100644
--- a/nano/rpc_test/rpc.cpp
+++ b/nano/rpc_test/rpc.cpp
@@ -2997,6 +2997,8 @@ TEST (rpc, work_cancel)
 		ASSERT_TIMELY (10s, response1.status != 0);
 		ASSERT_EQ (200, response1.status);
 		ASSERT_NO_ERROR (ec);
+		std::string success (response1.json.get<std::string> ("success"));
+		ASSERT_TRUE (success.empty ());
 	}
 }