Improve error handling for Redfish Chassis Drive endpoints

Google-Bug-Id:552840290
PiperOrigin-RevId: 980889855
Change-Id: Ie01fedebcf093881a859f6ed5723f0dbe911eeaa

redfish-core/lib/storage.hpp

diff --git a/redfish-core/lib/storage.hpp b/redfish-core/lib/storage.hpp
index 1a25c2e..67c31e6 100644
--- a/redfish-core/lib/storage.hpp
+++ b/redfish-core/lib/storage.hpp
@@ -30,6 +30,7 @@
 
 #include <algorithm>
 #include <array>
+#include <cerrno>
 #include <charconv>
 #include <chrono>  // NOLINT
 #include <cstdint>
@@ -1137,6 +1138,7 @@
   }
 
   // Iterate over all retrieved ObjectPaths.
+  bool found = false;
   for (const auto& [path, connectionNames] : subtree) {
     sdbusplus::message::object_path objPath(path);
     if (objPath.filename() != driveName) {
@@ -1148,6 +1150,8 @@
       continue;
     }
 
+    found = true;
+
     asyncResp->res.jsonValue["@odata.id"] = crow::utility::urlFromPieces(
         "redfish", "v1", "Chassis", chassisId, "Drives", driveName);
 
@@ -1210,6 +1214,10 @@
     addAllDriveInfo(asyncResp, driveName, connectionNames[0].first, path,
                     connectionNames[0].second, chassisId);
   }
+  if (!found) {
+    messages::resourceNotFound(asyncResp->res, "#Drive.v1_7_0.Drive",
+                               driveName);
+  }
 }
 
 inline void matchAndFillDrive(
@@ -1577,8 +1585,19 @@
                                      chassisId);
           return;
         }
-        dbus_utils::getAssociationEndPoints(chassisPath + "/drive",
-                                            requestContext, cb);
+        dbus_utils::getAssociationEndPoints(
+            chassisPath + "/drive", requestContext,
+            [cb{std::move(cb)}](const boost::system::error_code& ec2,
+                                const dbus::utility::MapperEndPoints& resp) {
+              if (ec2) {
+                if (ec2.value() == EBADR || ec2.value() == ENOENT) {
+                  // No drive association endpoints found on this chassis.
+                  cb(boost::system::error_code{}, {});
+                  return;
+                }
+              }
+              cb(ec2, resp);
+            });
       });
 }
 
@@ -1594,7 +1613,9 @@
       [asyncResp, chassisId, driveName](const boost::system::error_code ec,
                                         const std::vector<std::string>& resp) {
         if (ec) {
-          return;  // no drives = no failures
+          BMCWEB_LOG_ERROR << "DBUS response error " << ec;
+          messages::internalError(asyncResp->res);
+          return;
         }
         matchAndFillDrive(asyncResp, chassisId, driveName, resp);
       });
@@ -1829,9 +1850,9 @@
       [asyncResp, driveId, resetType](const boost::system::error_code ec,
                                       const std::vector<std::string>& drives) {
         if (ec) {
-          BMCWEB_LOG_ERROR << "failed to find drives";
+          BMCWEB_LOG_ERROR << "failed to find drives: " << ec;
           messages::internalError(asyncResp->res);
-          return;  // no drives = no failures
+          return;
         }
         handleChassisDriveReset(asyncResp, driveId, resetType, drives);
       });
