diff --git a/.gitignore b/.gitignore index 446a966cc..7de123256 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,8 @@ target/ # option (not recommended) you can uncomment the following to ignore the entire idea folder. #.idea/ .DS_Store +.serena +.codegraph config.json diff --git a/Cargo.lock b/Cargo.lock index 063784523..51d496f0d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5703,6 +5703,8 @@ dependencies = [ "aws-config", "aws-sdk-kms", "aws-sdk-sqs", + "aws-smithy-runtime-api", + "aws-smithy-types", "base64 0.22.1", "bincode", "bs58", diff --git a/Cargo.toml b/Cargo.toml index 6464f2d15..380789b31 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,8 @@ panic = "unwind" aws-config = { version = "1.6.3", features = ["behavior-version-latest"] } aws-sdk-kms = "1.91.0" aws-sdk-sqs = "1.56.0" +aws-smithy-types = "1" +aws-smithy-runtime-api = "1" actix-web = "4" tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter", "json"] } diff --git a/src/queues/sqs/backend.rs b/src/queues/sqs/backend.rs index d7c84fe50..60817a598 100644 --- a/src/queues/sqs/backend.rs +++ b/src/queues/sqs/backend.rs @@ -21,6 +21,7 @@ use crate::{ }, models::{DefaultAppState, NetworkType}, queues::QueueBackendType, + utils::{aws_error::DisplayErrorContext, classify_sdk_error}, }; use actix_web::web::ThinData; @@ -444,8 +445,13 @@ impl SqsBackend { } let response = request.send().await.map_err(|e| { - error!(error = %e, queue_url = %queue_url, "Failed to send message to SQS"); - QueueBackendError::SqsError(format!("SendMessage failed: {e}")) + error!( + error.kind = classify_sdk_error(&e), + error.detail = %DisplayErrorContext(&e), + queue_url = %queue_url, + "Failed to send message to SQS" + ); + QueueBackendError::SqsError(format!("SendMessage failed: {}", classify_sdk_error(&e))) })?; let message_id = response @@ -501,7 +507,12 @@ impl SqsBackend { match sqs_client.get_queue_url().queue_name(dlq_name).send().await { Ok(output) => output.queue_url().map(str::to_string), Err(err) => { - warn!(error = %err, dlq_name = %dlq_name, "Failed to resolve DLQ URL at startup"); + warn!( + error.kind = classify_sdk_error(&err), + error.detail = %DisplayErrorContext(&err), + dlq_name = %dlq_name, + "Failed to resolve DLQ URL at startup" + ); None } } @@ -532,7 +543,12 @@ impl SqsBackend { .and_then(|value| value.parse::().ok()) .unwrap_or(0), Err(err) => { - warn!(error = %err, dlq_url = %dlq_url, "Failed to fetch DLQ depth"); + warn!( + error.kind = classify_sdk_error(&err), + error.detail = %DisplayErrorContext(&err), + dlq_url = %dlq_url, + "Failed to fetch DLQ depth" + ); 0 } } diff --git a/src/queues/sqs/worker.rs b/src/queues/sqs/worker.rs index 5bf272b90..3e65e11d9 100644 --- a/src/queues/sqs/worker.rs +++ b/src/queues/sqs/worker.rs @@ -28,6 +28,7 @@ use crate::{ Job, NotificationSend, RelayerHealthCheck, TokenSwapRequest, TransactionRequest, TransactionSend, TransactionStatusCheck, }, + utils::{aws_error::DisplayErrorContext, classify_sdk_error}, }; use super::{HandlerError, WorkerContext}; @@ -341,23 +342,17 @@ async fn run_poll_loop( Err(e) => { consecutive_poll_errors = consecutive_poll_errors.saturating_add(1); let backoff_secs = poll_error_backoff_secs(consecutive_poll_errors); - let (error_kind, error_code, error_message) = match &e { - SdkError::ServiceError(ctx) => { - ("service", ctx.err().code(), ctx.err().message()) - } - SdkError::DispatchFailure(_) => ("dispatch", None, None), - SdkError::ResponseError(_) => ("response", None, None), - SdkError::TimeoutError(_) => ("timeout", None, None), - _ => ("other", None, None), + let (error_code, error_message) = match &e { + SdkError::ServiceError(ctx) => (ctx.err().code(), ctx.err().message()), + _ => (None, None), }; error!( queue_type = ?queue_type, poller_id = poller_id, - error_kind = error_kind, + error.kind = classify_sdk_error(&e), + error.detail = %DisplayErrorContext(&e), error_code = error_code.unwrap_or("unknown"), error_message = error_message.unwrap_or("n/a"), - error = %e, - error_debug = ?e, consecutive_errors = consecutive_poll_errors, backoff_secs = backoff_secs, "Failed to receive messages from SQS, backing off" @@ -671,7 +666,8 @@ async fn process_message( { error!( queue_type = ?queue_type, - error = %send_err, + error.kind = classify_sdk_error(&send_err), + error.detail = %DisplayErrorContext(&send_err), "Failed to re-enqueue message; leaving original for visibility timeout retry" ); // Fall through — original message will retry after visibility timeout @@ -771,8 +767,15 @@ async fn defer_message( .send() .await .map_err(|e| { + error!( + error.kind = classify_sdk_error(&e), + error.detail = %DisplayErrorContext(&e), + queue_url = %queue_url, + "Failed to defer FIFO message via visibility timeout" + ); QueueBackendError::SqsError(format!( - "Failed to defer FIFO message via visibility timeout: {e}" + "Failed to defer FIFO message via visibility timeout: {}", + classify_sdk_error(&e) )) })?; @@ -800,7 +803,16 @@ async fn defer_message( ); request.send().await.map_err(|e| { - QueueBackendError::SqsError(format!("Failed to defer scheduled message: {e}")) + error!( + error.kind = classify_sdk_error(&e), + error.detail = %DisplayErrorContext(&e), + queue_url = %queue_url, + "Failed to defer scheduled message" + ); + QueueBackendError::SqsError(format!( + "Failed to defer scheduled message: {}", + classify_sdk_error(&e) + )) })?; Ok(true) @@ -965,7 +977,8 @@ async fn flush_delete_batch( Err(e) => { error!( queue_type = ?queue_type, - error = %e, + error.kind = classify_sdk_error(&e), + error.detail = %DisplayErrorContext(&e), batch_size = chunk.len(), "Batch delete API call failed (messages will be redelivered)" ); diff --git a/src/services/aws_kms/mod.rs b/src/services/aws_kms/mod.rs index 968da9336..cdf83d3ba 100644 --- a/src/services/aws_kms/mod.rs +++ b/src/services/aws_kms/mod.rs @@ -49,11 +49,11 @@ use crate::{ client_cache::AsyncClientCache, signer::evm::utils::recover_evm_signature_from_der, }, utils::{ - self, derive_ethereum_address_from_der, derive_solana_address_from_der, - derive_stellar_address_from_der, + self, aws_error::DisplayErrorContext, classify_sdk_error, derive_ethereum_address_from_der, + derive_solana_address_from_der, derive_stellar_address_from_der, }, }; -use tracing::debug; +use tracing::{debug, warn}; #[cfg(test)] use mockall::{automock, mock}; @@ -68,8 +68,6 @@ pub enum AwsKmsError { GetError(String), #[error("AWS KMS signing error: {0}")] SignError(String), - #[error("AWS KMS permissions error: {0}")] - PermissionError(String), #[error("AWS KMS public key error: {0}")] RecoveryError(#[from] utils::Secp256k1Error), #[error("AWS KMS conversion error: {0}")] @@ -280,8 +278,16 @@ impl AwsKmsK256 for AwsKmsClient { .send() .await .map_err(|e| { + warn!( + error.kind = classify_sdk_error(&e), + error.detail = %DisplayErrorContext(&e), + kms_key_id = %key_id, + operation = "get_public_key_secp256k1", + "AWS KMS get_public_key failed" + ); AwsKmsError::GetError(format!( - "Failed to get secp256k1 public key for key '{key_id}': {e:?}" + "Failed to get secp256k1 public key for key '{key_id}': {}", + classify_sdk_error(&e) )) })?; @@ -316,7 +322,19 @@ impl AwsKmsK256 for AwsKmsClient { // Process the result, extract DER signature let der_signature = sign_result - .map_err(|e| AwsKmsError::PermissionError(e.to_string()))? + .map_err(|e| { + warn!( + error.kind = classify_sdk_error(&e), + error.detail = %DisplayErrorContext(&e), + kms_key_id = %key_id, + operation = "sign_digest_secp256k1", + "AWS KMS sign failed" + ); + AwsKmsError::SignError(format!( + "Failed to sign secp256k1 digest for key '{key_id}': {}", + classify_sdk_error(&e) + )) + })? .signature .ok_or(AwsKmsError::SignError( "Signature not found in response".to_string(), @@ -347,8 +365,16 @@ impl AwsKmsEd25519 for AwsKmsClient { .send() .await .map_err(|e| { + warn!( + error.kind = classify_sdk_error(&e), + error.detail = %DisplayErrorContext(&e), + kms_key_id = %key_id, + operation = "get_public_key_ed25519", + "AWS KMS get_public_key failed" + ); AwsKmsError::GetError(format!( - "Failed to get Ed25519 public key for key '{key_id}': {e:?}" + "Failed to get Ed25519 public key for key '{key_id}': {}", + classify_sdk_error(&e) )) })?; @@ -386,7 +412,19 @@ impl AwsKmsEd25519 for AwsKmsClient { // Process the result, extract signature let signature = sign_result - .map_err(|e| AwsKmsError::SignError(e.to_string()))? + .map_err(|e| { + warn!( + error.kind = classify_sdk_error(&e), + error.detail = %DisplayErrorContext(&e), + kms_key_id = %key_id, + operation = "sign_ed25519", + "AWS KMS sign failed" + ); + AwsKmsError::SignError(format!( + "Failed to sign Ed25519 message for key '{key_id}': {}", + classify_sdk_error(&e) + )) + })? .signature .ok_or(AwsKmsError::SignError( "Signature not found in response".to_string(), diff --git a/src/utils/aws_error.rs b/src/utils/aws_error.rs new file mode 100644 index 000000000..c2cf4ba62 --- /dev/null +++ b/src/utils/aws_error.rs @@ -0,0 +1,199 @@ +//! Helpers for diagnosing AWS SDK errors. +//! +//! `SdkError`'s `Display` impl collapses everything below the SDK +//! (DNS, TCP, TLS, connector pool, credential providers) into a single +//! short string like `"dispatch failure"`, which makes prod logs nearly +//! useless for distinguishing root causes. +//! +//! This module provides two utilities meant to be paired at every AWS SDK +//! call-site: +//! +//! * [`classify_sdk_error`] — returns a stable, low-cardinality `&'static str` +//! suitable for a `tracing` field or metric label, distinguishing the +//! actionable subcategories of `DispatchFailure` (timeout / io / user / +//! other) from `TimeoutError`, `ServiceError`, etc. +//! * [`DisplayErrorContext`] — re-export of the SDK's own helper that walks +//! the full `std::error::Error::source()` chain so the underlying cause +//! (e.g., `connect timed out`, `dns error: failed to lookup address`) +//! appears in the log instead of just the top-level wrapper. +//! +//! # Caution: log-only — do not embed in returned error values +//! +//! `DisplayErrorContext` walks the underlying SDK chain and can surface +//! internal infrastructure details (endpoint URLs, connector kinds, +//! credential-provider failures). Keep it confined to `tracing` fields +//! and metrics. For error values returned to upstream callers — which +//! ultimately reach API clients via `ApiError::InternalError(err.to_string())` — +//! prefer the stable kind tag from [`classify_sdk_error`]. +//! +//! Typical usage: +//! +//! ```ignore +//! tracing::error!( +//! error.kind = classify_sdk_error(&err), +//! error.detail = %DisplayErrorContext(&err), +//! "AWS call failed" +//! ); +//! // Returned error value carries only the kind tag, not the full chain: +//! return Err(MyError::Wrapped(format!("op X failed: {}", classify_sdk_error(&err)))); +//! ``` + +pub use aws_smithy_types::error::display::DisplayErrorContext; + +use aws_smithy_runtime_api::client::result::SdkError; + +/// Classify an [`SdkError`] into a stable, low-cardinality kind tag. +/// +/// `DispatchFailure` is split by its underlying [`ConnectorError`] kind so +/// log aggregators can distinguish a `dispatch_timeout` (likely runtime +/// starvation or a slow upstream) from a `dispatch_io` (connection reset, +/// pool exhaustion) without parsing free-form strings. +/// +/// [`ConnectorError`]: aws_smithy_runtime_api::client::result::ConnectorError +pub fn classify_sdk_error(err: &SdkError) -> &'static str { + match err { + SdkError::ConstructionFailure(_) => "construction", + SdkError::TimeoutError(_) => "timeout", + SdkError::DispatchFailure(inner) => match inner.as_connector_error() { + Some(ce) if ce.is_timeout() => "dispatch_timeout", + Some(ce) if ce.is_io() => "dispatch_io", + Some(ce) if ce.is_user() => "dispatch_user", + Some(_) => "dispatch_other", + None => "dispatch_unknown", + }, + SdkError::ResponseError(_) => "response_parse", + SdkError::ServiceError(_) => "service", + // SdkError is `#[non_exhaustive]`; future variants get a stable label. + _ => "unknown", + } +} + +#[cfg(test)] +mod tests { + use super::*; + use aws_smithy_runtime_api::client::orchestrator::HttpResponse; + use aws_smithy_runtime_api::client::result::ConnectorError; + use std::convert::Infallible; + use std::io; + + // SdkError — E is the operation error type, R is the response type. + // ConstructionFailure / TimeoutError / DispatchFailure don't actually carry + // an E, so Infallible is the cheapest stand-in. R is supplied explicitly + // because aws-smithy-runtime-api 1.11+ no longer defaults it. + type TestErr = SdkError; + + fn boxed(msg: &str) -> Box { + Box::new(io::Error::other(msg.to_string())) + } + + #[test] + fn classifies_construction_failure() { + let err: TestErr = SdkError::construction_failure(boxed("could not build request")); + assert_eq!(classify_sdk_error(&err), "construction"); + } + + #[test] + fn classifies_timeout_error() { + let err: TestErr = SdkError::timeout_error(boxed("operation timed out")); + assert_eq!(classify_sdk_error(&err), "timeout"); + } + + #[test] + fn classifies_dispatch_failure_timeout() { + // Connector-level timeout is the most likely shape under runtime + // saturation; this kind tag is what should drive the "we're starving + // the AWS SDK connector futures" diagnosis. + let err: TestErr = + SdkError::dispatch_failure(ConnectorError::timeout(boxed("connect timed out"))); + assert_eq!(classify_sdk_error(&err), "dispatch_timeout"); + } + + #[test] + fn classifies_dispatch_failure_io() { + let err: TestErr = + SdkError::dispatch_failure(ConnectorError::io(boxed("connection reset by peer"))); + assert_eq!(classify_sdk_error(&err), "dispatch_io"); + } + + #[test] + fn classifies_dispatch_failure_user() { + let err: TestErr = + SdkError::dispatch_failure(ConnectorError::user(boxed("invalid endpoint URL"))); + assert_eq!(classify_sdk_error(&err), "dispatch_user"); + } + + #[test] + fn classifies_dispatch_failure_other() { + let err: TestErr = + SdkError::dispatch_failure(ConnectorError::other(boxed("unexpected"), None)); + assert_eq!(classify_sdk_error(&err), "dispatch_other"); + } + + /// Sensitive marker that mimics the kind of content `DisplayErrorContext` + /// can surface via the SDK error source chain (endpoint URL, connector + /// internals, credential-provider failures). Tests below assert this + /// marker never leaks into strings produced by the call-site format. + const SENSITIVE_MARKER: &str = "https://kms.us-east-1.amazonaws.com/internal-endpoint"; + + fn dispatch_timeout_with_sensitive_chain() -> TestErr { + let inner = io::Error::new( + io::ErrorKind::TimedOut, + format!("connect timed out to {SENSITIVE_MARKER}"), + ); + SdkError::dispatch_failure(ConnectorError::timeout(Box::new(inner))) + } + + // The pattern every AWS call-site uses for the *returned* error value: + // format!("op X failed for key 'Y': {}", classify_sdk_error(&e)) + // This contract test pins that the bounded form never embeds the source + // chain — which is the whole security argument behind the split between + // DisplayErrorContext (log-only) and classify_sdk_error (return-safe). + #[test] + fn returned_error_string_is_bounded_to_kind_tag() { + let err = dispatch_timeout_with_sensitive_chain(); + let returned = format!( + "Failed to sign secp256k1 digest for key 'alias/test-key': {}", + classify_sdk_error(&err) + ); + assert!( + returned.contains("dispatch_timeout"), + "returned error should carry the kind tag; got: {returned}" + ); + assert!( + !returned.contains(SENSITIVE_MARKER), + "returned error must not leak the source chain; got: {returned}" + ); + // Also pin against DisplayErrorContext accidentally creeping in: it + // would surface phrases like "connect timed out" from the inner error. + assert!( + !returned.contains("connect timed out"), + "returned error must not embed inner cause text; got: {returned}" + ); + } + + // Counterpart: DisplayErrorContext is *expected* to surface the chain — + // that's why it's log-only. This pins both halves of the contract. + #[test] + fn display_error_context_does_surface_sensitive_chain() { + let err = dispatch_timeout_with_sensitive_chain(); + let rendered = format!("{}", DisplayErrorContext(&err)); + assert!( + rendered.contains(SENSITIVE_MARKER), + "DisplayErrorContext must surface the chain for ops logs; got: {rendered}" + ); + } + + #[test] + fn display_error_context_surfaces_underlying_cause() { + // Re-pins the behaviour the helper relies on: DisplayErrorContext must + // walk source() chains, otherwise the prod logs would still collapse to + // "dispatch failure" and we'd be back where we started. + let inner = io::Error::new(io::ErrorKind::TimedOut, "tcp connect timed out at layer 4"); + let err: TestErr = SdkError::dispatch_failure(ConnectorError::timeout(Box::new(inner))); + let rendered = format!("{}", DisplayErrorContext(&err)); + assert!( + rendered.contains("tcp connect timed out at layer 4"), + "DisplayErrorContext should expose the inner cause; got: {rendered}" + ); + } +} diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 0e2ced380..7b35f49f6 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -55,6 +55,14 @@ pub use url_security::*; mod error_sanitization; pub use error_sanitization::*; +pub mod aws_error; +// `classify_sdk_error` returns a stable kind tag and is safe to embed in +// returned errors, so we re-export it at `utils::`. `DisplayErrorContext` +// is intentionally NOT re-exported here — it can leak source-chain details +// and is log-only; callers must import it via the fully qualified +// `utils::aws_error::DisplayErrorContext` path to keep the misuse risk visible. +pub use aws_error::classify_sdk_error; + mod url; pub use url::*;