refactor: Warn on clippy::error_impl_error

Enables warning on `clippy::error_impl_error` and fixes the produced
warnings.

Signed-off-by: joaoleal <[email protected]>
Co-authored-by: Jonathan Wollett-Light <[email protected]>

Author: 2 people

Cargo.toml

@@ -16,6 +16,7 @@ cast_sign_loss = "warn"
 exit = "warn"
 tests_outside_test_module = "warn"
 assertions_on_result_states = "warn"
+error_impl_error = "warn"
 
 [profile.dev]
 panic = "abort"

src/firecracker/src/api_server/parsed_request.rs

@@ -58,7 +58,7 @@ pub(crate) struct ParsedRequest {
 }
 
 impl TryFrom<&Request> for ParsedRequest {
-    type Error = Error;
+    type Error = RequestError;
     fn try_from(request: &Request) -> Result<Self, Self::Error> {
         let request_uri = request.uri().get_abs_path().to_string();
         let description = describe(
@@ -109,9 +109,10 @@ impl TryFrom<&Request> for ParsedRequest {
             }
             (Method::Patch, "vm", Some(body)) => parse_patch_vm_state(body),
             (Method::Patch, _, None) => method_to_error(Method::Patch),
-            (method, unknown_uri, _) => {
-                Err(Error::InvalidPathMethod(unknown_uri.to_string(), method))
-            }
+            (method, unknown_uri, _) => Err(RequestError::InvalidPathMethod(
+                unknown_uri.to_string(),
+                method,
+            )),
         }
     }
 }
@@ -246,25 +247,25 @@ fn describe_with_body(method: Method, path: &str, payload_value: &Body) -> Strin
 }
 
 /// Generates a `GenericError` for each request method.
