Standardize error handling with error-stack

.agents/rules/rust-error-handling.mdc

@@ -19,7 +19,7 @@ alwaysApply: false
 
 - Use `change_context()` to map error types consistently
 - Ensure that errors include sufficient context for debugging
-- Add `attach_printable()` or `attach_printable_lazy()` to include relevant debug information
+- Add `attach()` or `attach_with()` to include relevant debug information
 - When reporting errors to users, provide actionable guidance where possible
 
 ## Error Definition

CONTRIBUTING.md

@@ -116,7 +116,23 @@ See also: #456, #789
 
 Consistency is the most important. Following the existing Rust style, formatting, and naming conventions of the file you are modifying and of the overall project. Failure to do so will result in a prolonged review process that has to focus on updating the superficial aspects of your code, rather than improving its functionality and performance.
 
-Style and format will be enforced with a linter when the PR is created.
+Style and format will be enforced with a linter when PR is created.
+
+## :warning: Error Handling
+
+We use [error-stack](https://docs.rs/error-stack/latest/error_stack/) for error handling to provide rich context and traceability.
+
+### Guidelines
+
+1. **Use `Report<E>`**: Public functions should generally return `Result<T, Report<TrustedServerError>>`.
+2. **Context**: Use `.change_context(TrustedServerError::Variant)` to wrap errors and provide semantic meaning.
+   ```rust
+   // Good
+   file.read_to_string(&mut content)
+       .change_context(TrustedServerError::Configuration { message: "Failed to read config".into() })?;
+   ```
+3. **Attachments**: Use `.attach_printable("additional info")` to add debugging context without changing the error variant.
+4. **Consistency**: Avoid returning bare `TrustedServerError` unless absolutely necessary (e.g. implementing traits). Wrap them in `Report::new()`.
 
 ## :pray: Credits
 

crates/common/src/fastly_storage.rs

@@ -1,5 +1,6 @@
 use std::io::Read;
 
+use error_stack::{Report, ResultExt};
 use fastly::{ConfigStore, Request, Response, SecretStore};
 use http::StatusCode;
 
@@ -24,17 +25,17 @@ impl FastlyConfigStore {
     /// # Errors
     ///
     /// Returns an error if the key is not found in the config store.
-    pub fn get(&self, key: &str) -> Result<String, TrustedServerError> {
+    pub fn get(&self, key: &str) -> Result<String, Report<TrustedServerError>> {
         // TODO use try_open and return the error
         let store = ConfigStore::open(&self.store_name);
-        store
-            .get(key)
-            .ok_or_else(|| TrustedServerError::Configuration {
+        store.get(key).ok_or_else(|| {
+            Report::new(TrustedServerError::Configuration {
                 message: format!(
                     "Key '{}' not found in config store '{}'",
                     key, self.store_name
                 ),
             })
+        })
     }
 }
 
@@ -55,25 +56,28 @@ impl FastlySecretStore {
     ///
     /// Returns an error if the secret store cannot be opened, the key is not found,
     /// or the secret plaintext cannot be retrieved.
-    pub fn get(&self, key: &str) -> Result<Vec<u8>, TrustedServerError> {
-        let store =
-            SecretStore::open(&self.store_name).map_err(|_| TrustedServerError::Configuration {
+    pub fn get(&self, key: &str) -> Result<Vec<u8>, Report<TrustedServerError>> {
+        let store = SecretStore::open(&self.store_name).map_err(|_| {
+            Report::new(TrustedServerError::Configuration {
                 message: format!("Failed to open SecretStore '{}'", self.store_name),
-            })?;
+            })
+        })?;
 
-        let secret = store
-            .get(key)
-            .ok_or_else(|| TrustedServerError::Configuration {
+        let secret = store.get(key).ok_or_else(|| {
+            Report::new(TrustedServerError::Configuration {
                 message: format!(
                     "Secret '{}' not found in secret store '{}'",
                     key, self.store_name
                 ),
-            })?;
+            })
+        })?;
 
         secret
             .try_plaintext()
-            .map_err(|_| TrustedServerError::Configuration {
-                message: "Failed to get secret plaintext".into(),
+            .map_err(|_| {
+                Report::new(TrustedServerError::Configuration {
+                    message: "Failed to get secret plaintext".into(),
+                })
             })
             .map(|bytes| bytes.into_iter().collect())
     }
@@ -83,10 +87,10 @@ impl FastlySecretStore {
     /// # Errors
     ///
     /// Returns an error if the secret cannot be retrieved or is not valid UTF-8.
-    pub fn get_string(&self, key: &str) -> Result<String, TrustedServerError> {
+    pub fn get_string(&self, key: &str) -> Result<String, Report<TrustedServerError>> {
         let bytes = self.get(key)?;
-        String::from_utf8(bytes).map_err(|e| TrustedServerError::Configuration {
-            message: format!("Failed to decode secret as UTF-8: {}", e),
+        String::from_utf8(bytes).change_context(TrustedServerError::Configuration {
+            message: "Failed to decode secret as UTF-8".to_string(),
         })
     }
 }
@@ -103,7 +107,7 @@ impl FastlyApiClient {
     /// # Errors
     ///
     /// Returns an error if the secret store cannot be opened or the API key cannot be retrieved.
-    pub fn new() -> Result<Self, TrustedServerError> {
+    pub fn new() -> Result<Self, Report<TrustedServerError>> {
         Self::from_secret_store("api-keys", "api_key")
     }
 
@@ -112,12 +116,11 @@ impl FastlyApiClient {
     /// # Errors
     ///
     /// Returns an error if the API backend cannot be ensured or the API key cannot be retrieved.
-    pub fn from_secret_store(store_name: &str, key_name: &str) -> Result<Self, TrustedServerError> {
-        let backend_name = ensure_backend_from_url(FASTLY_API_HOST).map_err(|e| {
-            TrustedServerError::Configuration {
-                message: format!("Failed to ensure API backend: {}", e),
-            }
-        })?;
+    pub fn from_secret_store(
+        store_name: &str,
+        key_name: &str,
+    ) -> Result<Self, Report<TrustedServerError>> {
+        let backend_name = ensure_backend_from_url("https://api.fastly.com")?;
 
         let secret_store = FastlySecretStore::new(store_name);
         let api_key = secret_store.get(key_name)?;
@@ -137,7 +140,7 @@ impl FastlyApiClient {
         path: &str,
         body: Option<String>,
         content_type: &str,
-    ) -> Result<Response, TrustedServerError> {
+    ) -> Result<Response, Report<TrustedServerError>> {
         let url = format!("{}{}", self.base_url, path);
 
         let api_key_str = String::from_utf8_lossy(&self.api_key).to_string();
@@ -148,9 +151,9 @@ impl FastlyApiClient {
             "PUT" => Request::put(&url),
             "DELETE" => Request::delete(&url),
             _ => {
-                return Err(TrustedServerError::Configuration {
+                return Err(Report::new(TrustedServerError::Configuration {
                     message: format!("Unsupported HTTP method: {}", method),
-                })
+                }))
             }
         };
 
@@ -164,11 +167,11 @@ impl FastlyApiClient {
                 .with_body(body_content);
         }
 
-        request
-            .send(&self.backend_name)
-            .map_err(|e| TrustedServerError::Configuration {
+        request.send(&self.backend_name).map_err(|e| {
+            Report::new(TrustedServerError::Configuration {
                 message: format!("Failed to send API request: {}", e),
             })
+        })
     }
 
     /// Updates a configuration item in a Fastly config store.
@@ -181,7 +184,7 @@ impl FastlyApiClient {
         store_id: &str,
         key: &str,
         value: &str,
-    ) -> Result<(), TrustedServerError> {
+    ) -> Result<(), Report<TrustedServerError>> {
         let path = format!("/resources/stores/config/{}/item/{}", store_id, key);
         let payload = format!("item_value={}", value);
 
@@ -196,20 +199,22 @@ impl FastlyApiClient {
         response
             .get_body_mut()
             .read_to_string(&mut buf)
-            .map_err(|e| TrustedServerError::Configuration {
-                message: format!("Failed to read API response: {}", e),
+            .map_err(|e| {
+                Report::new(TrustedServerError::Configuration {
+                    message: format!("Failed to read API response: {}", e),
+                })
             })?;
 
         if response.get_status() == StatusCode::OK {
             Ok(())
         } else {
-            Err(TrustedServerError::Configuration {
+            Err(Report::new(TrustedServerError::Configuration {
                 message: format!(
                     "Failed to update config item: HTTP {} - {}",
                     response.get_status(),
                     buf
                 ),
-            })
+            }))
         }
     }
 
@@ -223,7 +228,7 @@ impl FastlyApiClient {
         store_id: &str,
         secret_name: &str,
         secret_value: &str,
-    ) -> Result<(), TrustedServerError> {
+    ) -> Result<(), Report<TrustedServerError>> {
         let path = format!("/resources/stores/secret/{}/secrets", store_id);
 
         let payload = serde_json::json!({
@@ -238,20 +243,22 @@ impl FastlyApiClient {
         response
             .get_body_mut()
             .read_to_string(&mut buf)
-            .map_err(|e| TrustedServerError::Configuration {
-                message: format!("Failed to read API response: {}", e),
+            .map_err(|e| {
+                Report::new(TrustedServerError::Configuration {
+                    message: format!("Failed to read API response: {}", e),
+                })
             })?;
 
         if response.get_status() == StatusCode::OK {
             Ok(())
         } else {
-            Err(TrustedServerError::Configuration {
+            Err(Report::new(TrustedServerError::Configuration {
                 message: format!(
                     "Failed to create secret: HTTP {} - {}",
                     response.get_status(),
                     buf
                 ),
-            })
+            }))
         }
     }
 
@@ -260,7 +267,11 @@ impl FastlyApiClient {
     /// # Errors
     ///
     /// Returns an error if the API request fails or returns a non-OK/NO_CONTENT status.
-    pub fn delete_config_item(&self, store_id: &str, key: &str) -> Result<(), TrustedServerError> {
+    pub fn delete_config_item(
+        &self,
+        store_id: &str,
+        key: &str,
+    ) -> Result<(), Report<TrustedServerError>> {
         let path = format!("/resources/stores/config/{}/item/{}", store_id, key);
 
         let mut response = self.make_request("DELETE", &path, None, "application/json")?;
@@ -269,22 +280,24 @@ impl FastlyApiClient {
         response
             .get_body_mut()
             .read_to_string(&mut buf)
-            .map_err(|e| TrustedServerError::Configuration {
-                message: format!("Failed to read API response: {}", e),
+            .map_err(|e| {
+                Report::new(TrustedServerError::Configuration {
+                    message: format!("Failed to read API response: {}", e),
+                })
             })?;
 
         if response.get_status() == StatusCode::OK
             || response.get_status() == StatusCode::NO_CONTENT
         {
             Ok(())
         } else {
-            Err(TrustedServerError::Configuration {
+            Err(Report::new(TrustedServerError::Configuration {
                 message: format!(
                     "Failed to delete config item: HTTP {} - {}",
                     response.get_status(),
                     buf
                 ),
-            })
+            }))
         }
     }
 
@@ -297,7 +310,7 @@ impl FastlyApiClient {
         &self,
         store_id: &str,
         secret_name: &str,
-    ) -> Result<(), TrustedServerError> {
+    ) -> Result<(), Report<TrustedServerError>> {
         let path = format!(
             "/resources/stores/secret/{}/secrets/{}",
             store_id, secret_name
@@ -309,22 +322,24 @@ impl FastlyApiClient {
         response
             .get_body_mut()
             .read_to_string(&mut buf)
-            .map_err(|e| TrustedServerError::Configuration {
-                message: format!("Failed to read API response: {}", e),
+            .map_err(|e| {
+                Report::new(TrustedServerError::Configuration {
+                    message: format!("Failed to read API response: {}", e),
+                })
             })?;
 
         if response.get_status() == StatusCode::OK
             || response.get_status() == StatusCode::NO_CONTENT
         {
             Ok(())
         } else {
-            Err(TrustedServerError::Configuration {
+            Err(Report::new(TrustedServerError::Configuration {
                 message: format!(
                     "Failed to delete secret: HTTP {} - {}",
                     response.get_status(),
                     buf
                 ),
-            })
+            }))
         }
     }
 }
@@ -381,6 +396,7 @@ mod tests {
         }
     }
 
+    // Other tests logic is preserved, prints error which is now a Report
     #[test]
     fn test_update_config_item() {
         let result = FastlyApiClient::new();

crates/common/src/request_signing/jwks.rs

@@ -4,6 +4,7 @@
 //! Ed25519 keypairs in JWK format for request signing.
 
 use ed25519_dalek::{SigningKey, VerifyingKey};
+use error_stack::{Report, ResultExt};
 use jose_jwk::{
     jose_jwa::{Algorithm, Signing},
     Jwk, Key, Okp, OkpCurves, Parameters,
@@ -58,9 +59,11 @@ impl Keypair {
 /// # Errors
 ///
 /// Returns an error if the config store cannot be accessed or if active keys cannot be retrieved.
-pub fn get_active_jwks() -> Result<String, TrustedServerError> {
+pub fn get_active_jwks() -> Result<String, Report<TrustedServerError>> {
     let store = FastlyConfigStore::new("jwks_store");
-    let active_kids_str = store.get("active-kids")?;
+    let active_kids_str = store
+        .get("active-kids")
+        .attach("while fetching active kids list")?;
 
     let active_kids: Vec<&str> = active_kids_str
         .split(',')
@@ -70,7 +73,9 @@ pub fn get_active_jwks() -> Result<String, TrustedServerError> {
 
     let mut jwks = Vec::new();
     for kid in active_kids {
-        let jwk = store.get(kid)?;
+        let jwk = store
+            .get(kid)
+            .attach(format!("Failed to get JWK for kid: {}", kid))?;
         jwks.push(jwk);
     }