iox-eclipse-iceoryx#1032 Improve output our error messages

elBoberido · Feb 4, 2024 · 49eb12d · 49eb12d
1 parent bf2d793
commit 49eb12d
Show file tree

Hide file tree

Showing 8 changed files with 55 additions and 13 deletions.
diff --git a/iceoryx_binding_c/include/iceoryx_binding_c/internal/binding_c_error_reporting.hpp b/iceoryx_binding_c/include/iceoryx_binding_c/internal/binding_c_error_reporting.hpp
@@ -31,6 +31,7 @@
 
 // additional includes
 #include "iox/error_reporting/types.hpp"
+#include "iox/log/logstream.hpp"
 
 namespace iox
 {
@@ -62,6 +63,12 @@ enum class CBindingError : iox::er::ErrorCode::type
 
 const char* asStringLiteral(const CBindingError error) noexcept;
 
+inline log::LogStream& operator<<(log::LogStream& stream, CBindingError value) noexcept
+{
+    stream << asStringLiteral(value);
+    return stream;
+}
+
 class CBindingErrorType
 {
   public:

diff --git a/iceoryx_hoofs/reporting/include/iox/detail/hoofs_error_reporting.hpp b/iceoryx_hoofs/reporting/include/iox/detail/hoofs_error_reporting.hpp
@@ -30,6 +30,7 @@
 
 // additional includes
 #include "iox/error_reporting/types.hpp"
+#include "iox/log/logstream.hpp"
 
 namespace iox
 {
@@ -51,6 +52,12 @@ enum class HoofsError : iox::er::ErrorCode::type
 
 const char* asStringLiteral(const HoofsError error) noexcept;
 
+inline log::LogStream& operator<<(log::LogStream& stream, HoofsError value) noexcept
+{
+    stream << asStringLiteral(value);
+    return stream;
+}
+
 class HoofsErrorType
 {
   public:

diff --git a/iceoryx_hoofs/reporting/include/iox/error_reporting/custom/default/error_reporting_impl.hpp b/iceoryx_hoofs/reporting/include/iox/error_reporting/custom/default/error_reporting_impl.hpp
@@ -54,7 +54,7 @@ namespace er
 // Custom panic with location
 [[noreturn]] inline void panic(const SourceLocation& location)
 {
-    IOX_ERROR_INTERNAL_LOG_PANIC(location, "Panic");
+    IOX_ERROR_INTERNAL_LOG_PANIC(location, "[PANIC]");
     panic();
 }
 
@@ -64,7 +64,7 @@ namespace er
 template <class Message>
 [[noreturn]] inline void panic(const SourceLocation& location, Message&& msg)
 {
-    IOX_ERROR_INTERNAL_LOG_PANIC(location, "Panic " << msg);
+    IOX_ERROR_INTERNAL_LOG_PANIC(location, "[PANIC] " << msg);
     panic();
 }
 
@@ -80,8 +80,8 @@ inline void report(const SourceLocation& location, Kind, const Error& error)
     auto errorName = toErrorName(error);
 
     IOX_ERROR_INTERNAL_LOG(location,
-                           "'" << errorName << "' (code " << code.value << ") in module '" << moduleName << "' (id "
-                               << module.value << ")");
+                           "[" << errorName << " (code = " << code.value << ")] in module [" << moduleName
+                               << " (id = " << module.value << ")]");
     auto& h = ErrorHandler::get();
     h.onReportError(ErrorDescriptor(location, code, module));
 }
@@ -100,8 +100,8 @@ inline void report(const SourceLocation& location, iox::er::FatalKind kind, cons
     auto errorName = toErrorName(error);
 
     IOX_ERROR_INTERNAL_LOG_FATAL(location,
-                                 "'" << kind.name << "' '" << errorName << "' (code " << code.value << ") in module '"
-                                     << moduleName << "' (id " << module.value << ")");
+                                 "[" << kind.name << "] [" << errorName << " (code = " << code.value << ")] in module ["
+                                     << moduleName << " (id = " << module.value << ")]");
     auto& h = ErrorHandler::get();
     h.onReportError(ErrorDescriptor(location, code, module));
 }
@@ -119,11 +119,11 @@ inline void report(const SourceLocation& location, Kind kind, const Error& error
     auto module = toModule(error);
     if constexpr (std::is_same<NoMessage, Message>::value)
     {
-        IOX_ERROR_INTERNAL_LOG_FATAL(location, kind.name);
+        IOX_ERROR_INTERNAL_LOG_FATAL(location, "[" << kind.name << "]");
     }
     else
     {
-        IOX_ERROR_INTERNAL_LOG_FATAL(location, kind.name << " " << std::forward<Message>(msg));
+        IOX_ERROR_INTERNAL_LOG_FATAL(location, "[" << kind.name << "] " << std::forward<Message>(msg));
     }
     auto& h = ErrorHandler::get();
     h.onReportViolation(ErrorDescriptor(location, code, module));

diff --git a/iceoryx_hoofs/reporting/include/iox/error_reporting/error_kind.hpp b/iceoryx_hoofs/reporting/include/iox/error_reporting/error_kind.hpp
@@ -44,7 +44,7 @@ struct AssertViolationKind
 
 struct EnforceViolationKind
 {
-    static constexpr char const* name = "EnforceViolation";
+    static constexpr char const* name = "Enforce Violation";
 };
 
 template <class T>

diff --git a/iceoryx_hoofs/reporting/include/iox/error_reporting/error_logging.hpp b/iceoryx_hoofs/reporting/include/iox/error_reporting/error_logging.hpp
@@ -31,7 +31,7 @@
                      location.line,                                                                                    \
                      location.function,                                                                                \
                      iox::log::LogLevel::ERROR,                                                                        \
-                     location.file << " line " << location.line << ": " << msg_stream)
+                     location.file << ":" << location.line << " " << msg_stream)
 
 /// @brief Log the location of a fatal error.
 /// @param location the location of the error
