From 67225336df0a8480a0f460adbeb0ccc0677df6dd Mon Sep 17 00:00:00 2001 From: Stefan Bossbaly Date: Mon, 3 Jun 2024 21:59:09 -0400 Subject: [PATCH 1/2] Added serde_debugging feature We use to use the test feature flag but these tests are not unit tests and therefore should not go into the mod test { } namespace. They instead should be placed as part of an integration tests. Other users might want this feature so instead of strictly making this a test feature we will make it an optional feature that developers can enable if they so like. --- .github/workflows/ci.yml | 98 +++++++++++++--- .github/workflows/live_endpoint.yml | 4 +- Cargo.toml | 5 +- README.md | 14 +++ amtrak-api.code-workspace | 2 + src/client.rs | 170 ++++++++++++---------------- src/errors.rs | 16 +++ tests/live_endpoint_canary.rs | 30 +++++ 8 files changed, 228 insertions(+), 111 deletions(-) create mode 100644 tests/live_endpoint_canary.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 30a3861..aae3890 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,31 +2,46 @@ name: Rust CI on: push: - branches: [ master ] + branches: [master] pull_request: - branches: [ master ] + branches: [master] jobs: - test: - name: Test + lint: + name: Lint strategy: matrix: os: - ubuntu-latest - macos-latest - - windows-latest - rust: + toolchain: - stable + - beta + - nightly runs-on: ${{ matrix.os }} + continue-on-error: ${{ matrix.toolchain != 'stable' }} steps: - name: Checkout Source uses: actions/checkout@v4 - - name: Install Rust Toolchain + - name: Install Stable Rust Toolchain + if: matrix.toolchain == 'stable' uses: dtolnay/rust-toolchain@stable with: components: rustfmt, clippy + - name: Install Beta Rust Toolchain + if: matrix.toolchain == 'beta' + uses: dtolnay/rust-toolchain@beta + with: + components: rustfmt, clippy + + - name: Install Nightly Rust Toolchain + if: matrix.toolchain == 'nightly' + uses: dtolnay/rust-toolchain@nightly + with: + components: rustfmt, clippy + - name: Setup Rust Cache uses: Swatinem/rust-cache@v2 @@ -36,22 +51,79 @@ jobs: rustc --version cargo clippy --version - - name: Lint + - name: Format Check run: | cargo fmt -- --check + + - name: Lint + run: | cargo clippy -- -D warnings + - name: Dry Publish + uses: katyo/publish-crates@v2 + with: + dry-run: true + ignore-unpublished-changes: true + + test: + name: Test + strategy: + matrix: + os: + - ubuntu-latest + - macos-latest + features: + - default + - serde_debugging + toolchain: + - stable + - beta + - nightly + runs-on: ${{ matrix.os }} + continue-on-error: ${{ matrix.toolchain != 'stable' }} + steps: + - name: Checkout Source + uses: actions/checkout@v4 + + - name: Install Stable Rust Toolchain + if: matrix.toolchain == 'stable' + uses: dtolnay/rust-toolchain@stable + + - name: Install Beta Rust Toolchain + if: matrix.toolchain == 'beta' + uses: dtolnay/rust-toolchain@beta + + - name: Install Nightly Rust Toolchain + if: matrix.toolchain == 'nightly' + uses: dtolnay/rust-toolchain@nightly + + - name: Setup Rust Cache + uses: Swatinem/rust-cache@v2 + + - name: Display Toolchain Information + run: | + cargo --version --verbose + rustc --version + cargo clippy --version + - name: Test + if: matrix.features == 'default' run: | cargo check cargo test --all + - name: Test + if: matrix.features != 'default' + run: | + cargo check --features ${{ matrix.features }} + cargo test --all --features ${{ matrix.features }} + - name: Build + if: matrix.features == 'default' run: | cargo build --release - - name: Dry Publish - uses: katyo/publish-crates@v2 - with: - dry-run: true - ignore-unpublished-changes: true + - name: Build + if: matrix.features != 'default' + run: | + cargo build --release --features ${{ matrix.features }} diff --git a/.github/workflows/live_endpoint.yml b/.github/workflows/live_endpoint.yml index 4037b8a..fe09f9b 100644 --- a/.github/workflows/live_endpoint.yml +++ b/.github/workflows/live_endpoint.yml @@ -26,5 +26,5 @@ jobs: - name: Live Endpoint Test run: | - cargo check - cargo test --verbose --lib live + cargo check --features serde_debugging + cargo test --verbose --features serde_debugging --test live_endpoint_canary diff --git a/Cargo.toml b/Cargo.toml index 4ee78d6..38db1b8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,17 +16,20 @@ include = [ "/LICENSE" ] +[features] +serde_debugging = ["dep:serde_path_to_error"] + [dependencies] reqwest = { version = "0.12.4", features = ["json"] } serde_json = "1.0.117" serde = { version = "1.0.203", features = ["derive"] } chrono = { version = "0.4", features = ["serde"] } thiserror = "1.0.61" +serde_path_to_error = { version = "0.1.16", optional = true } [dev-dependencies] mockito = "1.4.0" tokio = { version = "1.38.0", features = ["full"] } -serde_path_to_error = "0.1.16" anyhow = "1.0.86" [[example]] diff --git a/README.md b/README.md index 6347b59..521ec16 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,20 @@ async fn main() -> Result<(), Box> { 662-27 train is heading to New York Penn, currently enroute to Parkesburg with an ETA of 2 minutes ``` +## Features + +- `serde_debugging` (Disabled by default): Enables the the following functions: + `trains_with_debugging`, `train_with_debugging`, `stations_with_debugging`, + `station_with_debugging`. These functions will operate the exact same way as + their counterparts with the exception that deserialization is completed using + [`serde_path_to_error`](https://crates.io/crates/serde_path_to_error) adapter. + This crate will print out the path of the offending field if deserialization + were to fail. The only quarky behavior is that instead of returning + `Error::DeserializeFailed`, the debugging functions will instead return + `Error:Other`. This is so that we can include the JSON response in the error + as well as the path to the field that caused the deserialization to fail, + making debugging a lot easier. + ## Authors Stefan Bossbaly diff --git a/amtrak-api.code-workspace b/amtrak-api.code-workspace index 880d8b8..b660227 100644 --- a/amtrak-api.code-workspace +++ b/amtrak-api.code-workspace @@ -6,6 +6,8 @@ ], "settings": { "rust-analyzer.check.command": "clippy", + "rust-analyzer.cargo.features": ["", "serde_debugging"], + "rust-analyzer.check.features": ["", "serde_debugging"], "editor.formatOnSave": true }, "launch": { diff --git a/src/client.rs b/src/client.rs index cb9f9ad..bfc4b3e 100644 --- a/src/client.rs +++ b/src/client.rs @@ -10,6 +10,9 @@ const BASE_API_URL: &str = "https://api-v3.amtraker.com/v3"; pub type Result = std::result::Result; +#[cfg(feature = "serde_debugging")] +pub type DebuggingResult = std::result::Result; + /// A client instance /// /// Note: This does not represent an active connection. Connections are @@ -150,15 +153,18 @@ impl Client { Ok(response.0) } - /// Same as [`trains`] but using `serde_path_to_error` as the deserialize adapter + /// Same as [`trains`] but using [`serde_path_to_error`] as the deserialize adapter /// - /// Used for debugging purposes only and should not be used in production + /// Note: This function will will return [`Error::Other`] instead of [`Error::DeserializeFailed`] + /// when a deserialization error occurs. The reason for this is that we want to log the offending + /// JSON when a deserialization error does occur and will use the [`anyhow`] crate to include the + /// JSON and failed field path to make debugging a lot easier. /// /// [`trains`]: Client::trains - #[cfg(test)] - pub async fn trains_with_debugging(&self) -> anyhow::Result { - use anyhow::anyhow; - + /// [`Error::Other`]: errors::Error::Other + /// [`Error::DeserializeFailed`]: errors::Error::DeserializeFailed + #[cfg(feature = "serde_debugging")] + pub async fn trains_with_debugging(&self) -> DebuggingResult { let url = format!("{}/trains", self.base_url); let bytes = reqwest::Client::new() @@ -171,13 +177,11 @@ impl Client { let response: responses::TrainResponseWrapper = serde_path_to_error::deserialize( &mut serde_json::Deserializer::from_slice(bytes.as_ref()), ) - .map_err(|err| { - let path = err.path().to_string(); - anyhow!( - "Error deserializing TrainResponseWrapper: {}: {}", - path, - std::str::from_utf8(bytes.as_ref()).unwrap_or("Failed to convert bytes to string") - ) + .map_err(|err| errors::DebuggingError::DeserializeFailed { + error: err, + response: std::str::from_utf8(bytes.as_ref()) + .unwrap_or("Failed to convert bytes to string") + .to_string(), })?; Ok(response.0) @@ -256,10 +260,10 @@ impl Client { /// [`TrainResponse`]: responses::TrainResponse /// [`train_id`]: responses::Train::train_id /// [`train_num`]: responses::Train::train_num - pub async fn train>( - &self, - train_identifier: S, - ) -> Result { + pub async fn train(&self, train_identifier: S) -> Result + where + S: AsRef, + { let url = format!("{}/trains/{}", self.base_url, train_identifier.as_ref()); let response = reqwest::Client::new() @@ -272,18 +276,24 @@ impl Client { Ok(response.0) } - /// Same as [`train`] but using `serde_path_to_error` as the deserialize adapter + /// Same as [`train`] but using [`serde_path_to_error`] as the deserialize adapter /// - /// Used for debugging purposes only and should not be used in production + /// Note: This function will will return [`Error::Other`] instead of [`Error::DeserializeFailed`] + /// when a deserialization error occurs. The reason for this is that we want to log the offending + /// JSON when a deserialization error does occur and will use the [`anyhow`] crate to include the + /// JSON and failed field path to make debugging a lot easier. /// /// [`train`]: Client::train - #[cfg(test)] - pub async fn train_with_debugging>( + /// [`Error::Other`]: errors::Error::Other + /// [`Error::DeserializeFailed`]: errors::Error::DeserializeFailed + #[cfg(feature = "serde_debugging")] + pub async fn train_with_debugging( &self, train_identifier: S, - ) -> anyhow::Result { - use anyhow::anyhow; - + ) -> DebuggingResult + where + S: AsRef, + { let url = format!("{}/trains/{}", self.base_url, train_identifier.as_ref()); let bytes = reqwest::Client::new() @@ -296,13 +306,11 @@ impl Client { let response: responses::TrainResponseWrapper = serde_path_to_error::deserialize( &mut serde_json::Deserializer::from_slice(bytes.as_ref()), ) - .map_err(|err| { - let path = err.path().to_string(); - anyhow!( - "Error deserializing TrainResponseWrapper: {}: {}", - path, - std::str::from_utf8(bytes.as_ref()).unwrap_or("Failed to convert bytes to string") - ) + .map_err(|err| errors::DebuggingError::DeserializeFailed { + error: err, + response: std::str::from_utf8(bytes.as_ref()) + .unwrap_or("Failed to convert bytes to string") + .to_string(), })?; Ok(response.0) @@ -350,15 +358,18 @@ impl Client { Ok(response.0) } - /// Same as [`stations`] but using `serde_path_to_error` as the deserialize adapter + /// Same as [`stations`] but using [`serde_path_to_error`] as the deserialize adapter /// - /// Used for debugging purposes only and should not be used in production + /// Note: This function will will return [`Error::Other`] instead of [`Error::DeserializeFailed`] + /// when a deserialization error occurs. The reason for this is that we want to log the offending + /// JSON when a deserialization error does occur and will use the [`anyhow`] crate to include the + /// JSON and failed field path to make debugging a lot easier. /// /// [`stations`]: Client::stations - #[cfg(test)] - pub async fn stations_with_debugging(&self) -> anyhow::Result { - use anyhow::anyhow; - + /// [`Error::Other`]: errors::Error::Other + /// [`Error::DeserializeFailed`]: errors::Error::DeserializeFailed + #[cfg(feature = "serde_debugging")] + pub async fn stations_with_debugging(&self) -> DebuggingResult { let url = format!("{}/stations", self.base_url); let bytes = reqwest::Client::new() @@ -371,13 +382,11 @@ impl Client { let response: responses::StationResponseWrapper = serde_path_to_error::deserialize( &mut serde_json::Deserializer::from_slice(bytes.as_ref()), ) - .map_err(|err| { - let path = err.path().to_string(); - anyhow!( - "Error deserializing StationResponseWrapper: {}: {}", - path, - std::str::from_utf8(bytes.as_ref()).unwrap_or("Failed to convert bytes to string") - ) + .map_err(|err| errors::DebuggingError::DeserializeFailed { + error: err, + response: std::str::from_utf8(bytes.as_ref()) + .unwrap_or("Failed to convert bytes to string") + .to_string(), })?; Ok(response.0) @@ -422,10 +431,10 @@ impl Client { /// /// [`StationResponse`]: responses::StationResponse /// [`code`]: responses::TrainStation::code - pub async fn station>( - &self, - station_code: S, - ) -> Result { + pub async fn station(&self, station_code: S) -> Result + where + S: AsRef, + { let url = format!("{}/stations/{}", self.base_url, station_code.as_ref()); let response = reqwest::Client::new() @@ -438,18 +447,24 @@ impl Client { Ok(response.0) } - /// Same as [`station`] but using `serde_path_to_error` as the deserialize adapter + /// Same as [`station`] but using [`serde_path_to_error`] as the deserialize adapter /// - /// Used for debugging purposes only and should not be used in production + /// Note: This function will will return [`Error::Other`] instead of [`Error::DeserializeFailed`] + /// when a deserialization error occurs. The reason for this is that we want to log the offending + /// JSON when a deserialization error does occur and will use the [`anyhow`] crate to include the + /// JSON and failed field path to make debugging a lot easier. /// /// [`station`]: Client::station - #[cfg(test)] - pub async fn station_with_debugging>( + /// [`Error::Other`]: errors::Error::Other + /// [`Error::DeserializeFailed`]: errors::Error::DeserializeFailed + #[cfg(feature = "serde_debugging")] + pub async fn station_with_debugging( &self, station_code: S, - ) -> anyhow::Result { - use anyhow::anyhow; - + ) -> DebuggingResult + where + S: AsRef, + { let url = format!("{}/stations/{}", self.base_url, station_code.as_ref()); let bytes = reqwest::Client::new() @@ -462,48 +477,13 @@ impl Client { let response: responses::StationResponseWrapper = serde_path_to_error::deserialize( &mut serde_json::Deserializer::from_slice(bytes.as_ref()), ) - .map_err(|err| { - let path = err.path().to_string(); - anyhow!( - "Error deserializing StationResponseWrapper: {}: {}", - path, - std::str::from_utf8(bytes.as_ref()).unwrap_or("Failed to convert bytes to string") - ) + .map_err(|err| errors::DebuggingError::DeserializeFailed { + error: err, + response: std::str::from_utf8(bytes.as_ref()) + .unwrap_or("Failed to convert bytes to string") + .to_string(), })?; Ok(response.0) } } - -#[cfg(test)] -mod tests { - use super::*; - - /// Test the live train endpoint using serde_path_to_error as the deserialize driver - /// - /// This test will call the live train endpoint to list all the trains that are currently - /// in the system. We do not test for correct deserialization since we do not have truth - /// data to compare against, we are just ensuring that we can deserialize the response - /// provided by the Amtrak API. - #[tokio::test] - async fn test_live_train_api() -> anyhow::Result<()> { - let client = Client::new(); - let _ = client.trains_with_debugging().await?; - - Ok(()) - } - - /// Test the live station endpoint using serde_path_to_error as the deserialize driver - /// - /// This test will call the live station endpoint to list all the stations that are currently - /// in the system. We do not test for correct deserialization since we do not have truth - /// data to compare against, we are just ensuring that we can deserialize the response - /// provided by the Amtrak API. - #[tokio::test] - async fn test_live_station_api() -> anyhow::Result<()> { - let client = Client::new(); - let _ = client.stations_with_debugging().await?; - - Ok(()) - } -} diff --git a/src/errors.rs b/src/errors.rs index e254b6f..631b1bd 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -9,3 +9,19 @@ pub enum Error { #[error("API returned an error response: {0}")] ApiErrorResponse(String), } + +#[cfg(feature = "serde_debugging")] +#[derive(Debug, thiserror::Error)] +pub enum DebuggingError { + #[error("Unable to send the request: {0}")] + RequestFailed(#[from] reqwest::Error), + + #[error("Unable to deserialize the received value: {error}: {response}")] + DeserializeFailed { + error: serde_path_to_error::Error, + response: String, + }, + + #[error("API returned an error response: {0}")] + ApiErrorResponse(String), +} diff --git a/tests/live_endpoint_canary.rs b/tests/live_endpoint_canary.rs new file mode 100644 index 0000000..b756b0f --- /dev/null +++ b/tests/live_endpoint_canary.rs @@ -0,0 +1,30 @@ +#![cfg(feature = "serde_debugging")] +use amtrak_api::Client; + +/// Test the live train endpoint using serde_path_to_error as the deserialize driver +/// +/// This test will call the live train endpoint to list all the trains that are currently +/// in the system. We do not test for correct deserialization since we do not have truth +/// data to compare against, we are just ensuring that we can deserialize the response +/// provided by the Amtrak API. +#[tokio::test] +async fn test_live_train_api() -> anyhow::Result<()> { + let client = Client::new(); + let _ = client.trains_with_debugging().await?; + + Ok(()) +} + +/// Test the live station endpoint using serde_path_to_error as the deserialize driver +/// +/// This test will call the live station endpoint to list all the stations that are currently +/// in the system. We do not test for correct deserialization since we do not have truth +/// data to compare against, we are just ensuring that we can deserialize the response +/// provided by the Amtrak API. +#[tokio::test] +async fn test_live_station_api() -> anyhow::Result<()> { + let client = Client::new(); + let _ = client.stations_with_debugging().await?; + + Ok(()) +} From e702bfd6cea13d36755e42eed00771e61cb76d7f Mon Sep 17 00:00:00 2001 From: Stefan Bossbaly Date: Sun, 16 Jun 2024 17:43:02 -0400 Subject: [PATCH 2/2] Fix CI --- .github/workflows/ci.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aae3890..6bd91ba 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,7 +48,7 @@ jobs: - name: Display Toolchain Information run: | cargo --version --verbose - rustc --version + rustc --version --verbose cargo clippy --version - name: Format Check @@ -103,8 +103,7 @@ jobs: - name: Display Toolchain Information run: | cargo --version --verbose - rustc --version - cargo clippy --version + rustc --version --verbose - name: Test if: matrix.features == 'default'