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

* 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]>

Author: 3 people

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();
         auto part_attributes = participant_->get_attributes();
         const PropertyPolicy log_properties = PropertyPolicyHelper::get_properties_with_prefix(
@@ -396,6 +396,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;
         }

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);
+}
+