Add api types crate & update apiclient errors

sources/Cargo.toml

@@ -129,6 +129,7 @@ members = [
     "shimpei",
 
     "xfscli",
+    "api/apitypes",
 ]
 
 [profile.release]

sources/api/apiclient/Cargo.toml

@@ -21,7 +21,7 @@ hyper = { version = "0.14", default-features = false, features = ["client", "htt
 hyper-unix-connector = "0.2"
 libc = "0.2"
 log = "0.4"
-models = { path = "../../models", version = "0.1" }
+apitypes = { path = "../apitypes", version = "0.1" }
 nix = "0.26"
 rand = "0.8"
 reqwest = { version = "0.11", default-features = false, features = ["rustls-tls-native-roots"] }

sources/api/apiclient/src/exec.rs

@@ -13,11 +13,11 @@
 // it and give it a channel it can use to send to the server, it starts a thread, and you get back
 // the struct, which contains a channel that tells you if the heartbeat dies.
 
+use apitypes::exec::{ClientMessage, Initialize, ServerMessage, Size};
 use futures::{Future, FutureExt, Stream, StreamExt, TryStream, TryStreamExt};
 use futures_channel::{mpsc, oneshot};
 use libc::{ioctl, winsize as WinSize, STDOUT_FILENO, TIOCGWINSZ as GetWinSize};
 use log::{debug, error, trace, warn};
-use model::exec::{ClientMessage, Initialize, ServerMessage, Size};
 use retry_read::RetryRead;
 use signal_hook::{consts::signal, iterator::Signals};
 use snafu::{OptionExt, ResultExt};

sources/api/apiclient/src/exec/terminal.rs

@@ -2,9 +2,9 @@
 //! settings, resetting them to their original state when the Terminal is dropped.
 
 use super::get_winsize;
+use apitypes::exec::TtyInit;
 use libc::{STDIN_FILENO, STDOUT_FILENO};
 use log::{debug, warn};
-use model::exec::TtyInit;
 use nix::{
     sys::termios::{cfmakeraw, tcgetattr, tcsetattr, SetArg, Termios},
     unistd::isatty,

sources/api/apiclient/src/lib.rs

@@ -47,6 +47,10 @@ mod error {
             body: String,
         },
 
+        // This type of error just returns the source.
+        #[snafu(display("{}", body))]
+        Raw { body: String },
+
         #[snafu(display("Failed to read body of response: {}", source))]
         ResponseBodyRead { source: hyper::Error },
 
@@ -84,18 +88,7 @@ where
     S2: AsRef<str>,
 {
     let (status, body) = raw_request_unchecked(&socket_path, &uri, &method, data).await?;
-
-    // Error if the response status is in not in the 2xx range.
-    ensure!(
-        status.is_success(),
-        error::ResponseStatusSnafu {
-            method: method.as_ref(),
-            code: status,
-            uri: uri.as_ref(),
-            body,
-        }
-    );
-
+    check_invalid_client_input(body.as_ref(), status, method.as_ref(), uri.as_ref())?;
     Ok((status, body))
 }
 
@@ -156,6 +149,50 @@ pub(crate) fn rando() -> String {
         .collect()
 }
 
+/// Different Client type errors we expect.
+const CLIENT_DESERIALIZATION_MAP_ERROR: &str = "Unable to match your input to the data model.  We may not have enough type information.  Please try the --json input form";
+const CLIENT_DESERIALIZATION_JSON_ERROR: &str = "Unable to deserialize input JSON into model";
+const SERVER_DESERIALIZATION_JSON_ERROR: &str = "Json deserialize error";
+const CLIENT_SERIALIZATION_ERROR: &str = "Unable to serialize data";
+
+#[derive(Debug)]
+enum ClientTypeErrors {}
+
+impl ClientTypeErrors {
+    fn from(input: &str) -> Option<&str> {
+        if input.contains(CLIENT_DESERIALIZATION_JSON_ERROR)
+            || input.contains(CLIENT_DESERIALIZATION_MAP_ERROR)
+            || input.contains(SERVER_DESERIALIZATION_JSON_ERROR)
+            || input.contains(CLIENT_SERIALIZATION_ERROR)
+        {
+            Some("client_error")
+        } else {
+            None
+        }
+    }
+}
+
+fn check_invalid_client_input(
+    body: &str,
+    status: http::StatusCode,
+    method: &str,
+    uri: &str,
+) -> Result<()> {
+    match ClientTypeErrors::from(body) {
+        Some(_) => ensure!(status.is_success(), error::RawSnafu { body }),
+        None => ensure!(
+            status.is_success(),
+            error::ResponseStatusSnafu {
+                method: method.to_string(),
+                code: status,
+                uri: uri.to_string(),
+                body,
+            }
+        ),
+    };
+    Ok(())
+}
+
 /// Different input types supported by the Settings API.
 #[derive(Debug)]
 pub enum SettingsInput {

sources/api/apiclient/src/main.rs

@@ -778,7 +778,12 @@ async fn run() -> Result<()> {
 
             set::set(&args.socket_path, settings)
                 .await
-                .context(error::SetSnafu)?;
+                .map_err(|e| match e {
+                    set::Error::Raw { source: _ } => error::Error::Raw {
+                        source: Box::new(e),
+                    },
+                    _ => error::Error::Set { source: e },
+                })?;
         }
 
         Subcommand::Update(subcommand) => match subcommand {
@@ -905,6 +910,12 @@ mod error {
             source: Box<apiclient::Error>,
         },
 
+        // This type of error just returns the source.
+        #[snafu(display("{}", source))]
+        Raw {
+            #[snafu(source(from(apiclient::set::Error, Box::new)))]
+            source: Box<apiclient::set::Error>,
+        },
         #[snafu(display("Failed to get report: {}", source))]
         Report { source: report::Error },
 
@@ -924,4 +935,5 @@ mod error {
         UpdateCheck { source: update::Error },
     }
 }
+
 type Result<T> = std::result::Result<T, error::Error>;

sources/api/apiclient/src/set.rs

@@ -21,7 +21,16 @@ where
     let method = "PATCH";
     let (_status, _body) = crate::raw_request(&socket_path, &uri, method, Some(settings_data))
         .await
-        .context(error::RequestSnafu { uri, method })?;
+        .map_err(|e| match e {
+            crate::Error::Raw { body } => Error::Raw {
+                source: Box::new(crate::Error::Raw { body }),
+            },
+            _ => Error::Request {
+                method: method.to_string(),
+                uri,
+                source: Box::new(e),
+            },
+        })?;
 
     // Commit the transaction and apply it to the system.
     let uri = format!("/tx/commit_and_apply?tx={}", transaction);
@@ -49,6 +58,13 @@ mod error {
             #[snafu(source(from(crate::Error, Box::new)))]
             source: Box<crate::Error>,
         },
+
+        // This type of error just returns the source.
+        #[snafu(display("{}", source))]
+        Raw {
+            #[snafu(source(from(crate::Error, Box::new)))]
+            source: Box<crate::Error>,
+        },
     }
 }
 pub use error::Error;

sources/api/apiserver/Cargo.toml

@@ -22,6 +22,7 @@ http = "0.2"
 libc = "0.2"
 log = "0.4"
 models = { path = "../../models", version = "0.1" }
+apitypes = { path = "../apitypes", version = "0.1" }
 nix = "0.26"
 num = "0.4"
 rand = "0.8"

sources/api/apiserver/src/server/exec.rs

@@ -13,8 +13,8 @@
 use actix::prelude::{Actor, ActorContext, AsyncContext, Handler, StreamHandler};
 use actix_web::{web, Error, HttpRequest, HttpResponse};
 use actix_web_actors::ws::{self, Message};
+use apitypes::exec::{Capacity, ClientMessage, ServerMessage};
 use log::{debug, error, info};
-use model::exec::{Capacity, ClientMessage, ServerMessage};
 use std::convert::TryFrom;
 use std::fmt::Debug;
 use std::path::PathBuf;

sources/api/apiserver/src/server/exec/child.rs

@@ -20,10 +20,10 @@
 
 use super::{message, WsExec, CAPACITY_UPDATE_INTERVAL, MAX_MESSAGES_OUTSTANDING};
 use actix::prelude::{Addr, SendError};
+use apitypes::exec::{Capacity, Initialize, Size, TtyInit};
 use bytes::Bytes;
 use libc::{ioctl, login_tty, winsize as WinSize, TIOCSWINSZ as SetWinSize};
 use log::{debug, error};
-use model::exec::{Capacity, Initialize, Size, TtyInit};
 use nix::{
     errno::Errno,
     fcntl::{fcntl, FcntlArg, FdFlag, OFlag},

sources/api/apitypes/Cargo.toml

@@ -0,0 +1,17 @@
+[package]
+name = "apitypes"
+version = "0.1.0"
+edition = "2021"
+authors = ["Sumukh Ballal <sballal@amazon.com>"]
+license = "Apache-2.0 OR MIT"
+publish = false
+build = "build.rs"
+# Don't rebuild crate just because of changes to README.
+exclude = ["README.md"]
+
+[dependencies]
+libc = "0.2"
+serde = { version = "1", features = ["derive"] }
+
+[build-dependencies]
+generate-readme = { version = "0.1", path = "../../generate-readme" }

sources/api/apitypes/README.md

@@ -0,0 +1,10 @@
+# apitypes
+
+Current version: 0.1.0
+
+The apitypes library provides types which are in common between apiclient and apiserver.
+The ['exec'] module holds types used to communicate between client and server.
+
+## Colophon
+
+This text was generated using [cargo-readme](https://crates.io/crates/cargo-readme), and includes the rustdoc from `src/lib.rs`.

sources/api/apitypes/README.tpl

@@ -0,0 +1,9 @@
+# {{crate}}
+
+Current version: {{version}}
+
+{{readme}}
+
+## Colophon
+
+This text was generated using [cargo-readme](https://crates.io/crates/cargo-readme), and includes the rustdoc from `src/lib.rs`.

sources/api/apitypes/build.rs

@@ -0,0 +1,3 @@
+fn main() {
+    generate_readme::from_lib().unwrap();
+}

sources/api/apitypes/src/lib.rs

@@ -0,0 +1,3 @@
+//! The apitypes library provides types which are in common between apiclient and apiserver.
+//! The ['exec'] module holds types used to communicate between client and server.
+pub mod exec;

sources/models/src/lib.rs

@@ -31,9 +31,6 @@ use modeled_types::KubernetesMemoryManagerPolicy;
 use modeled_types::KubernetesMemoryReservation;
 use modeled_types::NonNegativeInteger;
 
-// Types used to communicate between client and server for 'apiclient exec'.
-pub mod exec;
-
 // Below, we define common structures used in the API surface; specific variants build a Settings
 // structure based on these, and that's what gets exposed via the API.  (Specific variants' models
 // are in subdirectories and linked into place by build.rs at variant/current.)