[20/n] [wicketd] make GetInventoryError::InvalidSpIdentifier carry the id

wicketd/src/http_entrypoints.rs

@@ -9,7 +9,6 @@ use crate::context::CommonConfigContainer;
 use crate::context::RssOrMultirackJoinConfig;
 use crate::http_helpers::ba_lockstep_client;
 use crate::http_helpers::ba_lockstep_error_to_http;
-use crate::http_helpers::inventory_err_to_http;
 use crate::http_helpers::mgs_inventory_or_unavail;
 use crate::http_helpers::start_update;
 use crate::mgs::GetInventoryResponse as GetMgsInventoryResponse;
@@ -389,7 +388,7 @@ impl WicketdApi for WicketdApiImpl {
             }) => Some((inventory, mgs_last_seen)),
             Ok(GetMgsInventoryResponse::Unavailable) => None,
             Err(err) => {
-                return Err(inventory_err_to_http(err));
+                return Err(err.to_http_error());
             }
         };
 

wicketd/src/http_helpers.rs

@@ -24,7 +24,6 @@ use wicketd_commission_types::update::UpdateTargets;
 use crate::ServerContext;
 use crate::helpers::SpIdentifierDisplay;
 use crate::helpers::sps_to_string;
-use crate::mgs::GetInventoryError;
 use crate::mgs::GetInventoryResponse;
 use crate::mgs::MgsHandle;
 use crate::mgs::ShutdownInProgress;
@@ -59,19 +58,6 @@ pub(crate) fn shutdown_to_http(_err: ShutdownInProgress) -> HttpError {
     )
 }
 
-pub(crate) fn inventory_err_to_http(err: GetInventoryError) -> HttpError {
-    match err {
-        GetInventoryError::ShutdownInProgress => {
-            shutdown_to_http(ShutdownInProgress)
-        }
-        GetInventoryError::InvalidSpIdentifier => http_error_with_message(
-            ErrorStatusCode::SERVICE_UNAVAILABLE,
-            None,
-            "Invalid SP identifier in request".to_owned(),
-        ),
-    }
-}
-
 pub(crate) fn ba_lockstep_client(
     ctx: &ServerContext,
 ) -> Result<bootstrap_agent_lockstep_client::Client, HttpError> {

wicketd/src/mgs.rs

@@ -5,6 +5,7 @@
 //! The collection of tasks used for interacting with MGS and maintaining
 //! runtime state.
 
+use dropshot::HttpError;
 use futures::StreamExt;
 use gateway_types::ignition::SpIgnition;
 use slog::{Logger, info, o, warn};
@@ -15,6 +16,10 @@ use tokio::time::{Duration, Instant};
 use tokio_stream::StreamMap;
 use wicket_common::inventory::{MgsV1Inventory, SpIdentifier, SpInventory};
 
+use crate::helpers::SpIdentifierDisplay;
+use crate::http_helpers::http_error_with_message;
+use crate::http_helpers::shutdown_to_http;
+
 use self::inventory::{
     FetchedIgnitionState, FetchedSpData, IgnitionPresence,
     IgnitionStateFetcher, SpStateFetcher,
@@ -68,7 +73,30 @@ pub enum GetInventoryError {
 
     /// The client specified an invalid SP identifier in a `force_refresh`
     /// request.
-    InvalidSpIdentifier,
+    InvalidSpIdentifier {
+        /// The invalid SP identifier.
+        id: SpIdentifier,
+    },
+}
+
+impl GetInventoryError {
+    pub(crate) fn to_http_error(&self) -> HttpError {
+        match self {
+            GetInventoryError::ShutdownInProgress => {
+                shutdown_to_http(ShutdownInProgress)
+            }
+            GetInventoryError::InvalidSpIdentifier { id } => {
+                http_error_with_message(
+                    dropshot::ErrorStatusCode::BAD_REQUEST,
+                    None,
+                    format!(
+                        "invalid SP identifier in force_refresh request: {}",
+                        SpIdentifierDisplay(*id)
+                    ),
+                )
+            }
+        }
+    }
 }
 
 impl MgsHandle {
@@ -80,10 +108,12 @@ impl MgsHandle {
             Err(GetInventoryError::ShutdownInProgress) => {
                 Err(ShutdownInProgress)
             }
-            Err(GetInventoryError::InvalidSpIdentifier) => {
+            Err(GetInventoryError::InvalidSpIdentifier { id }) => {
                 // We pass no SP identifiers to refresh, so it's not possible
                 // for one of them to be invalid.
-                unreachable!("empty SP list cannot contain an invalid ID");
+                unreachable!(
+                    "empty SP list cannot contain an invalid ID, but got {id:?}"
+                );
             }
         }
     }
@@ -321,7 +351,8 @@ impl MgsManager {
         // Trigger immediate refreshes for all SPs listed in `force_refresh`.
         for &id in &force_refresh {
             let Some(handle) = sp_handles.get(&id) else {
-                _ = reply_tx.send(Err(GetInventoryError::InvalidSpIdentifier));
+                _ = reply_tx
+                    .send(Err(GetInventoryError::InvalidSpIdentifier { id }));
                 return;
             };