From 2b21d0f46490abc5e29730731b7e520b32d08ed4 Mon Sep 17 00:00:00 2001
From: Teyras <teyras@gmail.com>
Date: Thu, 5 Oct 2017 15:10:12 +0200
Subject: [PATCH] when cancelling a job after multiple internal errors, enclose
 the error message in the report for the frontend

---
 src/handlers/broker_handler.cpp | 13 +++++++------
 src/handlers/broker_handler.h   |  4 +++-
 tests/broker.cpp                |  3 ++-
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/handlers/broker_handler.cpp b/src/handlers/broker_handler.cpp
index e63c3b9..e6fd485 100644
--- a/src/handlers/broker_handler.cpp
+++ b/src/handlers/broker_handler.cpp
@@ -263,7 +263,7 @@ void broker_handler::process_worker_done(
 		if (!failed_request->data.is_complete()) {
 			status_notifier.rejected_job(
 				failed_request->data.get_job_id(), "Job failed with '" + message.at(3) + "' and cannot be reassigned");
-		} else if (check_failure_count(failed_request, status_notifier, respond)) {
+		} else if (check_failure_count(failed_request, status_notifier, respond, message.at(3))) {
 			reassign_request(failed_request, respond);
 		} else {
 			auto new_request = queue_->assign_request(worker);
@@ -344,6 +344,7 @@ void broker_handler::process_timer(const message_container &message, handler_int
 	}
 
 	reactor_status_notifier status_notifier(respond, broker_connect::KEY_STATUS_NOTIFIER);
+	const static std::string failure_msg = "Worker timed out and its job cannot be reassigned";
 
 	for (auto worker : to_remove) {
 		logger_->info("Worker {} expired", worker->get_description());
@@ -355,13 +356,12 @@ void broker_handler::process_timer(const message_container &message, handler_int
 
 		for (auto request : *requests) {
 			if (!request->data.is_complete()) {
-				status_notifier.rejected_job(
-					request->data.get_job_id(), "Worker timed out and its job cannot be reassigned");
+				status_notifier.rejected_job(request->data.get_job_id(), failure_msg);
 				notify_monitor(request, "FAILED", respond);
 				continue;
 			}
 
-			if (!check_failure_count(request, status_notifier, respond)) {
+			if (!check_failure_count(request, status_notifier, respond, failure_msg)) {
 				continue;
 			}
 
@@ -413,11 +413,12 @@ void broker_handler::send_request(worker_registry::worker_ptr worker, request_pt
 }
 
 bool broker_handler::check_failure_count(worker::request_ptr request, status_notifier_interface &status_notifier,
-					 response_cb respond)
+					 response_cb respond, const std::string &failure_msg)
 {
 	if (request->failure_count >= config_->get_max_request_failures()) {
 		status_notifier.job_failed(request->data.get_job_id(),
-			"Job was reassigned too many (" + std::to_string(request->failure_count - 1) + ") times");
+			"Job was reassigned too many (" + std::to_string(request->failure_count - 1) + ") times. Last"
+				" failure message was: " + failure_msg);
 		notify_monitor(request, "FAILED", respond);
 		return false;
 	}
diff --git a/src/handlers/broker_handler.h b/src/handlers/broker_handler.h
index f60a624..0d91ec4 100644
--- a/src/handlers/broker_handler.h
+++ b/src/handlers/broker_handler.h
@@ -108,10 +108,12 @@ class broker_handler : public handler_interface
 	 * Check if a request can be reassigned one more time and notify the frontend if not.
 	 * @param request the request to be checked
 	 * @param status_notifier used to notify the frontend when the request doesn't pass the check
+	 * @param respond a callback to notify the monitor in case of failure
+	 * @param failure_msg a message describing the failure of the last request
 	 * @return true if the request can be reassigned, false otherwise
 	 */
 	bool check_failure_count(worker::request_ptr request, status_notifier_interface &status_notifier,
-				 response_cb respond);
+				 response_cb respond, const std::string &failure_msg);
 
 	/**
 	 * Notify the monitor about an error that might not have been reported by a worker
diff --git a/tests/broker.cpp b/tests/broker.cpp
index 5812fb5..6bc6f40 100644
--- a/tests/broker.cpp
+++ b/tests/broker.cpp
@@ -677,7 +677,8 @@ TEST(broker, worker_expiration_cancel_job)
 				"status",
 				"FAILED",
 				"message",
-				"Job was reassigned too many (1) times"}),
+				"Job was reassigned too many (1) times. Last failure message was: Worker timed out "
+					"and its job cannot be reassigned"}),
 				message_container(broker_connect::KEY_MONITOR, broker_connect::MONITOR_IDENTITY, {
 					"job_id",
 					"FAILED"