-pub(crate) fn method_to_error(method: Method) -> Result<ParsedRequest, Error> {
+pub(crate) fn method_to_error(method: Method) -> Result<ParsedRequest, RequestError> {
     match method {
-        Method::Get => Err(Error::Generic(
+        Method::Get => Err(RequestError::Generic(
             StatusCode::BadRequest,
             "GET request cannot have a body.".to_string(),
         )),
-        Method::Put => Err(Error::Generic(
+        Method::Put => Err(RequestError::Generic(
             StatusCode::BadRequest,
             "Empty PUT request.".to_string(),
         )),
-        Method::Patch => Err(Error::Generic(
+        Method::Patch => Err(RequestError::Generic(
             StatusCode::BadRequest,
             "Empty PATCH request.".to_string(),
         )),
     }
 }
 
 #[derive(Debug, thiserror::Error)]
-pub(crate) enum Error {
+pub(crate) enum RequestError {
     // The resource ID is empty.
     #[error("The ID cannot be empty.")]
     EmptyID,
@@ -283,30 +284,30 @@ pub(crate) enum Error {
 }
 
 // It's convenient to turn errors into HTTP responses directly.
-impl From<Error> for Response {
-    fn from(err: Error) -> Self {
+impl From<RequestError> for Response {
+    fn from(err: RequestError) -> Self {
         let msg = ApiServer::json_fault_message(format!("{}", err));
         match err {
-            Error::Generic(status, _) => ApiServer::json_response(status, msg),
-            Error::EmptyID
-            | Error::InvalidID
-            | Error::InvalidPathMethod(_, _)
-            | Error::SerdeJson(_) => ApiServer::json_response(StatusCode::BadRequest, msg),
+            RequestError::Generic(status, _) => ApiServer::json_response(status, msg),
+            RequestError::EmptyID
+            | RequestError::InvalidID
+            | RequestError::InvalidPathMethod(_, _)
+            | RequestError::SerdeJson(_) => ApiServer::json_response(StatusCode::BadRequest, msg),
         }
     }
 }
 
 // This function is supposed to do id validation for requests.
-pub(crate) fn checked_id(id: &str) -> Result<&str, Error> {
+pub(crate) fn checked_id(id: &str) -> Result<&str, RequestError> {
     // todo: are there any checks we want to do on id's?
     // not allow them to be empty strings maybe?
     // check: ensure string is not empty
     if id.is_empty() {
-        return Err(Error::EmptyID);
+        return Err(RequestError::EmptyID);
     }
     // check: ensure string is alphanumeric
     if !id.chars().all(|c| c == '_' || c.is_alphanumeric()) {
-        return Err(Error::InvalidID);
+        return Err(RequestError::InvalidID);
     }
     Ok(id)
 }
@@ -435,7 +436,7 @@ pub mod tests {
         let parsed_request = ParsedRequest::try_from(&req);
         assert!(matches!(
             &parsed_request,
-            Err(Error::Generic(StatusCode::BadRequest, s)) if s == "GET request cannot have a body.",
+            Err(RequestError::Generic(StatusCode::BadRequest, s)) if s == "GET request cannot have a body.",
         ));
     }
 
@@ -451,7 +452,7 @@ pub mod tests {
         let parsed_request = ParsedRequest::try_from(&req);
         assert!(matches!(
             &parsed_request,
-            Err(Error::Generic(StatusCode::BadRequest, s)) if s == "Empty PUT request.",
+            Err(RequestError::Generic(StatusCode::BadRequest, s)) if s == "Empty PUT request.",
         ));
     }
 
@@ -467,7 +468,7 @@ pub mod tests {
         let parsed_request = ParsedRequest::try_from(&req);
         assert!(matches!(
             &parsed_request,
-            Err(Error::Generic(StatusCode::BadRequest, s)) if s == "Empty PATCH request.",
+            Err(RequestError::Generic(StatusCode::BadRequest, s)) if s == "Empty PATCH request.",
         ));
     }
 
@@ -476,23 +477,23 @@ pub mod tests {
         // Generic error.
         let mut buf = Cursor::new(vec![0]);
         let response: Response =
-            Error::Generic(StatusCode::BadRequest, "message".to_string()).into();
+            RequestError::Generic(StatusCode::BadRequest, "message".to_string()).into();
         response.write_all(&mut buf).unwrap();
         let body = ApiServer::json_fault_message("message");
         let expected_response = http_response(&body, 400);
         assert_eq!(buf.into_inner(), expected_response.as_bytes());
 
         // Empty ID error.
         let mut buf = Cursor::new(vec![0]);
-        let response: Response = Error::EmptyID.into();
+        let response: Response = RequestError::EmptyID.into();
         response.write_all(&mut buf).unwrap();
         let body = ApiServer::json_fault_message("The ID cannot be empty.");
         let expected_response = http_response(&body, 400);
         assert_eq!(buf.into_inner(), expected_response.as_bytes());
 
         // Invalid ID error.
         let mut buf = Cursor::new(vec![0]);
-        let response: Response = Error::InvalidID.into();
+        let response: Response = RequestError::InvalidID.into();
         response.write_all(&mut buf).unwrap();
         let body = ApiServer::json_fault_message(
             "API Resource IDs can only contain alphanumeric characters and underscores.",
@@ -502,7 +503,8 @@ pub mod tests {
 
         // Invalid path or method error.
         let mut buf = Cursor::new(vec![0]);
-        let response: Response = Error::InvalidPathMethod("path".to_string(), Method::Get).into();
+        let response: Response =
+            RequestError::InvalidPathMethod("path".to_string(), Method::Get).into();
         response.write_all(&mut buf).unwrap();
         let body = ApiServer::json_fault_message(format!(
             "Invalid request method and/or path: {} {}.",
@@ -515,7 +517,7 @@ pub mod tests {
         // Serde error.
         let mut buf = Cursor::new(vec![0]);
         let serde_error = serde_json::Value::from_str("").unwrap_err();
-        let response: Response = Error::SerdeJson(serde_error).into();
+        let response: Response = RequestError::SerdeJson(serde_error).into();
         response.write_all(&mut buf).unwrap();
         let body = ApiServer::json_fault_message(
             "An error occurred when deserializing the json body of a request: EOF while parsing a \

src/firecracker/src/api_server/request/actions.rs

@@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize};
 use vmm::logger::{IncMetric, METRICS};
 use vmm::rpc_interface::VmmAction;
 
-use super::super::parsed_request::{Error, ParsedRequest};
+use super::super::parsed_request::{ParsedRequest, RequestError};
 use super::Body;
 #[cfg(target_arch = "aarch64")]
 use super::StatusCode;
@@ -28,7 +28,7 @@ struct ActionBody {
     action_type: ActionType,
 }
 
-pub(crate) fn parse_put_actions(body: &Body) -> Result<ParsedRequest, Error> {
+pub(crate) fn parse_put_actions(body: &Body) -> Result<ParsedRequest, RequestError> {
     METRICS.put_api_requests.actions_count.inc();
     let action_body = serde_json::from_slice::<ActionBody>(body.raw()).map_err(|err| {
         METRICS.put_api_requests.actions_fails.inc();
@@ -41,7 +41,7 @@ pub(crate) fn parse_put_actions(body: &Body) -> Result<ParsedRequest, Error> {
         ActionType::SendCtrlAltDel => {
             // SendCtrlAltDel not supported on aarch64.
             #[cfg(target_arch = "aarch64")]
-            return Err(Error::Generic(
+            return Err(RequestError::Generic(
                 StatusCode::BadRequest,
                 "SendCtrlAltDel does not supported on aarch64.".to_string(),
             ));

src/firecracker/src/api_server/request/balloon.rs

@@ -7,14 +7,16 @@ use vmm::vmm_config::balloon::{
     BalloonDeviceConfig, BalloonUpdateConfig, BalloonUpdateStatsConfig,
 };
 
-use super::super::parsed_request::{Error, ParsedRequest};
+use super::super::parsed_request::{ParsedRequest, RequestError};
 use super::Body;
 
-pub(crate) fn parse_get_balloon(path_second_token: Option<&str>) -> Result<ParsedRequest, Error> {
+pub(crate) fn parse_get_balloon(
+    path_second_token: Option<&str>,
+) -> Result<ParsedRequest, RequestError> {
     match path_second_token {
         Some(stats_path) => match stats_path {
             "statistics" => Ok(ParsedRequest::new_sync(VmmAction::GetBalloonStats)),
-            _ => Err(Error::Generic(
+            _ => Err(RequestError::Generic(
                 StatusCode::BadRequest,
                 format!("Unrecognized GET request path `{}`.", stats_path),
             )),
@@ -23,7 +25,7 @@ pub(crate) fn parse_get_balloon(path_second_token: Option<&str>) -> Result<Parse
     }
 }
 
-pub(crate) fn parse_put_balloon(body: &Body) -> Result<ParsedRequest, Error> {
+pub(crate) fn parse_put_balloon(body: &Body) -> Result<ParsedRequest, RequestError> {
     Ok(ParsedRequest::new_sync(VmmAction::SetBalloonDevice(
         serde_json::from_slice::<BalloonDeviceConfig>(body.raw())?,
     )))
@@ -32,13 +34,13 @@ pub(crate) fn parse_put_balloon(body: &Body) -> Result<ParsedRequest, Error> {
 pub(crate) fn parse_patch_balloon(
     body: &Body,
     path_second_token: Option<&str>,
-) -> Result<ParsedRequest, Error> {
+) -> Result<ParsedRequest, RequestError> {
     match path_second_token {
         Some(config_path) => match config_path {
             "statistics" => Ok(ParsedRequest::new_sync(VmmAction::UpdateBalloonStatistics(
                 serde_json::from_slice::<BalloonUpdateStatsConfig>(body.raw())?,
             ))),
-            _ => Err(Error::Generic(
+            _ => Err(RequestError::Generic(
                 StatusCode::BadRequest,
                 format!("Unrecognized PATCH request path `{}`.", config_path),
             )),

src/firecracker/src/api_server/request/boot_source.rs

@@ -5,10 +5,10 @@ use vmm::logger::{IncMetric, METRICS};
 use vmm::rpc_interface::VmmAction;
 use vmm::vmm_config::boot_source::BootSourceConfig;
 
-use super::super::parsed_request::{Error, ParsedRequest};
+use super::super::parsed_request::{ParsedRequest, RequestError};
 use super::Body;
 
-pub(crate) fn parse_put_boot_source(body: &Body) -> Result<ParsedRequest, Error> {
+pub(crate) fn parse_put_boot_source(body: &Body) -> Result<ParsedRequest, RequestError> {
     METRICS.put_api_requests.boot_source_count.inc();
     Ok(ParsedRequest::new_sync(VmmAction::ConfigureBootSource(
         serde_json::from_slice::<BootSourceConfig>(body.raw()).map_err(|err| {

src/firecracker/src/api_server/request/cpu_configuration.rs

@@ -5,17 +5,17 @@ use vmm::cpu_config::templates::CustomCpuTemplate;
 use vmm::logger::{IncMetric, METRICS};
 use vmm::rpc_interface::VmmAction;
 
-use super::super::parsed_request::{Error, ParsedRequest};
+use super::super::parsed_request::{ParsedRequest, RequestError};
 use super::Body;
 
-pub(crate) fn parse_put_cpu_config(body: &Body) -> Result<ParsedRequest, Error> {
+pub(crate) fn parse_put_cpu_config(body: &Body) -> Result<ParsedRequest, RequestError> {
     METRICS.put_api_requests.cpu_cfg_count.inc();
 
     // Convert the API request into a a deserialized/binary format
     Ok(ParsedRequest::new_sync(VmmAction::PutCpuConfiguration(
         CustomCpuTemplate::try_from(body.raw()).map_err(|err| {
             METRICS.put_api_requests.cpu_cfg_fails.inc();
-            Error::SerdeJson(err)
+            RequestError::SerdeJson(err)
         })?,
     )))
 }
@@ -83,7 +83,7 @@ mod tests {
             expected_err_count
         );
         assert!(
-            matches!(invalid_put_result, Err(Error::SerdeJson(_))),
+            matches!(invalid_put_result, Err(RequestError::SerdeJson(_))),
             "{:?}",
             invalid_put_result
         );

src/firecracker/src/api_server/request/drive.rs

@@ -5,19 +5,19 @@ use vmm::logger::{IncMetric, METRICS};
 use vmm::rpc_interface::VmmAction;
 use vmm::vmm_config::drive::{BlockDeviceConfig, BlockDeviceUpdateConfig};
 
-use super::super::parsed_request::{checked_id, Error, ParsedRequest};
+use super::super::parsed_request::{checked_id, ParsedRequest, RequestError};
 use super::{Body, StatusCode};
 
 pub(crate) fn parse_put_drive(
     body: &Body,
     id_from_path: Option<&str>,
-) -> Result<ParsedRequest, Error> {
+) -> Result<ParsedRequest, RequestError> {
     METRICS.put_api_requests.drive_count.inc();
     let id = if let Some(id) = id_from_path {
         checked_id(id)?
     } else {
         METRICS.put_api_requests.drive_fails.inc();
-        return Err(Error::EmptyID);
+        return Err(RequestError::EmptyID);
     };
 
     let device_cfg = serde_json::from_slice::<BlockDeviceConfig>(body.raw()).map_err(|err| {
@@ -27,7 +27,7 @@ pub(crate) fn parse_put_drive(
 
     if id != device_cfg.drive_id {
         METRICS.put_api_requests.drive_fails.inc();
-        Err(Error::Generic(
+        Err(RequestError::Generic(
             StatusCode::BadRequest,
             "The id from the path does not match the id from the body!".to_string(),
         ))
@@ -41,13 +41,13 @@ pub(crate) fn parse_put_drive(
 pub(crate) fn parse_patch_drive(
     body: &Body,
     id_from_path: Option<&str>,
-) -> Result<ParsedRequest, Error> {
+) -> Result<ParsedRequest, RequestError> {
     METRICS.patch_api_requests.drive_count.inc();
     let id = if let Some(id) = id_from_path {
         checked_id(id)?
     } else {
         METRICS.patch_api_requests.drive_fails.inc();
-        return Err(Error::EmptyID);
+        return Err(RequestError::EmptyID);
     };
 
     let block_device_update_cfg: BlockDeviceUpdateConfig =
@@ -58,7 +58,7 @@ pub(crate) fn parse_patch_drive(
 
     if id != block_device_update_cfg.drive_id {
         METRICS.patch_api_requests.drive_fails.inc();
-        return Err(Error::Generic(
+        return Err(RequestError::Generic(
             StatusCode::BadRequest,
             String::from("The id from the path does not match the id from the body!"),
         ));

src/firecracker/src/api_server/request/entropy.rs

@@ -4,10 +4,10 @@
 use vmm::rpc_interface::VmmAction;
 use vmm::vmm_config::entropy::EntropyDeviceConfig;
 
-use super::super::parsed_request::{Error, ParsedRequest};
+use super::super::parsed_request::{ParsedRequest, RequestError};
 use super::Body;
 
-pub(crate) fn parse_put_entropy(body: &Body) -> Result<ParsedRequest, Error> {
+pub(crate) fn parse_put_entropy(body: &Body) -> Result<ParsedRequest, RequestError> {
     let cfg = serde_json::from_slice::<EntropyDeviceConfig>(body.raw())?;
     Ok(ParsedRequest::new_sync(VmmAction::SetEntropyDevice(cfg)))
 }

src/firecracker/src/api_server/request/instance_info.rs

@@ -4,9 +4,9 @@
 use vmm::logger::{IncMetric, METRICS};
 use vmm::rpc_interface::VmmAction;
 
-use super::super::parsed_request::{Error, ParsedRequest};
+use super::super::parsed_request::{ParsedRequest, RequestError};
 
-pub(crate) fn parse_get_instance_info() -> Result<ParsedRequest, Error> {
+pub(crate) fn parse_get_instance_info() -> Result<ParsedRequest, RequestError> {
     METRICS.get_api_requests.instance_info_count.inc();
     Ok(ParsedRequest::new_sync(VmmAction::GetVmInstanceInfo))
 }

src/firecracker/src/api_server/request/logger.rs

@@ -4,10 +4,10 @@
 use vmm::logger::{IncMetric, METRICS};
 use vmm::rpc_interface::VmmAction;
 
-use super::super::parsed_request::{Error, ParsedRequest};
+use super::super::parsed_request::{ParsedRequest, RequestError};
 use super::Body;
 
-pub(crate) fn parse_put_logger(body: &Body) -> Result<ParsedRequest, Error> {
+pub(crate) fn parse_put_logger(body: &Body) -> Result<ParsedRequest, RequestError> {
     METRICS.put_api_requests.logger_count.inc();
     let res = serde_json::from_slice::<vmm::logger::LoggerConfig>(body.raw());
     let config = res.map_err(|err| {