fix: create sdbus::Error correctly when errno is 0

Author: sangelovic

include/sdbus-c++/Error.h

@@ -64,7 +64,7 @@ namespace sdbus {
         }
 
         Error(Name name, std::string message)
-            : std::runtime_error("[" + name + "] " + message)
+            : std::runtime_error(!message.empty() ? "[" + name + "] " + message : "[" + name + "]")
             , name_(std::move(name))
             , message_(std::move(message))
         {
@@ -90,7 +90,7 @@ namespace sdbus {
         std::string message_;
     };
 
-    Error createError(int errNo, std::string customMsg);
+    Error createError(int errNo, std::string customMsg = {});
 
     inline const Error::Name SDBUSCPP_ERROR_NAME{"org.sdbuscpp.Error"};
 }

src/Error.cpp

@@ -32,17 +32,24 @@
 
 namespace sdbus
 {
-    sdbus::Error createError(int errNo, std::string customMsg)
+    Error createError(int errNo, std::string customMsg)
     {
         sd_bus_error sdbusError = SD_BUS_ERROR_NULL;
         sd_bus_error_set_errno(&sdbusError, errNo);
         SCOPE_EXIT{ sd_bus_error_free(&sdbusError); };
 
-        Error::Name name(sdbusError.name);
+        Error::Name name(sd_bus_error_is_set(&sdbusError) ? sdbusError.name : "");
         std::string message(std::move(customMsg));
-        message.append(" (");
-        message.append(sdbusError.message);
-        message.append(")");
+        if (!message.empty() && sdbusError.message != nullptr)
+        {
+            message.append(" (");
+            message.append(sdbusError.message);
+            message.append(")");
+        }
+        else if (sdbusError.message != nullptr)
+        {
+            message = sdbusError.message;
+        }
 
         return Error(std::move(name), std::move(message));
     }

tests/unittests/Types_test.cpp

@@ -430,6 +430,7 @@ TEST(AnError, CanBeConstructedFromANameAndAMessage)
     auto error = sdbus::Error(sdbus::Error::Name{"org.sdbuscpp.error"}, "message");
     EXPECT_THAT(error.getName(), Eq<std::string>("org.sdbuscpp.error"));
     EXPECT_THAT(error.getMessage(), Eq<std::string>("message"));
+    EXPECT_TRUE(error.isValid());
 }
 
 TEST(AnError, CanBeConstructedFromANameOnly)
@@ -439,6 +440,45 @@ TEST(AnError, CanBeConstructedFromANameOnly)
     EXPECT_THAT(error1.getName(), Eq<std::string>("org.sdbuscpp.error"));
     EXPECT_THAT(error2.getName(), Eq<std::string>("org.sdbuscpp.error"));
 
-    EXPECT_THAT(error1.getMessage(), Eq<std::string>(""));
-    EXPECT_THAT(error2.getMessage(), Eq<std::string>(""));
+    EXPECT_TRUE(error1.getMessage().empty());
+    EXPECT_TRUE(error2.getMessage().empty());
+
+    EXPECT_TRUE(error1.isValid());
+    EXPECT_TRUE(error2.isValid());
+}
+
+TEST(AnError, IsInvalidWhenConstructedWithAnEmptyName)
+{
+    auto error = sdbus::Error({});
+
+    EXPECT_TRUE(error.getName().empty());
+    EXPECT_TRUE(error.getMessage().empty());
+    EXPECT_FALSE(error.isValid());
+}
+
+TEST(AnErrorFactory, CanCreateAnErrorFromErrno)
+{
+    auto error = sdbus::createError(ENOENT, "custom message");
+
+    EXPECT_THAT(error.getName(), Eq<std::string>("org.freedesktop.DBus.Error.FileNotFound"));
+    EXPECT_THAT(error.getMessage(), Eq<std::string>("custom message (No such file or directory)"));
+    EXPECT_TRUE(error.isValid());
+}
+
+TEST(AnErrorFactory, CreatesGenericErrorWhenErrnoIsUnknown)
+{
+    auto error = sdbus::createError(123456, "custom message");
+
+    EXPECT_THAT(error.getName(), Eq<std::string>("org.freedesktop.DBus.Error.Failed"));
+    EXPECT_THAT(error.getMessage(), Eq<std::string>("custom message (Unknown error 123456)"));
+    EXPECT_TRUE(error.isValid());
+}
+
+TEST(AnErrorFactory, CreatesEmptyInvalidErrorWhenErrnoIsZero)
+{
+    auto error = sdbus::createError(0, "custom message");
+
+    EXPECT_TRUE(error.getName().empty());
+    EXPECT_THAT(error.getMessage(), Eq<std::string>("custom message"));
+    EXPECT_FALSE(error.isValid());
 }