diff --git a/common/cpp/include/asapo/common/error.h b/common/cpp/include/asapo/common/error.h index 8d045179f7d48c57df2445327b419f5b4b4e6992..510ea0866dc7c3e1659e3b3eb9680d487fe8fee1 100644 --- a/common/cpp/include/asapo/common/error.h +++ b/common/cpp/include/asapo/common/error.h @@ -23,6 +23,7 @@ class ErrorInterface { virtual std::string ExplainInJSON() const noexcept = 0; virtual ErrorInterface* AddContext(std::string key, std::string value) noexcept = 0; virtual ErrorInterface* SetCause(Error cause_err) noexcept = 0; + virtual const Error& GetCause() const noexcept = 0; virtual CustomErrorData* GetCustomData() noexcept = 0; virtual void SetCustomData(std::unique_ptr<CustomErrorData> data) noexcept = 0; virtual ~ErrorInterface() = default; // needed for unique_ptr to delete itself @@ -59,6 +60,7 @@ class ServiceError : public ErrorInterface { void SetCustomData(std::unique_ptr<CustomErrorData> data) noexcept override; ErrorInterface* AddContext(std::string key, std::string value) noexcept override; ErrorInterface* SetCause(Error cause_err) noexcept override; + const Error& GetCause() const noexcept override; std::string Explain() const noexcept override; virtual std::string ExplainPretty(uint8_t shift) const noexcept override; std::string ExplainInJSON() const noexcept override; @@ -68,6 +70,8 @@ class ErrorTemplateInterface { public: virtual Error Generate() const noexcept = 0; virtual Error Generate(std::string error_message) const noexcept = 0; + virtual Error Generate(std::string error_message, Error cause) const noexcept = 0; + virtual Error Generate(Error cause) const noexcept = 0; virtual bool operator==(const Error& rhs) const = 0; virtual bool operator!=(const Error& rhs) const = 0; @@ -103,7 +107,8 @@ class ServiceErrorTemplate : public ErrorTemplateInterface { Error Generate() const noexcept override; Error Generate(std::string error_message) const noexcept override; - + Error Generate(std::string error_message, Error cause) const noexcept override; + Error Generate(Error cause) const noexcept override; inline bool operator==(const Error& rhs) const override { return rhs != nullptr && GetServiceErrorType() == ((ServiceError<ServiceErrorType>*) rhs.get())->GetServiceErrorType(); diff --git a/common/cpp/include/asapo/common/error.tpp b/common/cpp/include/asapo/common/error.tpp index 35b6f8da4053d67d5234032b9a0fdb7d549c58a7..95f6eed6606794d1596731640ce58b58614841c2 100644 --- a/common/cpp/include/asapo/common/error.tpp +++ b/common/cpp/include/asapo/common/error.tpp @@ -107,15 +107,31 @@ std::string ServiceError<ServiceErrorType>::ExplainInJSON() const noexcept { } return err; } +template<typename ServiceErrorType> +const Error &ServiceError<ServiceErrorType>::GetCause() const noexcept { + return cause_err_; +} template<typename ServiceErrorType> Error ServiceErrorTemplate<ServiceErrorType>::Generate() const noexcept { - auto err = new ServiceError<ServiceErrorType>(error_name_, "", error_type_); - return Error(err); + return Generate(""); } + template<typename ServiceErrorType> Error ServiceErrorTemplate<ServiceErrorType>::Generate(std::string error_message) const noexcept { return Error(new ServiceError<ServiceErrorType>(error_name_, std::move(error_message), error_type_)); } +template<typename ServiceErrorType> +Error ServiceErrorTemplate<ServiceErrorType>::Generate(std::string error_message, Error cause) const noexcept { + auto err = Generate(std::move(error_message)); + err->SetCause(std::move(cause)); + return err; +} + +template<typename ServiceErrorType> +Error ServiceErrorTemplate<ServiceErrorType>::Generate(Error cause) const noexcept { + return Generate("",std::move(cause)); +} + } diff --git a/common/cpp/include/asapo/logger/logger.h b/common/cpp/include/asapo/logger/logger.h index 593f210311486882f514305d4593093c5a826a92..e4dc868959f8b5347b95a7a51b19623f166a6e73 100644 --- a/common/cpp/include/asapo/logger/logger.h +++ b/common/cpp/include/asapo/logger/logger.h @@ -22,12 +22,15 @@ class LogMessageWithFields { LogMessageWithFields(std::string key, uint64_t val); LogMessageWithFields(std::string key, double val, int precision); LogMessageWithFields(std::string val); + LogMessageWithFields(const Error& error); LogMessageWithFields(std::string key, std::string val); LogMessageWithFields& Append(std::string key, uint64_t val); LogMessageWithFields& Append(std::string key, double val, int precision); + LogMessageWithFields& Append(const LogMessageWithFields& log_msg); LogMessageWithFields& Append(std::string key, std::string val); std::string LogString() const; private: + inline std::string QuoteIFNeeded(); std::string log_string_; }; diff --git a/common/cpp/src/logger/logger.cpp b/common/cpp/src/logger/logger.cpp index 3d997032bc59b2ca3da6a301ff9c8de060e5445a..546112130bee461f1dc39e9160e0ecddbe865454 100644 --- a/common/cpp/src/logger/logger.cpp +++ b/common/cpp/src/logger/logger.cpp @@ -19,7 +19,6 @@ Logger CreateLogger(std::string name, bool console, bool centralized_log, const return Logger{logger}; } - Logger CreateDefaultLoggerBin(const std::string& name) { return CreateLogger(name, true, false, ""); } @@ -41,14 +40,13 @@ LogLevel StringToLogLevel(const std::string& name, Error* err) { } template<typename ... Args> -std::string string_format( const std::string& format, Args ... args ) { - size_t size = static_cast<size_t>(snprintf( nullptr, 0, format.c_str(), args ... ) + 1); - std::unique_ptr<char[]> buf( new char[ size ] ); - snprintf( buf.get(), size, format.c_str(), args ... ); - return std::string( buf.get(), buf.get() + size - 1 ); +std::string string_format(const std::string& format, Args ... args) { + size_t size = static_cast<size_t>(snprintf(nullptr, 0, format.c_str(), args ...) + 1); + std::unique_ptr<char[]> buf(new char[size]); + snprintf(buf.get(), size, format.c_str(), args ...); + return std::string(buf.get(), buf.get() + size - 1); } - std::string EncloseQuotes(std::string str) { return "\"" + std::move(str) + "\""; } @@ -62,25 +60,31 @@ LogMessageWithFields::LogMessageWithFields(std::string key, double val, int prec } LogMessageWithFields::LogMessageWithFields(std::string val) { - log_string_ = EncloseQuotes("message") + ":" + EncloseQuotes(escape_json(val)); + if (!val.empty()) { + log_string_ = EncloseQuotes("message") + ":" + EncloseQuotes(escape_json(val)); + } } LogMessageWithFields::LogMessageWithFields(std::string key, std::string val) { log_string_ = EncloseQuotes(key) + ":" + EncloseQuotes(escape_json(val)); } +inline std::string LogMessageWithFields::QuoteIFNeeded() { + return log_string_.empty() ? "" : ","; +} + LogMessageWithFields& LogMessageWithFields::Append(std::string key, uint64_t val) { - log_string_ += "," + EncloseQuotes(key) + ":" + std::to_string(val); + log_string_ += QuoteIFNeeded() + EncloseQuotes(key) + ":" + std::to_string(val); return *this; } LogMessageWithFields& LogMessageWithFields::Append(std::string key, double val, int precision) { - log_string_ += "," + EncloseQuotes(key) + ":" + string_format("%." + std::to_string(precision) + "f", val); + log_string_ += QuoteIFNeeded() + EncloseQuotes(key) + ":" + string_format("%." + std::to_string(precision) + "f", val); return *this; } LogMessageWithFields& LogMessageWithFields::Append(std::string key, std::string val) { - log_string_ += "," + EncloseQuotes(key) + ":" + EncloseQuotes(escape_json(val)); + log_string_ += QuoteIFNeeded() + EncloseQuotes(key) + ":" + EncloseQuotes(escape_json(val)); return *this; } @@ -88,4 +92,12 @@ std::string LogMessageWithFields::LogString() const { return log_string_; } +LogMessageWithFields::LogMessageWithFields(const Error& error) { + log_string_ = error->ExplainInJSON(); +} +LogMessageWithFields& LogMessageWithFields::Append(const LogMessageWithFields& log_msg) { + log_string_ += QuoteIFNeeded() + log_msg.LogString(); + return *this; +} + } diff --git a/receiver/src/receiver_error.h b/receiver/src/receiver_error.h index dbfa6c1e2e09092d5539d87b7bdfded11c80d2ae..dff04fc0655265a5de163b14ae28f1e532548998 100644 --- a/receiver/src/receiver_error.h +++ b/receiver/src/receiver_error.h @@ -12,7 +12,8 @@ enum class ReceiverErrorType { kInternalServerError, kReAuthorizationFailure, kWarningDuplicatedRequest, - kUnsupportedClient + kUnsupportedClient, + kProcessingError }; using ReceiverErrorTemplate = ServiceErrorTemplate<ReceiverErrorType>; @@ -33,6 +34,10 @@ auto const kInternalServerError = ReceiverErrorTemplate { "server error", ReceiverErrorType::kInternalServerError }; +auto const kProcessingError = ReceiverErrorTemplate { + "processing error", ReceiverErrorType::kProcessingError +}; + auto const kBadRequest = ReceiverErrorTemplate { "Bad request", ReceiverErrorType::kBadRequest diff --git a/receiver/src/request_handler/requests_dispatcher.cpp b/receiver/src/request_handler/requests_dispatcher.cpp index 827eb713b12c2877102139b0e4aba75645ed2be6..e222465641aaaeb919976ed713d9ef316214cb40 100644 --- a/receiver/src/request_handler/requests_dispatcher.cpp +++ b/receiver/src/request_handler/requests_dispatcher.cpp @@ -58,9 +58,9 @@ Error RequestsDispatcher::HandleRequest(const std::unique_ptr<Request>& request) handle_err = request->Handle(statistics__); if (handle_err) { if (handle_err == ReceiverErrorTemplates::kReAuthorizationFailure) { - log__->Warning(RequestLog("warning processing request: " + handle_err->Explain(), request.get())); + log__->Warning(LogMessageWithFields(handle_err).Append(RequestLog("", request.get()))); } else { - log__->Error(RequestLog("error processing request: " + handle_err->Explain(), request.get())); + log__->Error(LogMessageWithFields(handle_err).Append(RequestLog("", request.get()))); } } return handle_err; @@ -74,35 +74,37 @@ Error RequestsDispatcher::SendResponse(const std::unique_ptr<Request>& request, log__->Debug(log); io__->Send(socket_fd_, &generic_response, sizeof(GenericNetworkResponse), &io_err); if (io_err) { - log__->Error(RequestLog("error sending response: " + io_err->Explain(), request.get())); + auto err = ReceiverErrorTemplates::kProcessingError.Generate("cannot send response",std::move(io_err)); + log__->Error(LogMessageWithFields(err).Append(RequestLog("", request.get()))); + return err; } - return io_err; + return nullptr; } Error RequestsDispatcher::ProcessRequest(const std::unique_ptr<Request>& request) const noexcept { auto handle_err = HandleRequest(request); - auto io_err = SendResponse(request, handle_err); - return handle_err == nullptr ? std::move(io_err) : std::move(handle_err); + auto send_err = SendResponse(request, handle_err); + return handle_err == nullptr ? std::move(send_err) : std::move(handle_err); } std::unique_ptr<Request> RequestsDispatcher::GetNextRequest(Error* err) const noexcept { //TODO: to be overwritten with MessagePack (or similar) GenericRequestHeader generic_request_header; statistics__->StartTimer(StatisticEntity::kNetwork); + Error io_err; io__->Receive(socket_fd_, &generic_request_header, - sizeof(GenericRequestHeader), err); - if (*err) { - if (*err != GeneralErrorTemplates::kEndOfFile) { - log__->Error(LogMessageWithFields("error getting next request: " + (*err)->Explain()). - Append("origin", HostFromUri(producer_uri_))); + sizeof(GenericRequestHeader), &io_err); + if (io_err) { + *err = ReceiverErrorTemplates::kProcessingError.Generate("cannot get next request",std::move(io_err)); + if ((*err)->GetCause() != GeneralErrorTemplates::kEndOfFile) { + log__->Error(LogMessageWithFields(*err).Append("origin", HostFromUri(producer_uri_))); } return nullptr; } statistics__->StopTimer(); auto request = request_factory__->GenerateRequest(generic_request_header, socket_fd_, producer_uri_, err); if (*err) { - log__->Error(LogMessageWithFields("error processing request: " + (*err)->Explain()). - Append("origin", HostFromUri(producer_uri_))); + log__->Error(LogMessageWithFields(*err).Append("origin", HostFromUri(producer_uri_))); } return request; } diff --git a/receiver/unittests/request_handler/test_requests_dispatcher.cpp b/receiver/unittests/request_handler/test_requests_dispatcher.cpp index e0303fded0751f15661ff2ee15cdc87e033b48d5..c7831accb35c3ca48c118e3825fded1a086bdce1 100644 --- a/receiver/unittests/request_handler/test_requests_dispatcher.cpp +++ b/receiver/unittests/request_handler/test_requests_dispatcher.cpp @@ -99,7 +99,7 @@ class RequestsDispatcherTests : public Test { Return(0)) ); if (error) { - EXPECT_CALL(mock_logger, Error(AllOf(HasSubstr("getting next request"), HasSubstr(connected_uri)))); + EXPECT_CALL(mock_logger, Error(AllOf(HasSubstr("cannot get next request"), HasSubstr(connected_uri)))); } } @@ -110,7 +110,7 @@ class RequestsDispatcherTests : public Test { Return(nullptr)) ); if (error) { - EXPECT_CALL(mock_logger, Error(AllOf(HasSubstr("error processing request"), HasSubstr(connected_uri)))); + EXPECT_CALL(mock_logger, Error(AllOf(HasSubstr("error"), HasSubstr(connected_uri)))); } @@ -122,9 +122,9 @@ class RequestsDispatcherTests : public Test { Return(error_mode > 0 ? err.release() : nullptr) ); if (error_mode == 1) { - EXPECT_CALL(mock_logger, Error(AllOf(HasSubstr("error processing request"), HasSubstr(connected_uri)))); + EXPECT_CALL(mock_logger, Error(_)); } else if (error_mode == 2) { - EXPECT_CALL(mock_logger, Warning(AllOf(HasSubstr("warning processing request"), HasSubstr(connected_uri)))); + EXPECT_CALL(mock_logger, Warning(_)); } } void MockSendResponse(GenericNetworkResponse* response, bool error ) { @@ -135,7 +135,7 @@ class RequestsDispatcherTests : public Test { Return(0) )); if (error) { - EXPECT_CALL(mock_logger, Error(AllOf(HasSubstr("error sending response"), HasSubstr(connected_uri)))); + EXPECT_CALL(mock_logger, Error(AllOf(HasSubstr("cannot send response"), HasSubstr(connected_uri)))); } return; @@ -150,7 +150,7 @@ TEST_F(RequestsDispatcherTests, ErrorReceivetNextRequest) { Error err; dispatcher->GetNextRequest(&err); - ASSERT_THAT(err, Eq(asapo::IOErrorTemplates::kUnknownIOError)); + ASSERT_THAT(err, Eq(asapo::ReceiverErrorTemplates::kProcessingError)); } @@ -164,7 +164,7 @@ TEST_F(RequestsDispatcherTests, ClosedConnectionOnReceivetNextRequest) { Error err; dispatcher->GetNextRequest(&err); - ASSERT_THAT(err, Eq(asapo::GeneralErrorTemplates::kEndOfFile)); + ASSERT_THAT(err, Eq(asapo::ReceiverErrorTemplates::kProcessingError)); } @@ -176,7 +176,7 @@ TEST_F(RequestsDispatcherTests, ErrorCreatetNextRequest) { Error err; dispatcher->GetNextRequest(&err); - ASSERT_THAT(err, Eq(asapo::ReceiverErrorTemplates::kInvalidOpCode)); + ASSERT_THAT(err, Eq(asapo::IOErrorTemplates::kUnknownIOError)); } TEST_F(RequestsDispatcherTests, OkCreatetNextRequest) { @@ -206,7 +206,7 @@ TEST_F(RequestsDispatcherTests, OkProcessRequestErrorSend) { auto err = dispatcher->ProcessRequest(request); - ASSERT_THAT(err, Eq(asapo::IOErrorTemplates::kConnectionRefused)); + ASSERT_THAT(err, Eq(asapo::ReceiverErrorTemplates::kProcessingError)); }