From 565e3953507b0305b116d2fa01ef0f2c21ae6574 Mon Sep 17 00:00:00 2001 From: RA <70325462+RAprogramm@users.noreply.github.com> Date: Wed, 24 Sep 2025 07:10:12 +0700 Subject: [PATCH] Finalize context error migration --- CHANGELOG.md | 13 + Cargo.lock | 2 +- Cargo.toml | 2 +- README.md | 14 +- src/code/app_code.rs | 6 + src/convert/config.rs | 89 ++++- src/convert/multipart.rs | 23 +- src/convert/redis.rs | 111 +++++- src/convert/reqwest.rs | 149 +++++-- src/convert/serde_json.rs | 56 ++- src/convert/sqlx.rs | 373 ++++++++++++++++-- src/convert/telegram_webapp_sdk.rs | 48 ++- src/convert/teloxide.rs | 97 +++-- src/convert/tokio.rs | 27 +- src/convert/validator.rs | 80 +++- src/response/problem_json.rs | 12 + .../fail/enum_missing_variant.stderr | 7 +- tests/ui/app_error/fail/missing_code.stderr | 4 +- tests/ui/app_error/fail/missing_kind.stderr | 2 +- tests/ui/formatter/fail/duplicate_fmt.stderr | 2 +- .../fail/implicit_after_named.stderr | 1 - .../ui/formatter/fail/unsupported_flag.stderr | 4 +- .../fail/unsupported_formatter.stderr | 4 +- .../ui/formatter/fail/uppercase_binary.stderr | 4 +- .../formatter/fail/uppercase_pointer.stderr | 4 +- tests/ui/masterror/fail/duplicate_attr.stderr | 2 +- .../masterror/fail/duplicate_telemetry.stderr | 2 +- tests/ui/masterror/fail/empty_redact.stderr | 2 +- .../fail/enum_missing_variant.stderr | 7 +- .../ui/masterror/fail/missing_category.stderr | 4 +- tests/ui/masterror/fail/missing_code.stderr | 4 +- tests/ui/masterror/fail/unknown_option.stderr | 2 +- 32 files changed, 969 insertions(+), 188 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c341449..78e6ad4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,19 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +## [0.18.0] - 2025-09-28 + +### Added +- Added the `AppCode::UserAlreadyExists` classification and mapped it to RFC7807 + responses with the appropriate retry hint. + +### Changed +- Switched all integration converters in `src/convert/*` to build structured + `Context` metadata before producing `Error` values, including HTTP status, + operation, endpoint, duration and retry/edit flags. +- Extended integration tests to validate the enriched metadata, retry behavior + and error code/category mappings across the updated converters. + ## [0.17.0] - 2025-09-27 ### Added diff --git a/Cargo.lock b/Cargo.lock index dc61df7..2181e27 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1727,7 +1727,7 @@ dependencies = [ [[package]] name = "masterror" -version = "0.17.0" +version = "0.18.0" dependencies = [ "actix-web", "axum 0.8.4", diff --git a/Cargo.toml b/Cargo.toml index 3a8f8a7..55b022a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "masterror" -version = "0.17.0" +version = "0.18.0" rust-version = "1.90" edition = "2024" license = "MIT OR Apache-2.0" diff --git a/README.md b/README.md index 278be85..472f547 100644 --- a/README.md +++ b/README.md @@ -38,9 +38,9 @@ guides, comparisons with `thiserror`/`anyhow`, and troubleshooting recipes. ~~~toml [dependencies] -masterror = { version = "0.17.0", default-features = false } +masterror = { version = "0.18.0", default-features = false } # or with features: -# masterror = { version = "0.17.0", features = [ +# masterror = { version = "0.18.0", features = [ # "axum", "actix", "openapi", "serde_json", # "tracing", "metrics", "backtrace", "sqlx", # "sqlx-migrate", "reqwest", "redis", "validator", @@ -78,10 +78,10 @@ masterror = { version = "0.17.0", default-features = false } ~~~toml [dependencies] # lean core -masterror = { version = "0.17.0", default-features = false } +masterror = { version = "0.18.0", default-features = false } # with Axum/Actix + JSON + integrations -# masterror = { version = "0.17.0", features = [ +# masterror = { version = "0.18.0", features = [ # "axum", "actix", "openapi", "serde_json", # "tracing", "metrics", "backtrace", "sqlx", # "sqlx-migrate", "reqwest", "redis", "validator", @@ -720,13 +720,13 @@ assert_eq!(problem.grpc.expect("grpc").name, "UNAUTHENTICATED"); Minimal core: ~~~toml -masterror = { version = "0.17.0", default-features = false } +masterror = { version = "0.18.0", default-features = false } ~~~ API (Axum + JSON + deps): ~~~toml -masterror = { version = "0.17.0", features = [ +masterror = { version = "0.18.0", features = [ "axum", "serde_json", "openapi", "sqlx", "reqwest", "redis", "validator", "config", "tokio" ] } @@ -735,7 +735,7 @@ masterror = { version = "0.17.0", features = [ API (Actix + JSON + deps): ~~~toml -masterror = { version = "0.17.0", features = [ +masterror = { version = "0.18.0", features = [ "actix", "serde_json", "openapi", "sqlx", "reqwest", "redis", "validator", "config", "tokio" ] } diff --git a/src/code/app_code.rs b/src/code/app_code.rs index 11bf384..b46d699 100644 --- a/src/code/app_code.rs +++ b/src/code/app_code.rs @@ -36,6 +36,11 @@ pub enum AppCode { /// Typically mapped to HTTP **409 Conflict**. Conflict, + /// Attempted to create a user that already exists (unique constraint). + /// + /// Typically mapped to HTTP **409 Conflict**. + UserAlreadyExists, + /// Authentication required or failed (missing/invalid credentials). /// /// Typically mapped to HTTP **401 Unauthorized**. @@ -149,6 +154,7 @@ impl AppCode { AppCode::NotFound => "NOT_FOUND", AppCode::Validation => "VALIDATION", AppCode::Conflict => "CONFLICT", + AppCode::UserAlreadyExists => "USER_ALREADY_EXISTS", AppCode::Unauthorized => "UNAUTHORIZED", AppCode::Forbidden => "FORBIDDEN", AppCode::NotImplemented => "NOT_IMPLEMENTED", diff --git a/src/convert/config.rs b/src/convert/config.rs index e88ae5a..d3ef335 100644 --- a/src/convert/config.rs +++ b/src/convert/config.rs @@ -1,4 +1,4 @@ -//! Convert [`config::ConfigError`] into [`AppError`], +//! Convert [`config::ConfigError`] into [`Error`], //! producing [`AppErrorKind::Config`]. //! //! Enabled with the `config` feature. @@ -7,23 +7,90 @@ //! //! ```rust,ignore //! use config::ConfigError; -//! use masterror::{AppError, AppErrorKind}; +//! use masterror::{AppErrorKind, Error}; //! //! let err = ConfigError::Message("missing key".into()); -//! let app_err: AppError = err.into(); +//! let app_err: Error = err.into(); //! assert!(matches!(app_err.kind, AppErrorKind::Config)); //! ``` #[cfg(feature = "config")] use config::ConfigError; #[cfg(feature = "config")] -use crate::AppError; +use crate::{ + AppErrorKind, + app_error::{Context, Error, field} +}; #[cfg(feature = "config")] #[cfg_attr(docsrs, doc(cfg(feature = "config")))] -impl From for AppError { +impl From for Error { fn from(err: ConfigError) -> Self { - AppError::config(err.to_string()) + build_context(&err).into_error(err) + } +} + +#[cfg(feature = "config")] +fn build_context(error: &ConfigError) -> Context { + match error { + ConfigError::Frozen => { + Context::new(AppErrorKind::Config).with(field::str("config.phase", "frozen")) + } + ConfigError::NotFound(key) => Context::new(AppErrorKind::Config) + .with(field::str("config.phase", "not_found")) + .with(field::str("config.key", key.clone())), + ConfigError::PathParse { + .. + } => Context::new(AppErrorKind::Config).with(field::str("config.phase", "path_parse")), + ConfigError::FileParse { + uri, .. + } => { + let mut ctx = + Context::new(AppErrorKind::Config).with(field::str("config.phase", "file_parse")); + if let Some(path) = uri { + ctx = ctx.with(field::str("config.uri", path.clone())); + } + ctx + } + ConfigError::Type { + origin, + unexpected, + expected, + key + } => { + let mut ctx = Context::new(AppErrorKind::Config) + .with(field::str("config.phase", "type")) + .with(field::str("config.expected", *expected)) + .with(field::str("config.unexpected", unexpected.to_string())); + if let Some(origin) = origin { + ctx = ctx.with(field::str("config.origin", origin.clone())); + } + if let Some(key) = key { + ctx = ctx.with(field::str("config.key", key.clone())); + } + ctx + } + ConfigError::At { + origin, + key, + .. + } => { + let mut ctx = + Context::new(AppErrorKind::Config).with(field::str("config.phase", "at")); + if let Some(origin) = origin { + ctx = ctx.with(field::str("config.origin", origin.clone())); + } + if let Some(key) = key { + ctx = ctx.with(field::str("config.key", key.clone())); + } + ctx + } + ConfigError::Message(message) => Context::new(AppErrorKind::Config) + .with(field::str("config.phase", "message")) + .with(field::str("config.message", message.clone())), + ConfigError::Foreign(_) => { + Context::new(AppErrorKind::Config).with(field::str("config.phase", "foreign")) + } } } @@ -31,12 +98,18 @@ impl From for AppError { mod tests { use config::ConfigError; - use crate::{AppError, AppErrorKind}; + use super::*; + use crate::{AppErrorKind, FieldValue}; #[test] fn maps_to_config_kind() { let err = ConfigError::Message("dummy".into()); - let app_err = AppError::from(err); + let app_err = Error::from(err); assert!(matches!(app_err.kind, AppErrorKind::Config)); + let metadata = app_err.metadata(); + assert_eq!( + metadata.get("config.phase"), + Some(&FieldValue::Str("message".into())) + ); } } diff --git a/src/convert/multipart.rs b/src/convert/multipart.rs index f23750e..212db57 100644 --- a/src/convert/multipart.rs +++ b/src/convert/multipart.rs @@ -1,4 +1,4 @@ -//! Maps [`MultipartError`] into [`AppError`] with +//! Maps [`MultipartError`] into [`Error`] with //! [`AppErrorKind::BadRequest`], preserving the original message. //! //! Intended for Axum multipart form parsing so that client mistakes are @@ -8,11 +8,16 @@ use axum::extract::multipart::MultipartError; -use crate::{AppError, AppErrorKind}; +use crate::{ + AppErrorKind, + app_error::{Context, Error, field} +}; -impl From for AppError { +impl From for Error { fn from(err: MultipartError) -> Self { - AppError::with(AppErrorKind::BadRequest, format!("Multipart error: {err}")) + Context::new(AppErrorKind::BadRequest) + .with(field::str("multipart.reason", err.to_string())) + .into_error(err) } } @@ -24,7 +29,7 @@ mod tests { http::Request }; - use crate::{AppError, AppErrorKind}; + use crate::{AppErrorKind, FieldValue}; #[tokio::test] async fn multipart_error_maps_to_bad_request() { @@ -42,10 +47,12 @@ mod tests { .expect("extractor"); let err = multipart.next_field().await.expect_err("error"); - let expected = format!("Multipart error: {err}"); - let app_err: AppError = err.into(); + let app_err: Error = err.into(); assert_eq!(app_err.kind, AppErrorKind::BadRequest); - assert_eq!(app_err.message.as_deref(), Some(expected.as_str())); + assert_eq!( + app_err.metadata().get("multipart.reason"), + Some(&FieldValue::Str(err.to_string().into())) + ); } } diff --git a/src/convert/redis.rs b/src/convert/redis.rs index 8494ae3..05cc4c3 100644 --- a/src/convert/redis.rs +++ b/src/convert/redis.rs @@ -1,12 +1,14 @@ -//! Conversion from [`redis::RedisError`] into [`AppError`]. +//! Conversion from [`redis::RedisError`] into [`Error`]. //! //! Enabled with the `redis` feature flag. //! //! ## Mapping //! -//! All Redis client errors are mapped to `AppErrorKind::Cache`. -//! The full error string from the driver is preserved in `message` for logs -//! and JSON payloads (if applicable). +//! All Redis client errors are mapped to `AppErrorKind::Cache` by default and +//! enriched with structured metadata (error kind, code, retry hints). Timeout +//! and infrastructure-level failures are promoted to `Timeout` or +//! `DependencyUnavailable` respectively. Metadata captures cluster redirects, +//! retry strategy and low-level flags without exposing sensitive payloads. //! //! This categorization treats Redis as a cache infrastructure dependency. //! If you need a different taxonomy (e.g. distinguishing caches from queues), @@ -16,10 +18,10 @@ //! ## Example //! //! ```rust,ignore -//! use masterror::{AppError, AppErrorKind}; +//! use masterror::{AppErrorKind, Error}; //! use redis::RedisError; //! -//! fn handle_cache_error(e: RedisError) -> AppError { +//! fn handle_cache_error(e: RedisError) -> Error { //! e.into() //! } //! @@ -31,10 +33,13 @@ //! ``` #[cfg(feature = "redis")] -use redis::RedisError; +use redis::{RedisError, RetryMethod}; #[cfg(feature = "redis")] -use crate::AppError; +use crate::{ + AppErrorKind, + app_error::{Context, Error, field} +}; /// Map any [`redis::RedisError`] into an [`AppError`] with kind `Cache`. /// @@ -42,10 +47,75 @@ use crate::AppError; /// Detailed driver errors are kept in the message for diagnostics. #[cfg(feature = "redis")] #[cfg_attr(docsrs, doc(cfg(feature = "redis")))] -impl From for AppError { +impl From for Error { fn from(err: RedisError) -> Self { - // Infrastructure cache issue -> cache-level error - AppError::cache(format!("Redis error: {err}")) + let (context, retry_after) = build_context(&err); + let mut error = context.into_error(err); + if let Some(secs) = retry_after { + error = error.with_retry_after_secs(secs); + } + error + } +} + +#[cfg(feature = "redis")] +fn build_context(err: &RedisError) -> (Context, Option) { + let mut context = Context::new(AppErrorKind::Cache) + .with(field::str("redis.kind", format!("{:?}", err.kind()))) + .with(field::str("redis.category", err.category())) + .with(field::bool("redis.is_timeout", err.is_timeout())) + .with(field::bool( + "redis.is_cluster_error", + err.is_cluster_error() + )) + .with(field::bool( + "redis.is_connection_refused", + err.is_connection_refusal() + )) + .with(field::bool( + "redis.is_connection_dropped", + err.is_connection_dropped() + )); + + if let Some(code) = err.code() { + context = context.with(field::str("redis.code", code.to_owned())); + } + + if err.is_timeout() { + context = context.category(AppErrorKind::Timeout); + } else if err.is_connection_refusal() + || err.is_connection_dropped() + || err.is_cluster_error() + || err.is_io_error() + { + context = context.category(AppErrorKind::DependencyUnavailable); + } + + if let Some((addr, slot)) = err.redirect_node() { + context = context + .with(field::str("redis.redirect_addr", addr.to_owned())) + .with(field::u64("redis.redirect_slot", u64::from(slot))); + } + + let retry_method = err.retry_method(); + let retry_after = retry_after_hint(retry_method); + context = context.with(field::str( + "redis.retry_method", + format!("{:?}", retry_method) + )); + + (context, retry_after) +} + +#[cfg(feature = "redis")] +const fn retry_after_hint(method: RetryMethod) -> Option { + match method { + RetryMethod::NoRetry => None, + RetryMethod::RetryImmediately | RetryMethod::AskRedirect | RetryMethod::MovedRedirect => { + Some(0) + } + RetryMethod::Reconnect | RetryMethod::ReconnectFromInitialConnections => Some(1), + RetryMethod::WaitAndRetry => Some(2) } } @@ -54,12 +124,25 @@ mod tests { use redis::ErrorKind; use super::*; - use crate::AppErrorKind; + use crate::{AppErrorKind, FieldValue}; #[test] fn maps_to_cache_kind() { let redis_err = RedisError::from((ErrorKind::IoError, "boom")); - let app_err: AppError = redis_err.into(); - assert!(matches!(app_err.kind, AppErrorKind::Cache)); + let app_err: Error = redis_err.into(); + assert!(matches!(app_err.kind, AppErrorKind::DependencyUnavailable)); + let metadata = app_err.metadata(); + assert_eq!( + metadata.get("redis.kind"), + Some(&FieldValue::Str("IoError".into())) + ); + } + + #[test] + fn busy_loading_sets_retry_hint() { + let err = RedisError::from((ErrorKind::BusyLoadingError, "loading")); + let app_err: Error = err.into(); + assert_eq!(app_err.retry.map(|r| r.after_seconds), Some(2)); + assert!(matches!(app_err.kind, AppErrorKind::DependencyUnavailable)); } } diff --git a/src/convert/reqwest.rs b/src/convert/reqwest.rs index 19256e2..9eeb29f 100644 --- a/src/convert/reqwest.rs +++ b/src/convert/reqwest.rs @@ -1,4 +1,4 @@ -//! Conversion from [`reqwest::Error`] into [`AppError`]. +//! Conversion from [`reqwest::Error`] into [`Error`]. //! //! Enabled with the `reqwest` feature flag. //! @@ -7,11 +7,16 @@ //! - [`reqwest::Error::is_timeout`] → `AppErrorKind::Timeout` //! - [`reqwest::Error::is_connect`] or [`reqwest::Error::is_request`] → //! `AppErrorKind::Network` -//! - [`reqwest::Error::is_status`] → `AppErrorKind::ExternalApi` (with upstream -//! status info) +//! - HTTP status errors are classified by status family: +//! - `429` → `AppErrorKind::RateLimited` +//! - `5xx` → `AppErrorKind::DependencyUnavailable` +//! - `408` → `AppErrorKind::Timeout` +//! - others → `AppErrorKind::ExternalApi` //! - All other cases → `AppErrorKind::ExternalApi` //! -//! The original error string is preserved in the `message` for observability. +//! Structured metadata captures the upstream endpoint, status code and +//! low-level flags (timeout/connect/request). Potentially sensitive data (URL) +//! is marked for hashing/redaction in public payloads. //! //! ## Rationale //! @@ -22,10 +27,10 @@ //! ## Example //! //! ```rust,ignore -//! use masterror::{AppError, AppErrorKind}; +//! use masterror::{AppErrorKind, Error}; //! use reqwest::Error as ReqwestError; //! -//! fn handle_http_error(e: ReqwestError) -> AppError { +//! fn handle_http_error(e: ReqwestError) -> Error { //! e.into() //! } //! @@ -40,12 +45,13 @@ //! ``` #[cfg(feature = "reqwest")] -use reqwest::Error as ReqwestError; +use reqwest::{Error as ReqwestError, StatusCode}; +use crate::AppErrorKind; #[cfg(feature = "reqwest")] -use crate::AppError; +use crate::app_error::{Context, Error, FieldRedaction, field}; -/// Map a [`reqwest::Error`] into an [`AppError`] according to its category. +/// Map a [`reqwest::Error`] into an [`Error`] according to its category. /// /// - Timeout → `Timeout` /// - Connect or request build error → `Network` @@ -53,20 +59,77 @@ use crate::AppError; /// - Fallback for other cases → `ExternalApi` #[cfg(feature = "reqwest")] #[cfg_attr(docsrs, doc(cfg(feature = "reqwest")))] -impl From for AppError { +impl From for Error { fn from(err: ReqwestError) -> Self { - if err.is_timeout() { - AppError::timeout(format!("Request timeout: {err}")) - } else if err.is_connect() || err.is_request() { - AppError::network(format!("Network error: {err}")) - } else if err.is_status() { - AppError::external_api(format!("Upstream status error: {err}")) - } else { - AppError::external_api(format!("Upstream error: {err}")) + let (context, retry_after) = classify_reqwest_error(&err); + let mut error = context.into_error(err); + if let Some(secs) = retry_after { + error = error.with_retry_after_secs(secs); } + error } } +#[cfg(feature = "reqwest")] +fn classify_reqwest_error(err: &ReqwestError) -> (Context, Option) { + let mut context = Context::new(AppErrorKind::ExternalApi) + .with(field::bool("reqwest.is_timeout", err.is_timeout())) + .with(field::bool("reqwest.is_connect", err.is_connect())) + .with(field::bool("reqwest.is_request", err.is_request())) + .with(field::bool("reqwest.is_status", err.is_status())) + .with(field::bool("reqwest.is_body", err.is_body())) + .with(field::bool("reqwest.is_decode", err.is_decode())) + .with(field::bool("reqwest.is_redirect", err.is_redirect())); + + let mut retry_after = None; + + if err.is_timeout() { + context = context.category(AppErrorKind::Timeout); + } else if err.is_connect() || err.is_request() { + context = context.category(AppErrorKind::Network); + } + + if let Some(status) = err.status() { + let status_code = u16::from(status); + context = context.with(field::u64("http.status", u64::from(status_code))); + if let Some(reason) = status.canonical_reason() { + context = context.with(field::str("http.status_reason", reason)); + } + + context = match status { + StatusCode::TOO_MANY_REQUESTS => { + retry_after = Some(1); + context.category(AppErrorKind::RateLimited) + } + StatusCode::REQUEST_TIMEOUT => context.category(AppErrorKind::Timeout), + s if s.is_server_error() => context.category(AppErrorKind::DependencyUnavailable), + _ => context + }; + } + + if let Some(url) = err.url() { + context = context + .with(field::str("http.url", url.as_str())) + .redact_field("http.url", FieldRedaction::Hash); + + if let Some(host) = url.host_str() { + context = context.with(field::str("http.host", host.to_owned())); + } + + let path = url.path(); + if !path.is_empty() { + context = context.with(field::str("http.path", path.to_owned())); + } + + let scheme = url.scheme(); + if !scheme.is_empty() { + context = context.with(field::str("http.scheme", scheme.to_owned())); + } + } + + (context, retry_after) +} + #[cfg(all(test, feature = "reqwest", feature = "tokio"))] mod tests { use std::time::Duration; @@ -75,9 +138,10 @@ mod tests { use tokio::{net::TcpListener, time::sleep}; use super::*; + use crate::{AppCode, AppErrorKind, FieldRedaction, FieldValue}; #[tokio::test] - async fn timeout_message_includes_original_error() { + async fn timeout_sets_category_and_metadata() { let listener = TcpListener::bind("127.0.0.1:0") .await .expect("bind listener"); @@ -99,15 +163,48 @@ mod tests { .await .expect_err("expected timeout"); - assert!(err.is_timeout()); + let app_err: Error = err.into(); + assert_eq!(app_err.kind, AppErrorKind::Timeout); - let err_str = err.to_string(); - let app_err: AppError = err.into(); - let msg = app_err.message.expect("app error message"); - assert!( - msg.contains(err_str.as_str()), - "{msg} does not contain {err_str}" + let metadata = app_err.metadata(); + assert_eq!( + metadata.get("reqwest.is_timeout"), + Some(&FieldValue::Bool(true)) ); + assert_eq!(metadata.redaction("http.url"), Some(FieldRedaction::Hash)); + + server.abort(); + } + + #[tokio::test] + async fn status_error_maps_retry_and_rate_limit() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let listener = TcpListener::bind("127.0.0.1:0").await.expect("bind"); + let addr = listener.local_addr().expect("addr"); + + let server = tokio::spawn(async move { + let (mut socket, _) = listener.accept().await.expect("accept"); + let mut buf = [0_u8; 1024]; + let _ = socket.read(&mut buf).await; + let response = b"HTTP/1.1 429 Too Many Requests\r\ncontent-length: 0\r\n\r\n"; + let _ = socket.write_all(response).await; + }); + + let client = Client::new(); + let response = client + .get(format!("http://{addr}")) + .send() + .await + .expect("send"); + let err = response.error_for_status().expect_err("status error"); + + let app_err: Error = err.into(); + assert_eq!(app_err.kind, AppErrorKind::RateLimited); + assert_eq!(app_err.code, AppCode::RateLimited); + assert_eq!(app_err.retry.map(|r| r.after_seconds), Some(1)); + let metadata = app_err.metadata(); + assert_eq!(metadata.get("http.status"), Some(&FieldValue::U64(429))); server.abort(); } diff --git a/src/convert/serde_json.rs b/src/convert/serde_json.rs index f0cb308..f307217 100644 --- a/src/convert/serde_json.rs +++ b/src/convert/serde_json.rs @@ -1,4 +1,4 @@ -//! Conversion from [`serde_json::Error`] into [`AppError`]. +//! Conversion from [`serde_json::Error`] into [`Error`]. //! //! Enabled with the `serde_json` feature flag. //! @@ -18,10 +18,10 @@ //! ## Example //! //! ```rust,ignore -//! use masterror::{AppError, AppErrorKind}; +//! use masterror::{AppErrorKind, Error}; //! use serde_json::Error as SjError; //! -//! fn handle_json_error(e: SjError) -> AppError { +//! fn handle_json_error(e: SjError) -> Error { //! e.into() //! } //! @@ -34,7 +34,10 @@ use serde_json::{Error as SjError, error::Category}; #[cfg(feature = "serde_json")] -use crate::AppError; +use crate::{ + AppErrorKind, + app_error::{Context, Error, field} +}; /// Map a [`serde_json::Error`] into an [`AppError`]. /// @@ -43,15 +46,33 @@ use crate::AppError; /// logs and optional JSON payloads. #[cfg(feature = "serde_json")] #[cfg_attr(docsrs, doc(cfg(feature = "serde_json")))] -impl From for AppError { +impl From for Error { fn from(err: SjError) -> Self { - match err.classify() { - Category::Io => AppError::serialization(err.to_string()), - Category::Syntax | Category::Data | Category::Eof => { - AppError::deserialization(err.to_string()) - } + build_context(&err).into_error(err) + } +} + +#[cfg(feature = "serde_json")] +fn build_context(err: &SjError) -> Context { + let category = err.classify(); + let mut context = match category { + Category::Io => Context::new(AppErrorKind::Serialization), + Category::Syntax | Category::Data | Category::Eof => { + Context::new(AppErrorKind::Deserialization) } } + .with(field::str("serde_json.category", format!("{:?}", category))); + + let line = err.line(); + if line != 0 { + context = context.with(field::u64("serde_json.line", u64::from(line))); + } + let column = err.column(); + if column != 0 { + context = context.with(field::u64("serde_json.column", u64::from(column))); + } + + context } #[cfg(test)] @@ -61,7 +82,7 @@ mod tests { use serde_json::json; use super::*; - use crate::kind::AppErrorKind; + use crate::{AppErrorKind, FieldValue}; #[test] fn io_maps_to_serialization() { @@ -78,14 +99,23 @@ mod tests { } let err = serde_json::to_writer(FailWriter, &json!({"k": "v"})).unwrap_err(); - let app: AppError = err.into(); + let app: Error = err.into(); assert!(matches!(app.kind, AppErrorKind::Serialization)); + assert_eq!( + app.metadata().get("serde_json.category"), + Some(&FieldValue::Str("Io".into())) + ); } #[test] fn syntax_maps_to_deserialization() { let err = serde_json::from_str::("not-json").unwrap_err(); - let app: AppError = err.into(); + let app: Error = err.into(); assert!(matches!(app.kind, AppErrorKind::Deserialization)); + let metadata = app.metadata(); + assert_eq!( + metadata.get("serde_json.category"), + Some(&FieldValue::Str("Syntax".into())) + ); } } diff --git a/src/convert/sqlx.rs b/src/convert/sqlx.rs index 89fd254..1fbe506 100644 --- a/src/convert/sqlx.rs +++ b/src/convert/sqlx.rs @@ -1,4 +1,4 @@ -//! Conversions from `sqlx` errors into `AppError`. +//! Conversions from `sqlx` errors into [`Error`]. //! //! Feature flags: //! - `sqlx` → maps `sqlx_core::error::Error` @@ -7,21 +7,25 @@ //! ## Mappings //! //! - `sqlx_core::error::Error::RowNotFound` → `AppErrorKind::NotFound` -//! - any other `sqlx_core::error::Error` → `AppErrorKind::Database` -//! - `sqlx::migrate::MigrateError` → `AppErrorKind::Database` +//! - Database constraint errors capture SQLSTATE/constraint metadata and map to +//! `Conflict`/`Validation` +//! - Transient SQLSTATEs (e.g. `40001`, `55P03`) attach retry hints +//! - `sqlx::migrate::MigrateError` → `AppErrorKind::Database` with migration +//! phase metadata //! -//! The original error message is preserved in `AppError.message` for -//! observability. SQL driver–specific details are **not** mapped to separate -//! kinds to keep the taxonomy stable. +//! Structured metadata includes SQLSTATE codes, constraint names and migration +//! phases to aid observability while keeping secrets out of public payloads. +//! Known SQLSTATE codes override [`AppCode`] (`UNIQUE_VIOLATION` → +//! `USER_ALREADY_EXISTS`). //! //! ## Example //! //! ```rust,ignore //! // Requires: features = ["sqlx"] -//! use masterror::{AppError, AppErrorKind}; +//! use masterror::{AppErrorKind, Error}; //! use sqlx_core::error::Error as SqlxError; //! -//! fn handle_db_error(e: SqlxError) -> AppError { +//! fn handle_db_error(e: SqlxError) -> Error { //! e.into() //! } //! @@ -33,58 +37,365 @@ #[cfg(feature = "sqlx-migrate")] use sqlx::migrate::MigrateError; #[cfg(feature = "sqlx")] -use sqlx_core::error::Error as SqlxError; +use sqlx_core::error::{DatabaseError, Error as SqlxError, ErrorKind as SqlxErrorKind}; #[cfg(any(feature = "sqlx", feature = "sqlx-migrate"))] -use crate::AppError; +use crate::{ + AppCode, AppErrorKind, + app_error::{Context, Error, field} +}; -/// Map a `sqlx_core::error::Error` into an `AppError`. +#[cfg(feature = "sqlx")] +const SQLSTATE_CODE_OVERRIDES: &[(&str, AppCode)] = &[ + ("23505", AppCode::UserAlreadyExists), + ("23503", AppCode::Conflict), + ("23502", AppCode::Validation), + ("23514", AppCode::Validation) +]; + +#[cfg(feature = "sqlx")] +const SQLSTATE_RETRY_HINTS: &[(&str, u64)] = &[("40001", 1), ("55P03", 1)]; + +/// Map a `sqlx_core::error::Error` into [`Error`]. /// /// - `RowNotFound` → `AppErrorKind::NotFound` -/// - all other cases → `AppErrorKind::Database` -/// -/// The database error message is preserved for debugging and log correlation. +/// - database constraint errors attach SQLSTATE and constraint metadata +/// - concurrency SQLSTATEs attach retry hints #[cfg(feature = "sqlx")] #[cfg_attr(docsrs, doc(cfg(feature = "sqlx")))] -impl From for AppError { +impl From for Error { fn from(err: SqlxError) -> Self { - match err { - SqlxError::RowNotFound => AppError::not_found("Record not found"), - other => AppError::database_with_message(other.to_string()) + let (context, retry_after) = build_sqlx_context(&err); + let mut error = context.into_error(err); + if let Some(secs) = retry_after { + error = error.with_retry_after_secs(secs); } + error } } -/// Map a `sqlx::migrate::MigrateError` into an `AppError`. +/// Map a `sqlx::migrate::MigrateError` into [`Error`]. /// -/// All migration errors are considered `AppErrorKind::Database`. -/// The error string is preserved in `message`. +/// Errors are categorised as `Database` with metadata describing the failing +/// migration phase. #[cfg(feature = "sqlx-migrate")] #[cfg_attr(docsrs, doc(cfg(feature = "sqlx-migrate")))] -impl From for AppError { +impl From for Error { fn from(err: MigrateError) -> Self { - AppError::database_with_message(err.to_string()) + build_migrate_context(&err).into_error(err) + } +} + +#[cfg(feature = "sqlx")] +fn build_sqlx_context(err: &SqlxError) -> (Context, Option) { + match err { + SqlxError::RowNotFound => ( + Context::new(AppErrorKind::NotFound).with(field::str("db.reason", "row_not_found")), + None + ), + SqlxError::Database(db_err) => classify_database_error(db_err.as_ref()), + SqlxError::Io(io_err) => ( + Context::new(AppErrorKind::DependencyUnavailable) + .with(field::str("db.reason", "io_error")) + .with(field::str("io.kind", format!("{:?}", io_err.kind()))), + None + ), + SqlxError::PoolTimedOut => ( + Context::new(AppErrorKind::Timeout).with(field::str("db.reason", "pool_timeout")), + Some(1) + ), + SqlxError::PoolClosed => ( + Context::new(AppErrorKind::DependencyUnavailable) + .with(field::str("db.reason", "pool_closed")), + None + ), + SqlxError::WorkerCrashed => ( + Context::new(AppErrorKind::DependencyUnavailable) + .with(field::str("db.reason", "worker_crashed")), + Some(1) + ), + SqlxError::Configuration(source) => ( + Context::new(AppErrorKind::Config) + .with(field::str("db.reason", "configuration")) + .with(field::str("db.detail", source.to_string())), + None + ), + SqlxError::InvalidArgument(message) => ( + Context::new(AppErrorKind::BadRequest) + .with(field::str("db.reason", "invalid_argument")) + .with(field::str("db.argument", message.clone())), + None + ), + SqlxError::ColumnDecode { + index, .. + } => ( + Context::new(AppErrorKind::Deserialization) + .with(field::str("db.reason", "column_decode")) + .with(field::str("db.column", index.clone())), + None + ), + SqlxError::ColumnNotFound(name) => ( + Context::new(AppErrorKind::Internal) + .with(field::str("db.reason", "column_not_found")) + .with(field::str("db.column", name.clone())), + None + ), + SqlxError::ColumnIndexOutOfBounds { + index, + len + } => ( + Context::new(AppErrorKind::Internal) + .with(field::str("db.reason", "column_index_out_of_bounds")) + .with(field::u64("db.index", *index as u64)) + .with(field::u64("db.len", *len as u64)), + None + ), + SqlxError::TypeNotFound { + type_name + } => ( + Context::new(AppErrorKind::Internal) + .with(field::str("db.reason", "type_not_found")) + .with(field::str("db.type", type_name.clone())), + None + ), + SqlxError::Encode(_) => ( + Context::new(AppErrorKind::Serialization).with(field::str("db.reason", "encode")), + None + ), + SqlxError::Decode(_) => ( + Context::new(AppErrorKind::Deserialization).with(field::str("db.reason", "decode")), + None + ), + SqlxError::Protocol(detail) => ( + Context::new(AppErrorKind::DependencyUnavailable) + .with(field::str("db.reason", "protocol")) + .with(field::str("db.detail", detail.clone())), + Some(1) + ), + SqlxError::Tls(_) => ( + Context::new(AppErrorKind::Network).with(field::str("db.reason", "tls")), + Some(1) + ), + SqlxError::AnyDriverError(_) => ( + Context::new(AppErrorKind::Database).with(field::str("db.reason", "driver_error")), + None + ), + SqlxError::InvalidSavePointStatement => ( + Context::new(AppErrorKind::Internal) + .with(field::str("db.reason", "invalid_savepoint")), + None + ), + SqlxError::BeginFailed => ( + Context::new(AppErrorKind::DependencyUnavailable) + .with(field::str("db.reason", "begin_failed")), + Some(1) + ), + other => ( + Context::new(AppErrorKind::Database) + .with(field::str("db.reason", "unclassified")) + .with(field::str("db.detail", format!("{:?}", other))), + None + ) + } +} + +#[cfg(feature = "sqlx")] +fn classify_database_error(error: &(dyn DatabaseError + 'static)) -> (Context, Option) { + let mut context = Context::new(AppErrorKind::Database) + .with(field::str("db.reason", "database_error")) + .with(field::str("db.message", error.message().to_owned())); + + if let Some(constraint) = error.constraint() { + context = context.with(field::str("db.constraint", constraint.to_owned())); + } + if let Some(table) = error.table() { + context = context.with(field::str("db.table", table.to_owned())); + } + + let mut retry_after = None; + let mut category = AppErrorKind::Database; + let mut code_override = None; + + let code = error.code().map(|code| code.into_owned()); + if let Some(ref sqlstate) = code { + context = context.with(field::str("db.code", sqlstate.clone())); + if let Some((_, secs)) = SQLSTATE_RETRY_HINTS + .iter() + .find(|(state, _)| *state == sqlstate.as_str()) + { + retry_after = Some(*secs); + } + if let Some((_, app_code)) = SQLSTATE_CODE_OVERRIDES + .iter() + .find(|(state, _)| *state == sqlstate.as_str()) + { + code_override = Some(*app_code); + } + } + + category = match error.kind() { + SqlxErrorKind::UniqueViolation => AppErrorKind::Conflict, + SqlxErrorKind::ForeignKeyViolation => AppErrorKind::Conflict, + SqlxErrorKind::NotNullViolation | SqlxErrorKind::CheckViolation => { + AppErrorKind::Validation + } + SqlxErrorKind::Other => AppErrorKind::Database + }; + + context = context.category(category); + if let Some(code) = code_override { + context = context.code(code); + } + + (context, retry_after) +} + +#[cfg(feature = "sqlx-migrate")] +fn build_migrate_context(err: &MigrateError) -> Context { + match err { + MigrateError::Execute(inner) => Context::new(AppErrorKind::Database) + .with(field::str("migration.phase", "execute")) + .with(field::str("migration.source", inner.to_string())), + MigrateError::ExecuteMigration(inner, version) => Context::new(AppErrorKind::Database) + .with(field::str("migration.phase", "execute_migration")) + .with(field::i64("migration.version", *version)) + .with(field::str("migration.source", inner.to_string())), + MigrateError::Source(source) => Context::new(AppErrorKind::Database) + .with(field::str("migration.phase", "source")) + .with(field::str("migration.source", source.to_string())), + MigrateError::VersionMissing(version) => Context::new(AppErrorKind::Database) + .with(field::str("migration.phase", "version_missing")) + .with(field::i64("migration.version", *version)), + MigrateError::VersionMismatch(version) => Context::new(AppErrorKind::Database) + .with(field::str("migration.phase", "version_mismatch")) + .with(field::i64("migration.version", *version)), + MigrateError::VersionNotPresent(version) => Context::new(AppErrorKind::Database) + .with(field::str("migration.phase", "version_not_present")) + .with(field::i64("migration.version", *version)), + MigrateError::VersionTooOld(version, latest) => Context::new(AppErrorKind::Database) + .with(field::str("migration.phase", "version_too_old")) + .with(field::i64("migration.version", *version)) + .with(field::i64("migration.latest", *latest)), + MigrateError::VersionTooNew(version, latest) => Context::new(AppErrorKind::Database) + .with(field::str("migration.phase", "version_too_new")) + .with(field::i64("migration.version", *version)) + .with(field::i64("migration.latest", *latest)), + MigrateError::ForceNotSupported => Context::new(AppErrorKind::Database) + .with(field::str("migration.phase", "force_not_supported")), + MigrateError::InvalidMixReversibleAndSimple => { + Context::new(AppErrorKind::Database).with(field::str("migration.phase", "invalid_mix")) + } + MigrateError::Dirty(version) => Context::new(AppErrorKind::Database) + .with(field::str("migration.phase", "dirty")) + .with(field::i64("migration.version", *version)) } } #[cfg(all(test, feature = "sqlx"))] mod tests_sqlx { - use std::io; + use std::fmt; use super::*; - use crate::AppErrorKind; + use crate::{AppCode, AppErrorKind, FieldValue}; #[test] fn row_not_found_maps_to_not_found() { - let err: AppError = SqlxError::RowNotFound.into(); + let err: Error = SqlxError::RowNotFound.into(); assert!(matches!(err.kind, AppErrorKind::NotFound)); } #[test] - fn other_error_maps_to_database() { - // Prefer modern constructor; avoids clippy::io-other-error - let io_err = io::Error::other("boom"); - let err: AppError = SqlxError::Io(io_err).into(); - assert!(matches!(err.kind, AppErrorKind::Database)); + fn io_error_maps_to_dependency_unavailable() { + let io_err = std::io::Error::other("boom"); + let err: Error = SqlxError::Io(io_err).into(); + assert!(matches!(err.kind, AppErrorKind::DependencyUnavailable)); + let metadata = err.metadata(); + assert_eq!( + metadata.get("db.reason"), + Some(&FieldValue::Str("io_error".into())) + ); + } + + #[test] + fn unique_violation_sets_code_override() { + let db_err = DummyDbError { + message: "duplicate key".into(), + code: Some("23505".into()), + constraint: Some("users_email_key".into()), + table: Some("users".into()), + kind: SqlxErrorKind::UniqueViolation + }; + let err: Error = SqlxError::Database(Box::new(db_err)).into(); + assert_eq!(err.kind, AppErrorKind::Conflict); + assert_eq!(err.code, AppCode::UserAlreadyExists); + let metadata = err.metadata(); + assert_eq!( + metadata.get("db.constraint"), + Some(&FieldValue::Str("users_email_key".into())) + ); + } + + #[test] + fn serialization_failure_carries_retry_hint() { + let db_err = DummyDbError { + message: "serialization failure".into(), + code: Some("40001".into()), + constraint: None, + table: None, + kind: SqlxErrorKind::Other + }; + let err: Error = SqlxError::Database(Box::new(db_err)).into(); + assert_eq!(err.retry.map(|r| r.after_seconds), Some(1)); + } + + #[derive(Debug)] + struct DummyDbError { + message: String, + code: Option, + constraint: Option, + table: Option, + kind: SqlxErrorKind + } + + impl fmt::Display for DummyDbError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.message) + } + } + + impl std::error::Error for DummyDbError {} + + impl DatabaseError for DummyDbError { + fn message(&self) -> &str { + &self.message + } + + fn code(&self) -> Option> { + self.code.as_deref().map(std::borrow::Cow::Borrowed) + } + + fn as_error(&self) -> &(dyn std::error::Error + Send + Sync + 'static) { + self + } + + fn as_error_mut(&mut self) -> &mut (dyn std::error::Error + Send + Sync + 'static) { + self + } + + fn into_error(self: Box) -> Box { + self + } + + fn constraint(&self) -> Option<&str> { + self.constraint.as_deref() + } + + fn table(&self) -> Option<&str> { + self.table.as_deref() + } + + fn kind(&self) -> SqlxErrorKind { + self.kind + } } } diff --git a/src/convert/telegram_webapp_sdk.rs b/src/convert/telegram_webapp_sdk.rs index 252a089..063eeba 100644 --- a/src/convert/telegram_webapp_sdk.rs +++ b/src/convert/telegram_webapp_sdk.rs @@ -1,6 +1,6 @@ //! Conversion from //! [`telegram_webapp_sdk::utils::validate_init_data::ValidationError`] into -//! [`AppError`]. +//! [`Error`]. //! //! Enabled with the `telegram-webapp-sdk` feature flag. //! @@ -22,10 +22,10 @@ //! # #[cfg(feature = "telegram-webapp-sdk")] //! # { //! '''rust,ignore -//! use masterror::{AppError, AppErrorKind}; +//! use masterror::{AppErrorKind, Error}; //! use telegram_webapp_sdk::utils::validate_init_data::ValidationError; //! -//! fn convert(err: ValidationError) -> AppError { +//! fn convert(err: ValidationError) -> Error { //! err.into() //! } //! @@ -39,14 +39,37 @@ use telegram_webapp_sdk::utils::validate_init_data::ValidationError; #[cfg(feature = "telegram-webapp-sdk")] -use crate::AppError; +use crate::{ + AppErrorKind, + app_error::{Context, Error, field} +}; /// Map [`ValidationError`] into an [`AppError`] with kind `TelegramAuth`. #[cfg(feature = "telegram-webapp-sdk")] #[cfg_attr(docsrs, doc(cfg(feature = "telegram-webapp-sdk")))] -impl From for AppError { +impl From for Error { fn from(err: ValidationError) -> Self { - AppError::telegram_auth(err.to_string()) + build_context(&err).into_error(err) + } +} + +#[cfg(feature = "telegram-webapp-sdk")] +fn build_context(error: &ValidationError) -> Context { + match error { + ValidationError::MissingField(field) => Context::new(AppErrorKind::TelegramAuth) + .with(field::str("telegram_webapp.reason", "missing_field")) + .with(field::str("telegram_webapp.field", (*field).to_owned())), + ValidationError::InvalidEncoding => Context::new(AppErrorKind::TelegramAuth) + .with(field::str("telegram_webapp.reason", "invalid_encoding")), + ValidationError::InvalidSignatureEncoding => Context::new(AppErrorKind::TelegramAuth) + .with(field::str( + "telegram_webapp.reason", + "invalid_signature_encoding" + )), + ValidationError::SignatureMismatch => Context::new(AppErrorKind::TelegramAuth) + .with(field::str("telegram_webapp.reason", "signature_mismatch")), + ValidationError::InvalidPublicKey => Context::new(AppErrorKind::TelegramAuth) + .with(field::str("telegram_webapp.reason", "invalid_public_key")) } } @@ -55,7 +78,7 @@ mod tests { use telegram_webapp_sdk::utils::validate_init_data::ValidationError; use super::*; - use crate::AppErrorKind; + use crate::{AppErrorKind, FieldValue}; #[test] fn all_variants_map_to_telegram_auth_and_preserve_message() { @@ -68,16 +91,19 @@ mod tests { ]; for case in cases { - let msg = case.to_string(); - let app: AppError = case.into(); + let app: Error = case.into(); assert!(matches!(app.kind, AppErrorKind::TelegramAuth)); - assert_eq!(app.message.as_deref(), Some(msg.as_str())); + assert!(app.metadata().get("telegram_webapp.reason").is_some()); } } #[test] fn validation_error_maps_to_telegram_auth() { - let err: AppError = ValidationError::SignatureMismatch.into(); + let err: Error = ValidationError::SignatureMismatch.into(); assert!(matches!(err.kind, AppErrorKind::TelegramAuth)); + assert_eq!( + err.metadata().get("telegram_webapp.reason"), + Some(&FieldValue::Str("signature_mismatch".into())) + ); } } diff --git a/src/convert/teloxide.rs b/src/convert/teloxide.rs index 6e30032..9686f55 100644 --- a/src/convert/teloxide.rs +++ b/src/convert/teloxide.rs @@ -1,4 +1,4 @@ -//! Conversion from [`teloxide_core::RequestError`] into [`AppError`]. +//! Conversion from [`teloxide_core::RequestError`] into [`Error`]. //! //! Enabled with the `teloxide` feature flag. //! @@ -16,11 +16,11 @@ //! ## Example //! //! ```rust,ignore -//! use masterror::{AppError, AppErrorKind}; +//! use masterror::{AppErrorKind, Error}; //! use teloxide_core::{errors::ApiError, RequestError, types::Seconds}; //! use std::{io, sync::Arc}; //! -//! fn map(err: RequestError) -> AppError { err.into() } +//! fn map(err: RequestError) -> Error { err.into() } //! //! let err = RequestError::RetryAfter(Seconds::from_seconds(1)); //! let app_err = map(err); @@ -30,26 +30,70 @@ use teloxide_core::RequestError; #[cfg(feature = "teloxide")] -use crate::AppError; +use crate::{ + AppErrorKind, + app_error::{Context, Error, field} +}; #[cfg(feature = "teloxide")] #[cfg_attr(docsrs, doc(cfg(feature = "teloxide")))] -impl From for AppError { +impl From for Error { fn from(err: RequestError) -> Self { - match err { - RequestError::Api(api) => AppError::external_api(format!("Telegram API error: {api}")), - RequestError::MigrateToChatId(id) => { - AppError::external_api(format!("Group migrated to {id}")) - } - RequestError::RetryAfter(secs) => { - AppError::rate_limited(format!("Retry after {secs}")) - } - RequestError::Network(e) => AppError::network(format!("Network error: {e}")), - RequestError::InvalidJson { - source, .. - } => AppError::deserialization(format!("Invalid Telegram JSON: {source}")), - RequestError::Io(e) => AppError::internal(format!("I/O error: {e}")) + let (context, retry_after) = build_teloxide_context(&err); + let mut error = context.into_error(err); + if let Some(secs) = retry_after { + error = error.with_retry_after_secs(secs); } + error + } +} + +#[cfg(feature = "teloxide")] +fn build_teloxide_context(err: &RequestError) -> (Context, Option) { + match err { + RequestError::Api(api) => ( + Context::new(AppErrorKind::ExternalApi) + .with(field::str("telegram.reason", "api")) + .with(field::str("telegram.api_error", api.to_string())), + None + ), + RequestError::MigrateToChatId(id) => ( + Context::new(AppErrorKind::ExternalApi) + .with(field::str("telegram.reason", "migrate_to_chat")) + .with(field::i64("telegram.chat_id", id.0)), + None + ), + RequestError::RetryAfter(secs) => { + let seconds = u64::from(secs.seconds()); + ( + Context::new(AppErrorKind::RateLimited) + .with(field::str("telegram.reason", "retry_after")) + .with(field::u64("telegram.retry_after_secs", seconds)), + Some(seconds) + ) + } + RequestError::Network(e) => ( + Context::new(AppErrorKind::Network) + .with(field::str("telegram.reason", "network")) + .with(field::str("telegram.detail", e.to_string())), + None + ), + RequestError::InvalidJson { + source, + raw + } => ( + Context::new(AppErrorKind::Deserialization) + .with(field::str("telegram.reason", "invalid_json")) + .with(field::str("telegram.detail", source.to_string())) + .with(field::u64("telegram.payload_len", raw.len() as u64)), + None + ), + RequestError::Io(e) => ( + Context::new(AppErrorKind::Internal) + .with(field::str("telegram.reason", "io")) + .with(field::str("io.kind", format!("{:?}", e.kind()))), + None + ) } } @@ -60,27 +104,36 @@ mod tests { use teloxide_core::{errors::ApiError, types::Seconds}; use super::*; - use crate::AppErrorKind; + use crate::{AppErrorKind, FieldValue}; #[test] fn api_maps_to_external_api() { let err = RequestError::Api(ApiError::BotBlocked); - let app_err: AppError = err.into(); + let app_err: Error = err.into(); assert!(matches!(app_err.kind, AppErrorKind::ExternalApi)); + assert_eq!( + app_err.metadata().get("telegram.api_error"), + Some(&FieldValue::Str(ApiError::BotBlocked.to_string().into())) + ); } #[test] fn retry_after_maps_to_rate_limited() { let err = RequestError::RetryAfter(Seconds::from_seconds(5)); - let app_err: AppError = err.into(); + let app_err: Error = err.into(); assert!(matches!(app_err.kind, AppErrorKind::RateLimited)); + assert_eq!(app_err.retry.map(|r| r.after_seconds), Some(5)); } #[test] fn io_maps_to_internal() { let io_err = Arc::new(io::Error::other("disk")); let err = RequestError::Io(io_err); - let app_err: AppError = err.into(); + let app_err: Error = err.into(); assert!(matches!(app_err.kind, AppErrorKind::Internal)); + assert_eq!( + app_err.metadata().get("telegram.reason"), + Some(&FieldValue::Str("io".into())) + ); } } diff --git a/src/convert/tokio.rs b/src/convert/tokio.rs index 8592095..a2c79e1 100644 --- a/src/convert/tokio.rs +++ b/src/convert/tokio.rs @@ -1,4 +1,4 @@ -//! Conversion from [`tokio::time::error::Elapsed`] into [`AppError`]. +//! Conversion from [`tokio::time::error::Elapsed`] into [`Error`]. //! //! Enabled with the `tokio` feature flag. //! @@ -17,7 +17,7 @@ //! ## Example //! //! ```rust,ignore -//! use masterror::{AppError, AppErrorKind}; +//! use masterror::{AppErrorKind, Error}; //! use tokio::time::{sleep, timeout, Duration}; //! //! #[tokio::main] @@ -26,7 +26,7 @@ //! let res = timeout(Duration::from_millis(10), fut).await; //! //! let err = res.unwrap_err(); // tokio::time::error::Elapsed -//! let app_err: AppError = err.into(); +//! let app_err: Error = err.into(); //! //! assert!(matches!(app_err.kind, AppErrorKind::Timeout)); //! } @@ -36,7 +36,10 @@ use tokio::time::error::Elapsed; #[cfg(feature = "tokio")] -use crate::AppError; +use crate::{ + AppErrorKind, + app_error::{Context, Error, field} +}; /// Map a [`tokio::time::error::Elapsed`] into an [`AppError`] with kind /// `Timeout`. @@ -44,9 +47,11 @@ use crate::AppError; /// Message is fixed to avoid leaking timing specifics to the client. #[cfg(feature = "tokio")] #[cfg_attr(docsrs, doc(cfg(feature = "tokio")))] -impl From for AppError { - fn from(_: Elapsed) -> Self { - AppError::timeout("Operation timed out") +impl From for Error { + fn from(err: Elapsed) -> Self { + Context::new(AppErrorKind::Timeout) + .with(field::str("timeout.source", "tokio::time::timeout")) + .into_error(err) } } @@ -55,7 +60,7 @@ mod tests { use tokio::time::{Duration, sleep, timeout}; use super::*; - use crate::AppErrorKind; + use crate::{AppErrorKind, FieldValue}; #[tokio::test] async fn elapsed_maps_to_timeout() { @@ -63,7 +68,11 @@ mod tests { let err = timeout(Duration::from_millis(1), fut) .await .expect_err("expect timeout"); - let app_err: AppError = err.into(); + let app_err: Error = err.into(); assert!(matches!(app_err.kind, AppErrorKind::Timeout)); + assert_eq!( + app_err.metadata().get("timeout.source"), + Some(&FieldValue::Str("tokio::time::timeout".into())) + ); } } diff --git a/src/convert/validator.rs b/src/convert/validator.rs index e5f7f47..e0fb4e2 100644 --- a/src/convert/validator.rs +++ b/src/convert/validator.rs @@ -1,4 +1,4 @@ -//! Conversion from [`validator::ValidationErrors`] into [`AppError`]. +//! Conversion from [`validator::ValidationErrors`] into [`Error`]. //! //! Enabled with the `validator` feature flag. //! @@ -19,7 +19,7 @@ //! ## Example //! //! ```rust,ignore -//! use masterror::{AppError, AppErrorKind, AppResult}; +//! use masterror::{AppErrorKind, AppResult, Error}; //! use validator::{Validate, ValidationError}; //! //! #[derive(Validate)] @@ -39,10 +39,13 @@ //! ``` #[cfg(feature = "validator")] -use validator::ValidationErrors; +use validator::{ValidationErrors, ValidationErrorsKind}; #[cfg(feature = "validator")] -use crate::AppError; +use crate::{ + AppErrorKind, + app_error::{Context, Error, field} +}; /// Map [`validator::ValidationErrors`] into an [`AppError`] with kind /// `Validation`. @@ -51,18 +54,74 @@ use crate::AppError; /// Consider extending `AppError` if you want to expose structured details. #[cfg(feature = "validator")] #[cfg_attr(docsrs, doc(cfg(feature = "validator")))] -impl From for AppError { +impl From for Error { fn from(err: ValidationErrors) -> Self { - AppError::validation(err.to_string()) + build_context(&err).into_error(err) } } +#[cfg(feature = "validator")] +fn build_context(errors: &ValidationErrors) -> Context { + let mut context = Context::new(AppErrorKind::Validation); + + let field_errors = errors.field_errors(); + if !field_errors.is_empty() { + context = context.with(field::u64( + "validation.field_count", + field_errors.len() as u64 + )); + + let total: u64 = field_errors.values().map(|errs| errs.len() as u64).sum(); + if total > 0 { + context = context.with(field::u64("validation.error_count", total)); + } + + let mut names = String::new(); + for (idx, name) in field_errors.keys().take(3).enumerate() { + if idx > 0 { + names.push(','); + } + names.push_str(name.as_ref()); + } + if !names.is_empty() { + context = context.with(field::str("validation.fields", names)); + } + + let mut codes: Vec = Vec::new(); + for errors in field_errors.values() { + for error in *errors { + let code = error.code.as_ref(); + if codes.len() >= 3 { + break; + } + if codes.iter().any(|existing| existing == code) { + continue; + } + codes.push(code.to_string()); + } + } + if !codes.is_empty() { + context = context.with(field::str("validation.codes", codes.join(","))); + } + } + + let has_nested = errors + .errors() + .values() + .any(|kind| !matches!(kind, ValidationErrorsKind::Field(_))); + if has_nested { + context = context.with(field::bool("validation.has_nested", true)); + } + + context +} + #[cfg(all(test, feature = "validator"))] mod tests { use validator::Validate; use super::*; - use crate::AppErrorKind; + use crate::{AppErrorKind, FieldValue}; #[derive(Validate)] struct Payload { @@ -75,7 +134,12 @@ mod tests { let bad = Payload { val: 0 }; - let err: AppError = bad.validate().unwrap_err().into(); + let err: Error = bad.validate().unwrap_err().into(); assert!(matches!(err.kind, AppErrorKind::Validation)); + let metadata = err.metadata(); + assert_eq!( + metadata.get("validation.field_count"), + Some(&FieldValue::U64(1)) + ); } } diff --git a/src/response/problem_json.rs b/src/response/problem_json.rs index 440dd2e..bdb2a0b 100644 --- a/src/response/problem_json.rs +++ b/src/response/problem_json.rs @@ -553,6 +553,18 @@ pub const CODE_MAPPINGS: &[(AppCode, CodeMapping)] = &[ kind: AppErrorKind::Conflict } ), + ( + AppCode::UserAlreadyExists, + CodeMapping { + http_status: 409, + grpc: GrpcCode { + name: "ALREADY_EXISTS", + value: 6 + }, + problem_type: "https://errors.masterror.rs/user-already-exists", + kind: AppErrorKind::Conflict + } + ), ( AppCode::Unauthorized, CodeMapping { diff --git a/tests/ui/app_error/fail/enum_missing_variant.stderr b/tests/ui/app_error/fail/enum_missing_variant.stderr index bbc297c..d000de1 100644 --- a/tests/ui/app_error/fail/enum_missing_variant.stderr +++ b/tests/ui/app_error/fail/enum_missing_variant.stderr @@ -1,9 +1,8 @@ error: all variants must use #[app_error(...)] to derive AppError conversion --> tests/ui/app_error/fail/enum_missing_variant.rs:8:5 | -8 | / #[error("without")] -9 | | Without, - | |___________^ +8 | #[error("without")] + | ^ warning: unused import: `AppErrorKind` --> tests/ui/app_error/fail/enum_missing_variant.rs:1:17 @@ -11,4 +10,4 @@ warning: unused import: `AppErrorKind` 1 | use masterror::{AppErrorKind, Error}; | ^^^^^^^^^^^^ | - = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default + = note: `#[warn(unused_imports)]` on by default diff --git a/tests/ui/app_error/fail/missing_code.stderr b/tests/ui/app_error/fail/missing_code.stderr index 4f02301..70ccade 100644 --- a/tests/ui/app_error/fail/missing_code.stderr +++ b/tests/ui/app_error/fail/missing_code.stderr @@ -2,7 +2,7 @@ error: AppCode conversion requires `code = ...` in #[app_error(...)] --> tests/ui/app_error/fail/missing_code.rs:9:5 | 9 | #[app_error(kind = AppErrorKind::Service)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^ warning: unused imports: `AppCode` and `AppErrorKind` --> tests/ui/app_error/fail/missing_code.rs:1:17 @@ -10,4 +10,4 @@ warning: unused imports: `AppCode` and `AppErrorKind` 1 | use masterror::{AppCode, AppErrorKind, Error}; | ^^^^^^^ ^^^^^^^^^^^^ | - = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default + = note: `#[warn(unused_imports)]` on by default diff --git a/tests/ui/app_error/fail/missing_kind.stderr b/tests/ui/app_error/fail/missing_kind.stderr index 021c135..c615e98 100644 --- a/tests/ui/app_error/fail/missing_kind.stderr +++ b/tests/ui/app_error/fail/missing_kind.stderr @@ -2,4 +2,4 @@ error: missing `kind = ...` in #[app_error(...)] --> tests/ui/app_error/fail/missing_kind.rs:5:1 | 5 | #[app_error(message)] - | ^^^^^^^^^^^^^^^^^^^^^ + | ^ diff --git a/tests/ui/formatter/fail/duplicate_fmt.stderr b/tests/ui/formatter/fail/duplicate_fmt.stderr index 5b8f363..5b08225 100644 --- a/tests/ui/formatter/fail/duplicate_fmt.stderr +++ b/tests/ui/formatter/fail/duplicate_fmt.stderr @@ -2,4 +2,4 @@ error: duplicate `fmt` handler specified --> tests/ui/formatter/fail/duplicate_fmt.rs:4:36 | 4 | #[error(fmt = crate::format_error, fmt = crate::format_error)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^ diff --git a/tests/ui/formatter/fail/implicit_after_named.stderr b/tests/ui/formatter/fail/implicit_after_named.stderr index be76742..d416399 100644 --- a/tests/ui/formatter/fail/implicit_after_named.stderr +++ b/tests/ui/formatter/fail/implicit_after_named.stderr @@ -8,5 +8,4 @@ error: multiple unused formatting arguments | argument never used | argument never used | - = note: consider adding 2 format specifiers = note: this error originates in the derive macro `Error` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/tests/ui/formatter/fail/unsupported_flag.stderr b/tests/ui/formatter/fail/unsupported_flag.stderr index b8bf229..d7acdb1 100644 --- a/tests/ui/formatter/fail/unsupported_flag.stderr +++ b/tests/ui/formatter/fail/unsupported_flag.stderr @@ -1,5 +1,5 @@ error: placeholder spanning bytes 0..11 uses an unsupported formatter - --> tests/ui/formatter/fail/unsupported_flag.rs:4:10 + --> tests/ui/formatter/fail/unsupported_flag.rs:4:9 | 4 | #[error("{value:##x}")] - | ^^^^^^^^^^^ + | ^^^^^^^^^^^^^ diff --git a/tests/ui/formatter/fail/unsupported_formatter.stderr b/tests/ui/formatter/fail/unsupported_formatter.stderr index a6a40c2..5869420 100644 --- a/tests/ui/formatter/fail/unsupported_formatter.stderr +++ b/tests/ui/formatter/fail/unsupported_formatter.stderr @@ -1,5 +1,5 @@ error: placeholder spanning bytes 0..9 uses an unsupported formatter - --> tests/ui/formatter/fail/unsupported_formatter.rs:4:10 + --> tests/ui/formatter/fail/unsupported_formatter.rs:4:9 | 4 | #[error("{value:y}")] - | ^^^^^^^^^ + | ^^^^^^^^^^^ diff --git a/tests/ui/formatter/fail/uppercase_binary.stderr b/tests/ui/formatter/fail/uppercase_binary.stderr index 3d332c7..bbe04b4 100644 --- a/tests/ui/formatter/fail/uppercase_binary.stderr +++ b/tests/ui/formatter/fail/uppercase_binary.stderr @@ -1,5 +1,5 @@ error: placeholder spanning bytes 0..9 uses an unsupported formatter - --> tests/ui/formatter/fail/uppercase_binary.rs:4:10 + --> tests/ui/formatter/fail/uppercase_binary.rs:4:9 | 4 | #[error("{value:B}")] - | ^^^^^^^^^ + | ^^^^^^^^^^^ diff --git a/tests/ui/formatter/fail/uppercase_pointer.stderr b/tests/ui/formatter/fail/uppercase_pointer.stderr index 0bd10fa..2c30e71 100644 --- a/tests/ui/formatter/fail/uppercase_pointer.stderr +++ b/tests/ui/formatter/fail/uppercase_pointer.stderr @@ -1,5 +1,5 @@ error: placeholder spanning bytes 0..9 uses an unsupported formatter - --> tests/ui/formatter/fail/uppercase_pointer.rs:4:10 + --> tests/ui/formatter/fail/uppercase_pointer.rs:4:9 | 4 | #[error("{value:P}")] - | ^^^^^^^^^ + | ^^^^^^^^^^^ diff --git a/tests/ui/masterror/fail/duplicate_attr.stderr b/tests/ui/masterror/fail/duplicate_attr.stderr index 113a10d..c3fb86b 100644 --- a/tests/ui/masterror/fail/duplicate_attr.stderr +++ b/tests/ui/masterror/fail/duplicate_attr.stderr @@ -10,4 +10,4 @@ warning: unused imports: `AppCode` and `AppErrorKind` 1 | use masterror::{AppCode, AppErrorKind, Masterror}; | ^^^^^^^ ^^^^^^^^^^^^ | - = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default + = note: `#[warn(unused_imports)]` on by default diff --git a/tests/ui/masterror/fail/duplicate_telemetry.stderr b/tests/ui/masterror/fail/duplicate_telemetry.stderr index 9ada290..b331baa 100644 --- a/tests/ui/masterror/fail/duplicate_telemetry.stderr +++ b/tests/ui/masterror/fail/duplicate_telemetry.stderr @@ -10,4 +10,4 @@ warning: unused imports: `AppCode` and `AppErrorKind` 1 | use masterror::{AppCode, AppErrorKind, Masterror}; | ^^^^^^^ ^^^^^^^^^^^^ | - = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default + = note: `#[warn(unused_imports)]` on by default diff --git a/tests/ui/masterror/fail/empty_redact.stderr b/tests/ui/masterror/fail/empty_redact.stderr index fd151cc..b2658a1 100644 --- a/tests/ui/masterror/fail/empty_redact.stderr +++ b/tests/ui/masterror/fail/empty_redact.stderr @@ -10,4 +10,4 @@ warning: unused imports: `AppCode` and `AppErrorKind` 1 | use masterror::{AppCode, AppErrorKind, Masterror}; | ^^^^^^^ ^^^^^^^^^^^^ | - = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default + = note: `#[warn(unused_imports)]` on by default diff --git a/tests/ui/masterror/fail/enum_missing_variant.stderr b/tests/ui/masterror/fail/enum_missing_variant.stderr index 5a25e12..83d517f 100644 --- a/tests/ui/masterror/fail/enum_missing_variant.stderr +++ b/tests/ui/masterror/fail/enum_missing_variant.stderr @@ -1,9 +1,8 @@ error: all variants must use #[masterror(...)] to derive masterror::Error conversion --> tests/ui/masterror/fail/enum_missing_variant.rs:8:5 | -8 | / #[error("missing")] -9 | | Missing - | |___________^ +8 | #[error("missing")] + | ^ warning: unused imports: `AppCode` and `AppErrorKind` --> tests/ui/masterror/fail/enum_missing_variant.rs:1:17 @@ -11,4 +10,4 @@ warning: unused imports: `AppCode` and `AppErrorKind` 1 | use masterror::{AppCode, AppErrorKind, Masterror}; | ^^^^^^^ ^^^^^^^^^^^^ | - = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default + = note: `#[warn(unused_imports)]` on by default diff --git a/tests/ui/masterror/fail/missing_category.stderr b/tests/ui/masterror/fail/missing_category.stderr index bdadf45..f929951 100644 --- a/tests/ui/masterror/fail/missing_category.stderr +++ b/tests/ui/masterror/fail/missing_category.stderr @@ -2,7 +2,7 @@ error: missing `category = ...` in #[masterror(...)] --> tests/ui/masterror/fail/missing_category.rs:5:1 | 5 | #[masterror(code = AppCode::Internal)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^ warning: unused import: `AppCode` --> tests/ui/masterror/fail/missing_category.rs:1:17 @@ -10,4 +10,4 @@ warning: unused import: `AppCode` 1 | use masterror::{AppCode, Masterror}; | ^^^^^^^ | - = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default + = note: `#[warn(unused_imports)]` on by default diff --git a/tests/ui/masterror/fail/missing_code.stderr b/tests/ui/masterror/fail/missing_code.stderr index 037fac8..34abc91 100644 --- a/tests/ui/masterror/fail/missing_code.stderr +++ b/tests/ui/masterror/fail/missing_code.stderr @@ -2,7 +2,7 @@ error: missing `code = ...` in #[masterror(...)] --> tests/ui/masterror/fail/missing_code.rs:5:1 | 5 | #[masterror(category = AppErrorKind::Internal)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^ warning: unused import: `AppErrorKind` --> tests/ui/masterror/fail/missing_code.rs:1:17 @@ -10,4 +10,4 @@ warning: unused import: `AppErrorKind` 1 | use masterror::{AppErrorKind, Masterror}; | ^^^^^^^^^^^^ | - = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default + = note: `#[warn(unused_imports)]` on by default diff --git a/tests/ui/masterror/fail/unknown_option.stderr b/tests/ui/masterror/fail/unknown_option.stderr index 1822edf..d579838 100644 --- a/tests/ui/masterror/fail/unknown_option.stderr +++ b/tests/ui/masterror/fail/unknown_option.stderr @@ -10,4 +10,4 @@ warning: unused imports: `AppCode` and `AppErrorKind` 1 | use masterror::{AppCode, AppErrorKind, Masterror}; | ^^^^^^^ ^^^^^^^^^^^^ | - = note: `#[warn(unused_imports)]` (part of `#[warn(unused)]`) on by default + = note: `#[warn(unused_imports)]` on by default