@@ -41,7 +41,7 @@
                      location.line,                                                                                    \
                      location.function,                                                                                \
                      iox::log::LogLevel::FATAL,                                                                        \
-                     location.file << " line " << location.line << ": " << msg_stream)
+                     location.file << ":" << location.line << " " << msg_stream)
 
 /// @brief Log a panic invocation.
 /// @param location the location of the panic invocation.

diff --git a/iceoryx_hoofs/testing/include/iceoryx_hoofs/testing/fatal_failure.hpp b/iceoryx_hoofs/testing/include/iceoryx_hoofs/testing/fatal_failure.hpp
@@ -17,9 +17,12 @@
 #ifndef IOX_HOOFS_TESTING_FATAL_FAILURE_HPP
 #define IOX_HOOFS_TESTING_FATAL_FAILURE_HPP
 
+#include "iox/error_reporting/error_kind.hpp"
+#include "iox/error_reporting/types.hpp"
 #include "iox/function_ref.hpp"
-#include "iox/optional.hpp"
+#include "iox/logging.hpp"
 
+#include "iceoryx_hoofs/testing/error_reporting/testing_support.hpp"
 #include "test.hpp"
 
 #include <atomic>

diff --git a/iceoryx_hoofs/testing/include/iceoryx_hoofs/testing/fatal_failure.inl b/iceoryx_hoofs/testing/include/iceoryx_hoofs/testing/fatal_failure.inl
@@ -18,36 +18,54 @@
 #ifndef IOX_HOOFS_TESTING_FATAL_FAILURE_INL
 #define IOX_HOOFS_TESTING_FATAL_FAILURE_INL
 
-#include "iceoryx_hoofs/testing/error_reporting/testing_support.hpp"
 #include "iceoryx_hoofs/testing/fatal_failure.hpp"
 
 namespace iox
 {
 namespace testing
 {
 template <typename ErrorType>
+// NOLINTJUSTIFICATION The complexity comes from the expanded macros; without the expansions the functions is quite readable
+// NOLINTNEXTLINE(readability-function-size, readability-function-cognitive-complexity)
 inline bool IOX_EXPECT_FATAL_FAILURE(const function_ref<void()> testFunction,
                                      const ErrorType expectedError [[maybe_unused]])
 {
     iox::testing::ErrorHandler::instance().reset();
     runInTestThread([&] { testFunction(); });
+    IOX_TESTING_EXPECT_PANIC();
 
     auto hasExpectedError{false};
     if constexpr (std::is_same_v<ErrorType, iox::er::FatalKind>)
     {
         hasExpectedError = iox::testing::hasPanicked();
+        if (!hasExpectedError)
+        {
+            IOX_LOG(ERROR, "Expected '" << iox::er::FatalKind::name << "' but it did not happen!");
+        }
     }
     else if constexpr (std::is_same_v<ErrorType, iox::er::EnforceViolationKind>)
     {
         hasExpectedError = iox::testing::hasEnforceViolation() && iox::testing::hasPanicked();
+        if (!hasExpectedError)
+        {
+            IOX_LOG(ERROR, "Expected '" << iox::er::EnforceViolationKind::name << "' but it did not happen!");
+        }
     }
     else if constexpr (std::is_same_v<ErrorType, iox::er::AssertViolationKind>)
     {
         hasExpectedError = iox::testing::hasAssertViolation() && iox::testing::hasPanicked();
+        if (!hasExpectedError)
+        {
+            IOX_LOG(ERROR, "Expected '" << iox::er::AssertViolationKind::name << "' but it did not happen!");
+        }
     }
     else
     {
         hasExpectedError = iox::testing::hasError(expectedError) && iox::testing::hasPanicked();
+        if (!hasExpectedError)
+        {
+            IOX_LOG(ERROR, "Expected an '" << expectedError << "' error but it did not happen!");
+        }
     }
 
     EXPECT_TRUE(hasExpectedError);

diff --git a/iceoryx_posh/include/iceoryx_posh/internal/posh_error_reporting.hpp b/iceoryx_posh/include/iceoryx_posh/internal/posh_error_reporting.hpp
@@ -31,6 +31,7 @@
 
 // additional includes
 #include "iox/error_reporting/types.hpp"
+#include "iox/log/logstream.hpp"
 
 namespace iox
 {
@@ -182,6 +183,12 @@ enum class PoshError : iox::er::ErrorCode::type
 
 const char* asStringLiteral(const PoshError error) noexcept;
 
+inline log::LogStream& operator<<(log::LogStream& stream, PoshError value) noexcept
+{
+    stream << asStringLiteral(value);
+    return stream;
+}
+
 class PoshErrorType
 {
   public: