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
9 changes: 2 additions & 7 deletions src/app_error/core.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand Down Expand Up @@ -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())
}
}

Expand Down
10 changes: 10 additions & 0 deletions src/app_error/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
19 changes: 12 additions & 7 deletions src/kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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**
Expand Down
6 changes: 3 additions & 3 deletions src/response/mapping.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use alloc::string::ToString;
use alloc::string::String;
use core::fmt::{Display, Formatter, Result as FmtResult};

use super::core::ErrorResponse;
Expand All @@ -22,7 +22,7 @@ impl From<AppError> 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) {
Expand Down Expand Up @@ -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()
};
Expand Down
14 changes: 9 additions & 5 deletions src/response/problem_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -412,15 +412,19 @@ 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<Cow<'static, str>> {
if matches!(error.edit_policy, MessageEditPolicy::Redact) {
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")]
Expand Down
40 changes: 33 additions & 7 deletions src/response/tests.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::borrow::Cow;

use super::ErrorResponse;
use crate::{AppCode, AppError, AppErrorKind, ProblemJson};

Expand Down Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -289,19 +291,43 @@ 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]
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]
Expand All @@ -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));
}
Expand Down
Loading