Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 28 additions & 26 deletions src/app_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@
//! `kind`, `code` and optional `message` fields. Prefer calling it at the
//! transport boundary (e.g. in `IntoResponse`) to avoid duplicate logs.

use std::borrow::Cow;

use thiserror::Error;
use tracing::error;

Expand All @@ -73,7 +75,7 @@ pub struct AppError {
/// Semantic category of the error.
pub kind: AppErrorKind,
/// Optional, public-friendly message.
pub message: Option<String>
pub message: Option<Cow<'static, str>>
}

/// Conventional result alias for application code.
Expand All @@ -92,15 +94,15 @@ impl AppError {
/// let err = AppError::new(AppErrorKind::BadRequest, "invalid payload");
/// assert!(err.message.is_some());
/// ```
pub fn new(kind: AppErrorKind, msg: impl Into<String>) -> Self {
pub fn new(kind: AppErrorKind, msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(kind, msg)
}

/// Create an error with the given kind and message.
///
/// Prefer named helpers (e.g. [`AppError::not_found`]) where it clarifies
/// intent.
pub fn with(kind: AppErrorKind, msg: impl Into<String>) -> Self {
pub fn with(kind: AppErrorKind, msg: impl Into<Cow<'static, str>>) -> Self {
Self {
Comment on lines 102 to 106

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Restrict AppError constructors to 'static string slices

Changing the message parameter to impl Into<Cow<'static, str>> means only 'static borrows are accepted. Callers that previously passed short‑lived &str values (e.g. AppError::bad_request(err.as_str())) now hit a compile error because such slices no longer implement Into<Cow<'static, str>>. This contradicts the stated goal of accepting borrowed or owned strings and introduces a breaking API change unless every non‑static borrow is converted to String first. Consider accepting impl Into<Cow<'_, str>> and promoting non‑'static values to owned strings internally, or keep the original impl Into<String> and wrap with Cow::Owned so existing call sites continue to compile.

Useful? React with 👍 / 👎.

kind,
message: Some(msg.into())
Expand Down Expand Up @@ -142,103 +144,103 @@ impl AppError {

// 4xx-ish
/// Build a `NotFound` error.
pub fn not_found(msg: impl Into<String>) -> Self {
pub fn not_found(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::NotFound, msg)
}
/// Build a `Validation` error.
pub fn validation(msg: impl Into<String>) -> Self {
pub fn validation(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::Validation, msg)
}
/// Build an `Unauthorized` error.
pub fn unauthorized(msg: impl Into<String>) -> Self {
pub fn unauthorized(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::Unauthorized, msg)
}
/// Build a `Forbidden` error.
pub fn forbidden(msg: impl Into<String>) -> Self {
pub fn forbidden(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::Forbidden, msg)
}
/// Build a `Conflict` error.
pub fn conflict(msg: impl Into<String>) -> Self {
pub fn conflict(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::Conflict, msg)
}
/// Build a `BadRequest` error.
pub fn bad_request(msg: impl Into<String>) -> Self {
pub fn bad_request(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::BadRequest, msg)
}
/// Build a `RateLimited` error.
pub fn rate_limited(msg: impl Into<String>) -> Self {
pub fn rate_limited(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::RateLimited, msg)
}
/// Build a `TelegramAuth` error.
pub fn telegram_auth(msg: impl Into<String>) -> Self {
pub fn telegram_auth(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::TelegramAuth, msg)
}

// 5xx-ish
/// Build an `Internal` error.
pub fn internal(msg: impl Into<String>) -> Self {
pub fn internal(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::Internal, msg)
}
/// Build a `Service` error (generic server-side service failure).
pub fn service(msg: impl Into<String>) -> Self {
pub fn service(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::Service, msg)
}
/// Build a `Database` error with an optional message.
///
/// Accepts `Option` to avoid gratuitous `.map(|...| ...)` at call sites
/// when you may or may not have a safe-to-print string at hand.
pub fn database(msg: Option<impl Into<String>>) -> Self {
pub fn database(msg: Option<impl Into<Cow<'static, str>>>) -> Self {
Self {
kind: AppErrorKind::Database,
message: msg.map(|m| m.into())
message: msg.map(Into::into)
}
}
/// Build a `Config` error.
pub fn config(msg: impl Into<String>) -> Self {
pub fn config(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::Config, msg)
}
/// Build a `Turnkey` error.
pub fn turnkey(msg: impl Into<String>) -> Self {
pub fn turnkey(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::Turnkey, msg)
}

// Infra / network
/// Build a `Timeout` error.
pub fn timeout(msg: impl Into<String>) -> Self {
pub fn timeout(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::Timeout, msg)
}
/// Build a `Network` error.
pub fn network(msg: impl Into<String>) -> Self {
pub fn network(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::Network, msg)
}
/// Build a `DependencyUnavailable` error.
pub fn dependency_unavailable(msg: impl Into<String>) -> Self {
pub fn dependency_unavailable(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::DependencyUnavailable, msg)
}
/// Backward-compatible alias; routes to `DependencyUnavailable`.
pub fn service_unavailable(msg: impl Into<String>) -> Self {
pub fn service_unavailable(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::DependencyUnavailable, msg)
}

// Serialization / external API / subsystems
/// Build a `Serialization` error.
pub fn serialization(msg: impl Into<String>) -> Self {
pub fn serialization(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::Serialization, msg)
}
/// Build a `Deserialization` error.
pub fn deserialization(msg: impl Into<String>) -> Self {
pub fn deserialization(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::Deserialization, msg)
}
/// Build an `ExternalApi` error.
pub fn external_api(msg: impl Into<String>) -> Self {
pub fn external_api(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::ExternalApi, msg)
}
/// Build a `Queue` error.
pub fn queue(msg: impl Into<String>) -> Self {
pub fn queue(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::Queue, msg)
}
/// Build a `Cache` error.
pub fn cache(msg: impl Into<String>) -> Self {
pub fn cache(msg: impl Into<Cow<'static, str>>) -> Self {
Self::with(AppErrorKind::Cache, msg)
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/convert/reqwest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,10 @@ mod tests {
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), "{msg} does not contain {err_str}");
assert!(
msg.contains(err_str.as_str()),
"{msg} does not contain {err_str}"
);

server.abort();
}
Expand Down
10 changes: 5 additions & 5 deletions src/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@
//! provided as a deprecated shim.

use std::{
fmt::{Display, Formatter, Result as FmtResult},
time::Duration
borrow::Cow,
fmt::{Display, Formatter, Result as FmtResult}
};

use http::StatusCode;
Expand Down Expand Up @@ -297,9 +297,9 @@ impl From<&AppError> for ErrorResponse {

let message = err
.message
.as_deref()
.unwrap_or("An error occurred")
.to_owned();
.clone()
.unwrap_or(Cow::Borrowed("An error occurred"))
.into_owned();

Self {
status,
Expand Down
Loading