From f5e4083952d203c70e9746ed4e0afb656504a569 Mon Sep 17 00:00:00 2001 From: RA <70325462+RAprogramm@users.noreply.github.com> Date: Sat, 27 Sep 2025 07:28:17 +0700 Subject: [PATCH] Use AppErrorKind labels without extra allocations --- src/app_error/core.rs | 9 ++------ src/app_error/tests.rs | 10 +++++++++ src/kind.rs | 19 ++++++++++------- src/response/mapping.rs | 6 +++--- src/response/problem_json.rs | 14 ++++++++----- src/response/tests.rs | 40 +++++++++++++++++++++++++++++------- 6 files changed, 69 insertions(+), 29 deletions(-) diff --git a/src/app_error/core.rs b/src/app_error/core.rs index 70a6f97..cc743c0 100644 --- a/src/app_error/core.rs +++ b/src/app_error/core.rs @@ -1,9 +1,4 @@ -use alloc::{ - borrow::Cow, - boxed::Box, - string::{String, ToString}, - sync::Arc -}; +use alloc::{borrow::Cow, boxed::Box, string::String, sync::Arc}; use core::{ error::Error as CoreError, fmt::{Display, Formatter, Result as FmtResult}, @@ -632,7 +627,7 @@ impl Error { pub fn render_message(&self) -> Cow<'_, str> { match &self.message { Some(msg) => Cow::Borrowed(msg.as_ref()), - None => Cow::Owned(self.kind.to_string()) + None => Cow::Borrowed(self.kind.label()) } } diff --git a/src/app_error/tests.rs b/src/app_error/tests.rs index 7e74863..92ed1e6 100644 --- a/src/app_error/tests.rs +++ b/src/app_error/tests.rs @@ -189,6 +189,16 @@ fn bare_sets_kind_without_message() { ); } +#[test] +fn render_message_returns_borrowed_label_for_bare_errors() { + let err = AppError::bare(AppErrorKind::Forbidden); + let rendered = err.render_message(); + assert!(matches!( + rendered, + Cow::Borrowed(label) if label == AppErrorKind::Forbidden.label() + )); +} + #[test] fn retry_and_www_authenticate_are_attached() { let err = AppError::internal("boom") diff --git a/src/kind.rs b/src/kind.rs index b0589f0..e66cc4b 100644 --- a/src/kind.rs +++ b/src/kind.rs @@ -184,7 +184,17 @@ pub enum AppErrorKind { impl Display for AppErrorKind { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - let label = match self { + f.write_str(self.label()) + } +} + +impl CoreError for AppErrorKind {} + +impl AppErrorKind { + /// Human-readable label exposed in HTTP and telemetry payloads. + #[must_use] + pub const fn label(&self) -> &'static str { + match self { Self::NotFound => "Not found", Self::Validation => "Validation error", Self::Conflict => "Conflict", @@ -208,14 +218,9 @@ impl Display for AppErrorKind { Self::ExternalApi => "External API error", Self::Queue => "Queue processing error", Self::Cache => "Cache error" - }; - f.write_str(label) + } } -} -impl CoreError for AppErrorKind {} - -impl AppErrorKind { /// Framework-agnostic mapping to an HTTP status code (`u16`). /// /// This mapping is intentionally conservative and stable. It should **not** diff --git a/src/response/mapping.rs b/src/response/mapping.rs index 9addbbb..c43616d 100644 --- a/src/response/mapping.rs +++ b/src/response/mapping.rs @@ -1,4 +1,4 @@ -use alloc::string::ToString; +use alloc::string::String; use core::fmt::{Display, Formatter, Result as FmtResult}; use super::core::ErrorResponse; @@ -22,7 +22,7 @@ impl From for ErrorResponse { let status = kind.http_status(); let message = match err.message.take() { Some(msg) if !matches!(policy, crate::MessageEditPolicy::Redact) => msg.into_owned(), - _ => kind.to_string() + _ => String::from(kind.label()) }; #[cfg(feature = "serde_json")] let details = if matches!(policy, crate::MessageEditPolicy::Redact) { @@ -52,7 +52,7 @@ impl From<&AppError> for ErrorResponse { fn from(err: &AppError) -> Self { let status = err.kind.http_status(); let message = if matches!(err.edit_policy, crate::MessageEditPolicy::Redact) { - err.kind.to_string() + String::from(err.kind.label()) } else { err.render_message().into_owned() }; diff --git a/src/response/problem_json.rs b/src/response/problem_json.rs index 73ced8f..8c9e1f1 100644 --- a/src/response/problem_json.rs +++ b/src/response/problem_json.rs @@ -173,7 +173,7 @@ impl ProblemJson { let mapping = mapping_for_code(&code); let status = kind.http_status(); - let title = Cow::Owned(kind.to_string()); + let title = Cow::Borrowed(kind.label()); let detail = sanitize_detail(message, kind, edit_policy); let metadata = sanitize_metadata_owned(metadata, edit_policy); @@ -210,7 +210,7 @@ impl ProblemJson { pub fn from_ref(error: &AppError) -> Self { let mapping = mapping_for_code(&error.code); let status = error.kind.http_status(); - let title = Cow::Owned(error.kind.to_string()); + let title = Cow::Borrowed(error.kind.label()); let detail = sanitize_detail_ref(error); let details = sanitize_details_ref(error); let metadata = sanitize_metadata_ref(error.metadata(), error.edit_policy); @@ -263,7 +263,7 @@ impl ProblemJson { Self { type_uri: Some(Cow::Borrowed(mapping.problem_type())), - title: Cow::Owned(mapping.kind().to_string()), + title: Cow::Borrowed(mapping.kind().label()), status, detail, details, @@ -412,7 +412,7 @@ fn sanitize_detail( return None; } - Some(message.unwrap_or_else(|| Cow::Owned(kind.to_string()))) + Some(message.unwrap_or_else(|| Cow::Borrowed(kind.label()))) } fn sanitize_detail_ref(error: &AppError) -> Option> { @@ -420,7 +420,11 @@ fn sanitize_detail_ref(error: &AppError) -> Option> { return None; } - Some(Cow::Owned(error.render_message().into_owned())) + match error.message.as_ref() { + Some(Cow::Borrowed(msg)) => Some(Cow::Borrowed(*msg)), + Some(Cow::Owned(msg)) => Some(Cow::Owned(msg.clone())), + None => Some(Cow::Borrowed(error.kind.label())) + } } #[cfg(feature = "serde_json")] diff --git a/src/response/tests.rs b/src/response/tests.rs index ddd59cc..5213fb7 100644 --- a/src/response/tests.rs +++ b/src/response/tests.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use super::ErrorResponse; use crate::{AppCode, AppError, AppErrorKind, ProblemJson}; @@ -255,7 +257,7 @@ fn from_app_error_uses_default_message_when_none() { let e: ErrorResponse = (&app).into(); assert_eq!(e.status, 500); assert!(matches!(e.code, AppCode::Internal)); - assert_eq!(e.message, AppErrorKind::Internal.to_string()); + assert_eq!(e.message, AppErrorKind::Internal.label()); } #[test] @@ -279,7 +281,7 @@ fn from_owned_app_error_defaults_message_when_absent() { assert_eq!(resp.status, 500); assert!(matches!(resp.code, AppCode::Internal)); - assert_eq!(resp.message, AppErrorKind::Internal.to_string()); + assert_eq!(resp.message, AppErrorKind::Internal.label()); } #[test] @@ -289,7 +291,31 @@ fn from_app_error_bare_uses_kind_display_as_message() { assert_eq!(resp.status, 504); assert!(matches!(resp.code, AppCode::Timeout)); - assert_eq!(resp.message, AppErrorKind::Timeout.to_string()); + assert_eq!(resp.message, AppErrorKind::Timeout.label()); +} + +#[test] +fn problem_json_fallbacks_borrow_bare_labels() { + let owned = ProblemJson::from_app_error(AppError::bare(AppErrorKind::Internal)); + assert!(matches!( + owned.title, + Cow::Borrowed(label) if label == AppErrorKind::Internal.label() + )); + assert!(matches!( + owned.detail, + Some(Cow::Borrowed(label)) if label == AppErrorKind::Internal.label() + )); + + let borrowed_error = AppError::bare(AppErrorKind::Timeout); + let borrowed_problem = ProblemJson::from_ref(&borrowed_error); + assert!(matches!( + borrowed_problem.title, + Cow::Borrowed(label) if label == AppErrorKind::Timeout.label() + )); + assert!(matches!( + borrowed_problem.detail, + Some(Cow::Borrowed(label)) if label == AppErrorKind::Timeout.label() + )); } #[test] @@ -297,11 +323,11 @@ fn from_app_error_redacts_message_when_policy_allows() { let app = AppError::internal("sensitive").redactable(); let resp: ErrorResponse = app.into(); - assert_eq!(resp.message, AppErrorKind::Internal.to_string()); + assert_eq!(resp.message, AppErrorKind::Internal.label()); let borrowed = AppError::internal("private").redactable(); let resp_ref: ErrorResponse = (&borrowed).into(); - assert_eq!(resp_ref.message, AppErrorKind::Internal.to_string()); + assert_eq!(resp_ref.message, AppErrorKind::Internal.label()); } #[test] @@ -310,10 +336,10 @@ fn error_response_serialization_hides_redacted_message() { let resp: ErrorResponse = AppError::internal(secret).redactable().into(); let json = serde_json::to_value(&resp).expect("serialize response"); - let fallback = AppErrorKind::Internal.to_string(); + let fallback = AppErrorKind::Internal.label(); assert_eq!( json.get("message").and_then(|value| value.as_str()), - Some(fallback.as_str()) + Some(fallback) ); assert!(!json.to_string().contains(secret)); }