Log any errors before cancel_init() (#5530) (#5547)

* Log any errors before cancel_init() (#5530) * Log any errors before cancel_init() Signed-off-by: Santti4go <[email protected]> * Code style Signed-off-by: Santti4go <[email protected]> * Apply suggestion Signed-off-by: Santti4go <[email protected]> --------- Signed-off-by: Santti4go <[email protected]> (cherry picked from commit 165d64e) * Add test for security initialization error (#5550) * Regression tests Signed-off-by: Eugenio Collado <[email protected]> * Update log macro Signed-off-by: Eugenio Collado <[email protected]> * Uncrustify Signed-off-by: Eugenio Collado <[email protected]> * Fix CI log flush Signed-off-by: Eugenio Collado <[email protected]> --------- Signed-off-by: Eugenio Collado <[email protected]> --------- Signed-off-by: Eugenio Collado <[email protected]> Co-authored-by: Santiago <[email protected]> Co-authored-by: EugenioCollado <[email protected]>
eProsima · Jan 20, 2025 · ed616cb · ed616cb
1 parent e589daa
commit ed616cb
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 1 deletion.
diff --git a/src/cpp/rtps/security/SecurityManager.cpp b/src/cpp/rtps/security/SecurityManager.cpp
@@ -117,9 +117,9 @@ bool SecurityManager::init(
         ParticipantSecurityAttributes& attributes,
         const PropertyPolicy& participant_properties)
 {
+    SecurityException exception;
     try
     {
-        SecurityException exception;
         domain_id_ = participant_->get_domain_id();
         const PropertyPolicy log_properties = PropertyPolicyHelper::get_properties_with_prefix(
             participant_->getRTPSParticipantAttributes().properties,
@@ -385,6 +385,13 @@ bool SecurityManager::init(
     {
         if (!e)
         {
+            // Unexpected code path. Let's log any errors
+            EPROSIMA_LOG_ERROR(SECURITY, "Error while configuring security plugin.");
+            if (0 != strlen(exception.what()))
+            {
+                EPROSIMA_LOG_ERROR(SECURITY, exception.what());
+            }
+
             cancel_init();
             return false;
         }

diff --git a/test/unittest/rtps/security/SecurityInitializationTests.cpp b/test/unittest/rtps/security/SecurityInitializationTests.cpp
@@ -14,6 +14,8 @@
 
 #include "SecurityTests.hpp"
 
+#include "../../logging/mock/MockConsumer.h"
+
 const char* const MockIdentity::class_id_ = "MockIdentityHandle";
 const char* const MockHandshake::class_id_ = "MockHandshakeHandle";
 const char* const SharedSecret::class_id_ = "SharedSecretHandle";
@@ -208,3 +210,43 @@ TEST_F(SecurityTest, initialization_ok)
 
 }
 
+/* Regression test for Redmine 22545.
+ *
+ * Triggering a throw false in SecurityManager::init() should be logged properly as
+ * the error: "Error while configuring security plugin.".
+ */
+TEST_F(SecurityTest, initialization_logging_error)
+{
+    DefaultValue<const GUID_t&>::Set(guid);
+    DefaultValue<const ParticipantSecurityAttributes&>::Set(security_attributes_);
+
+    EXPECT_CALL(*auth_plugin_, validate_local_identity(_, _, _, _, _, _)).Times(1).
+            WillOnce(DoAll(SetArgPointee<0>(&local_identity_handle_), Return(ValidationResult_t::VALIDATION_OK)));
+    EXPECT_CALL(crypto_plugin_->cryptokeyfactory_,
+            register_local_participant(Ref(local_identity_handle_), _, _, _, _)).Times(1).
+            WillOnce(Return(nullptr));
+
+    eprosima::fastdds::dds::MockConsumer* mockConsumer = new eprosima::fastdds::dds::MockConsumer();
+    eprosima::fastdds::dds::Log::RegisterConsumer(std::unique_ptr<eprosima::fastdds::dds::LogConsumer>(mockConsumer));
+    eprosima::fastdds::dds::Log::SetVerbosity(eprosima::fastdds::dds::Log::Error);
+
+    security_activated_ = manager_.init(security_attributes_, participant_properties_);
+
+    // Check that the error message was logged.
+    // First flush the log to make sure the message is there.
+    eprosima::fastdds::dds::Log::Flush();
+
+    auto log_entries = mockConsumer->ConsumedEntries();
+    ASSERT_GE(log_entries.size(), 1);
+    bool found = false;
+    for (auto entry : log_entries)
+    {
+        if (entry.message.find("Error while configuring security plugin.") != std::string::npos)
+        {
+            found = true;
+            break;
+        }
+    }
+    ASSERT_TRUE(found);
+}
+