@@ -1965,9 +1986,9 @@
       [asyncResp, chassisId, driveId](const boost::system::error_code ec,
                                       const std::vector<std::string>& drives) {
         if (ec) {
-          BMCWEB_LOG_ERROR << "failed to find drives";
+          BMCWEB_LOG_ERROR << "failed to find drives: " << ec;
           messages::internalError(asyncResp->res);
-          return;  // no drives = no failures
+          return;
         }
         handleChassisDriveResetActionInfo(asyncResp, chassisId, driveId,
                                           drives);

test/redfish-core/lib/storage_test.cpp

diff --git a/test/redfish-core/lib/storage_test.cpp b/test/redfish-core/lib/storage_test.cpp
index e9c5f2c..bcd042b 100644
--- a/test/redfish-core/lib/storage_test.cpp
+++ b/test/redfish-core/lib/storage_test.cpp
@@ -2,6 +2,7 @@
 
 #include <systemd/sd-bus.h>
 
+#include <cerrno>
 #include <filesystem>
 #include <memory>
 #include <string>
@@ -898,5 +899,306 @@
             "Google/Metrics/FirstMetric");
   EXPECT_EQ(json["Name"], "FirstMetric");
 }
+
+TEST_F(StorageSnapshotFixture, HandleChassisDriveGetInvalidDriveNotFound) {
+  handleChassisDriveGet(app_, CreateRequest(), share_async_resp_, "platform1",
+                        "invalid_drive");
+  RunIoUntilDone();
+
+  EXPECT_EQ(share_async_resp_->res.result(),
+            boost::beast::http::status::not_found);
+}
+
+TEST_F(StorageSnapshotFixture, HandleChassisDriveGetInvalidChassisNotFound) {
+  handleChassisDriveGet(app_, CreateRequest(), share_async_resp_,
+                        "invalid_chassis", "invalid_drive");
+  RunIoUntilDone();
+
+  EXPECT_EQ(share_async_resp_->res.result(),
+            boost::beast::http::status::not_found);
+}
+
+TEST_F(StorageSnapshotFixture, HandleChassisDriveGetEbadr) {
+  managedStore::KeyType driveAssocKey(
+      managedStore::ManagedType::kManagedProperty,
+      "xyz.openbmc_project.ObjectMapper",
+      sdbusplus::message::object_path(
+          "/xyz/openbmc_project/inventory/system/board/platform1/drive"),
+      "xyz.openbmc_project.Association", "endpoints");
+  ASSERT_TRUE(
+      dynamic_cast<managedStore::MockSerializedManagedObjectStore*>(
+          managedStore::GetManagedObjectStore())
+          ->upsertMockObjectIntoManagedStore(
+              driveAssocKey, CreateErrorValueType(
+                                 dbus::utility::DbusVariantType{},
+                                 boost::system::error_code(
+                                     EBADR, boost::system::generic_category())))
+          .ok());
+
+  handleChassisDriveGet(app_, CreateRequest(), share_async_resp_, "platform1",
+                        "invalid_drive");
+  RunIoUntilDone();
+
+  EXPECT_EQ(share_async_resp_->res.result(),
+            boost::beast::http::status::not_found);
+}
+
+TEST_F(StorageSnapshotFixture, HandleChassisDriveGetEnoent) {
+  managedStore::KeyType driveAssocKey(
+      managedStore::ManagedType::kManagedProperty,
+      "xyz.openbmc_project.ObjectMapper",
+      sdbusplus::message::object_path(
+          "/xyz/openbmc_project/inventory/system/board/platform1/drive"),
+      "xyz.openbmc_project.Association", "endpoints");
+  ASSERT_TRUE(dynamic_cast<managedStore::MockSerializedManagedObjectStore*>(
+                  managedStore::GetManagedObjectStore())
+                  ->upsertMockObjectIntoManagedStore(
+                      driveAssocKey,
+                      CreateErrorValueType(
+                          dbus::utility::DbusVariantType{},
+                          boost::system::error_code(
+                              ENOENT, boost::system::generic_category())))
+                  .ok());
+
+  handleChassisDriveGet(app_, CreateRequest(), share_async_resp_, "platform1",
+                        "invalid_drive");
+  RunIoUntilDone();
+
+  EXPECT_EQ(share_async_resp_->res.result(),
+            boost::beast::http::status::not_found);
+}
+
+TEST_F(StorageSnapshotFixture, HandleChassisDriveGetDbusError) {
+  managedStore::KeyType driveAssocKey(
+      managedStore::ManagedType::kManagedProperty,
+      "xyz.openbmc_project.ObjectMapper",
+      sdbusplus::message::object_path(
+          "/xyz/openbmc_project/inventory/system/board/platform1/drive"),
+      "xyz.openbmc_project.Association", "endpoints");
+  ASSERT_TRUE(dynamic_cast<managedStore::MockSerializedManagedObjectStore*>(
+                  managedStore::GetManagedObjectStore())
+                  ->upsertMockObjectIntoManagedStore(
+                      driveAssocKey,
+                      CreateErrorValueType(dbus::utility::DbusVariantType{},
+                                           boost::system::errc::make_error_code(
+                                               boost::system::errc::io_error)))
+                  .ok());
+
+  handleChassisDriveGet(app_, CreateRequest(), share_async_resp_, "platform1",
+                        "invalid_drive");
+  RunIoUntilDone();
+
+  EXPECT_EQ(share_async_resp_->res.result(),
+            boost::beast::http::status::internal_server_error);
+}
+
+TEST_F(StorageSnapshotFixture, BuildDriveFound) {
+  KeyType storageSubtreeKey(ManagedType::kManagedSubtree,
+                            "/xyz/openbmc_project/inventory", 0,
+                            {"xyz.openbmc_project.Inventory.Item.Storage"});
+  ASSERT_TRUE(dynamic_cast<managedStore::MockSerializedManagedObjectStore*>(
+                  managedStore::GetManagedObjectStore())
+                  ->upsertMockObjectIntoManagedStore(
+                      storageSubtreeKey,
+                      managedStore::MockManagedStoreTest::CreateValueType(
+                          dbus::utility::MapperGetSubTreeResponse{}))
+                  .ok());
+
+  std::string driveName = "drive_0";
+  dbus::utility::MapperGetSubTreeResponse mockSubtree = {
+      {"/xyz/openbmc_project/inventory/drive_0",
+       {{"xyz.openbmc_project.Drive", {}}}},
+  };
+
+  buildDrive(share_async_resp_, "platform1", driveName,
+             boost::system::error_code{}, mockSubtree);
+  RunIoUntilDone();
+
+  EXPECT_EQ(share_async_resp_->res.result(), boost::beast::http::status::ok);
+  EXPECT_EQ(share_async_resp_->res.jsonValue["@odata.id"],
+            "/redfish/v1/Chassis/platform1/Drives/drive_0");
+  EXPECT_EQ(share_async_resp_->res.jsonValue["Name"], "drive_0");
+  EXPECT_EQ(share_async_resp_->res.jsonValue["Id"], "drive_0");
+  EXPECT_EQ(share_async_resp_->res.jsonValue["Status"]["State"], "Enabled");
+}
+
+TEST_F(StorageSnapshotFixture, BuildDriveNotFound) {
+  dbus::utility::MapperGetSubTreeResponse mockSubtree = {
+      {"/xyz/openbmc_project/inventory/other_drive",
+       {{"xyz.openbmc_project.Drive", {}}}},
+  };
+
+  buildDrive(share_async_resp_, "platform1", "drive_0",
+             boost::system::error_code{}, mockSubtree);
+  RunIoUntilDone();
+
+  EXPECT_EQ(share_async_resp_->res.result(),
+            boost::beast::http::status::not_found);
+}
+
+TEST_F(StorageSnapshotFixture, BuildDriveEmptyConnectionNames) {
+  dbus::utility::MapperGetSubTreeResponse mockSubtree = {
+      {"/xyz/openbmc_project/inventory/drive_0", {}},
+  };
+
+  buildDrive(share_async_resp_, "platform1", "drive_0",
+             boost::system::error_code{}, mockSubtree);
+  RunIoUntilDone();
+
+  EXPECT_EQ(share_async_resp_->res.result(),
+            boost::beast::http::status::not_found);
+}
+
+TEST_F(StorageSnapshotFixture, BuildDriveDbusError) {
+  buildDrive(
+      share_async_resp_, "platform1", "drive_0",
+      boost::system::errc::make_error_code(boost::system::errc::io_error), {});
+  RunIoUntilDone();
+
+  EXPECT_EQ(share_async_resp_->res.result(),
+            boost::beast::http::status::internal_server_error);
+}
+
+TEST_F(StorageSnapshotFixture, HandleChassisDriveGetValidDrive) {
+  KeyType storageSubtreeKey(ManagedType::kManagedSubtree,
+                            "/xyz/openbmc_project/inventory", 0,
+                            {"xyz.openbmc_project.Inventory.Item.Storage"});
+  ASSERT_TRUE(dynamic_cast<managedStore::MockSerializedManagedObjectStore*>(
+                  managedStore::GetManagedObjectStore())
+                  ->upsertMockObjectIntoManagedStore(
+                      storageSubtreeKey,
+                      managedStore::MockManagedStoreTest::CreateValueType(
+                          dbus::utility::MapperGetSubTreeResponse{}))
+                  .ok());
+
+  managedStore::KeyType driveAssocKey(
+      managedStore::ManagedType::kManagedProperty,
+      "xyz.openbmc_project.ObjectMapper",
+      sdbusplus::message::object_path(
+          "/xyz/openbmc_project/inventory/system/board/platform1/drive"),
+      "xyz.openbmc_project.Association", "endpoints");
+  dbus::utility::MapperEndPoints driveEndpoints = {
+      "/xyz/openbmc_project/inventory/system/chassis/drive0"};
+  ASSERT_TRUE(dynamic_cast<managedStore::MockSerializedManagedObjectStore*>(
+                  managedStore::GetManagedObjectStore())
+                  ->upsertMockObjectIntoManagedStore(
+                      driveAssocKey,
+                      managedStore::MockManagedStoreTest::CreateValueType(
+                          dbus::utility::DbusVariantType(driveEndpoints)))
+                  .ok());
+
+  managedStore::KeyType driveSubtreeKey(
+      managedStore::ManagedType::kManagedSubtree,
+      "/xyz/openbmc_project/inventory", 0,
+      {"xyz.openbmc_project.Inventory.Item.Drive"});
+  dbus::utility::MapperGetSubTreeResponse mockDriveSubtree = {
+      {"/xyz/openbmc_project/inventory/system/chassis/drive0",
+       {{"xyz.openbmc_project.DriveService", {}}}},
+  };
+  ASSERT_TRUE(dynamic_cast<managedStore::MockSerializedManagedObjectStore*>(
+                  managedStore::GetManagedObjectStore())
+                  ->upsertMockObjectIntoManagedStore(
+                      driveSubtreeKey,
+                      managedStore::MockManagedStoreTest::CreateValueType(
+                          std::move(mockDriveSubtree)))
+                  .ok());
+
+  handleChassisDriveGet(app_, CreateRequest(), share_async_resp_, "platform1",
+                        "drive0");
+  RunIoUntilDone();
+
+  EXPECT_EQ(share_async_resp_->res.result(), boost::beast::http::status::ok);
+  EXPECT_EQ(share_async_resp_->res.jsonValue["@odata.id"],
+            "/redfish/v1/Chassis/platform1/Drives/drive0");
+  EXPECT_EQ(share_async_resp_->res.jsonValue["Name"], "drive0");
+  EXPECT_EQ(share_async_resp_->res.jsonValue["Id"], "drive0");
+}
+
+TEST_F(StorageSnapshotFixture, HandleChassisDriveGetDriveNotInSubtree) {
+  managedStore::KeyType driveAssocKey(
+      managedStore::ManagedType::kManagedProperty,
+      "xyz.openbmc_project.ObjectMapper",
+      sdbusplus::message::object_path(
+          "/xyz/openbmc_project/inventory/system/board/platform1/drive"),
+      "xyz.openbmc_project.Association", "endpoints");
+  dbus::utility::MapperEndPoints driveEndpoints = {
+      "/xyz/openbmc_project/inventory/system/chassis/drive0"};
+  ASSERT_TRUE(dynamic_cast<managedStore::MockSerializedManagedObjectStore*>(
+                  managedStore::GetManagedObjectStore())
+                  ->upsertMockObjectIntoManagedStore(
+                      driveAssocKey,
+                      managedStore::MockManagedStoreTest::CreateValueType(
+                          dbus::utility::DbusVariantType(driveEndpoints)))
+                  .ok());
+
+  managedStore::KeyType driveSubtreeKey(
+      managedStore::ManagedType::kManagedSubtree,
+      "/xyz/openbmc_project/inventory", 0,
+      {"xyz.openbmc_project.Inventory.Item.Drive"});
+  dbus::utility::MapperGetSubTreeResponse mockDriveSubtree = {};
+  ASSERT_TRUE(dynamic_cast<managedStore::MockSerializedManagedObjectStore*>(
+                  managedStore::GetManagedObjectStore())
+                  ->upsertMockObjectIntoManagedStore(
+                      driveSubtreeKey,
+                      managedStore::MockManagedStoreTest::CreateValueType(
+                          std::move(mockDriveSubtree)))
+                  .ok());
+
+  handleChassisDriveGet(app_, CreateRequest(), share_async_resp_, "platform1",
+                        "drive0");
+  RunIoUntilDone();
+
+  EXPECT_EQ(share_async_resp_->res.result(),
+            boost::beast::http::status::not_found);
+}
+
+TEST_F(StorageSnapshotFixture, HandlePostDriveResetActionDbusError) {
+  managedStore::KeyType driveAssocKey(
+      managedStore::ManagedType::kManagedProperty,
+      "xyz.openbmc_project.ObjectMapper",
+      sdbusplus::message::object_path(
+          "/xyz/openbmc_project/inventory/system/board/platform1/drive"),
+      "xyz.openbmc_project.Association", "endpoints");
+  ASSERT_TRUE(dynamic_cast<managedStore::MockSerializedManagedObjectStore*>(
+                  managedStore::GetManagedObjectStore())
+                  ->upsertMockObjectIntoManagedStore(
+                      driveAssocKey,
+                      CreateErrorValueType(dbus::utility::DbusVariantType{},
+                                           boost::system::errc::make_error_code(
+                                               boost::system::errc::io_error)))
+                  .ok());
+
+  handlePostDriveResetAction(app_, CreateRequest(), share_async_resp_,
+                             "platform1", "drive_0");
+  RunIoUntilDone();
+
+  EXPECT_EQ(share_async_resp_->res.result(),
+            boost::beast::http::status::internal_server_error);
+}
+
+TEST_F(StorageSnapshotFixture, HandleGetDriveResetActionInfoDbusError) {
+  managedStore::KeyType driveAssocKey(
+      managedStore::ManagedType::kManagedProperty,
+      "xyz.openbmc_project.ObjectMapper",
+      sdbusplus::message::object_path(
+          "/xyz/openbmc_project/inventory/system/board/platform1/drive"),
+      "xyz.openbmc_project.Association", "endpoints");
+  ASSERT_TRUE(dynamic_cast<managedStore::MockSerializedManagedObjectStore*>(
+                  managedStore::GetManagedObjectStore())
+                  ->upsertMockObjectIntoManagedStore(
+                      driveAssocKey,
+                      CreateErrorValueType(dbus::utility::DbusVariantType{},
+                                           boost::system::errc::make_error_code(
+                                               boost::system::errc::io_error)))
+                  .ok());
+
+  handleGetDriveResetActionInfo(app_, CreateRequest(), share_async_resp_,
+                                "platform1", "drive_0");
+  RunIoUntilDone();
+
+  EXPECT_EQ(share_async_resp_->res.result(),
+            boost::beast::http::status::internal_server_error);
+}
+
 }  // namespace
 }  // namespace redfish