465fbe2480
Step 1 of docs/plan/jobs-handling-recoverable-error.md, and the blocker
for the rest of it: the engine cannot retry-then-pause until it can tell
"the provider is down" from "this data is wrong". Both arrived as
`ErrorKind::InternalError`, so the distinction survived only inside a
formatted message.
`RetryBlobBackend` was reading that message. Literally:
let msg = err.to_string().to_lowercase();
msg.contains("timeout") || msg.contains("503") || msg.contains("reset by peer")
Fragile in a specific way — an SDK reformatting its `Display` turns
retrying off with nothing failing to say so — and blind to any status
code that never made it into the text.
Adds `ErrorKind::TransientBackend` and `DomainError::is_transient()`.
One predicate, so the retry decorator and the job engine cannot classify
the same failure differently. `Timeout` counts (transient by
construction); everything else must say so explicitly. The default is
"not retryable" because that fails visibly, whereas retrying a permanent
fault burns attempts and — once the engine wires this up — holds
`migration_readonly` while it does.
A kind rather than a `transient: bool` field: 21 struct-literal sites
construct `DomainError` directly and would all have needed touching for
a change that is conceptually about classification. The plan allowed
either.
`s3_domain_error` does the classification where the status is still in
hand. Transient: 5xx, 429, and the SlowDown / RequestTimeout /
ThrottlingException codes that arrive as 400 (status alone is not
enough), plus dispatch-level I/O and timeouts. Permanent: other 4xx —
credentials, missing bucket, malformed request — and construction
failures. `ResponseError` counts as transient since truncation on the
wire is the usual cause and the attempt cap bounds being wrong.
Applied at the five S3 sites that wrap an SDK error, including
`ListObjectsV2` — `backend_consistency` fails the whole run on an
enumeration error, so a throttle midway through a large bucket should be
retryable rather than discarding the sweep.
The exhaustive `ErrorKind` match in `interfaces/errors.rs` forced the
HTTP decision, which is the right friction: 503, not 500. The request
was fine and may succeed shortly, which is what a caller needs to decide
whether to retry and what a proxy keys off to avoid caching the failure.
The substring matcher stays for now, behind the typed check, with the
deletion condition written down: it goes when every backend wrapping a
remote SDK error classifies at the point of wrapping. Removing it before
then would silently reduce retrying on the unconverted backends, which
is the worse direction. Azure is the one left, and it is queued for the
official-SDK migration anyway.
Not yet wired: `RunOutcome::PausedRetryable` (step 2) and the engine's
bounded backoff (step 3). This commit only makes the distinction
representable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
181 lines
6.4 KiB
Rust
181 lines
6.4 KiB
Rust
//! HTTP/API Error types for the interfaces layer.
|
|
//!
|
|
//! This module contains error types specific to the HTTP/API layer.
|
|
//! These errors handle the conversion from domain errors to HTTP responses.
|
|
|
|
use std::borrow::Cow;
|
|
|
|
use axum::Json;
|
|
use axum::http::StatusCode;
|
|
use axum::response::{IntoResponse, Response};
|
|
use serde::Serialize;
|
|
|
|
use crate::domain::errors::{DomainError, ErrorKind};
|
|
|
|
/// Error type for HTTP/API responses.
|
|
///
|
|
/// This struct represents errors that will be returned to HTTP clients.
|
|
/// It contains the HTTP status code, a user-friendly message, and an error
|
|
/// type identifier. `error_type` is a `Cow`: every built-in constructor and
|
|
/// the `DomainError` conversion use a `&'static str` from a closed set, so
|
|
/// the common 4xx path allocates nothing for it (benches/ROUND11.md §9).
|
|
#[derive(Debug)]
|
|
pub struct AppError {
|
|
pub status_code: StatusCode,
|
|
pub message: String,
|
|
pub error_type: Cow<'static, str>,
|
|
}
|
|
|
|
/// JSON response structure for errors, borrowing from the `AppError` it
|
|
/// renders — `error` and `message` intentionally serialize the SAME string
|
|
/// for backwards compatibility (legacy handlers returned `{"error": …}`,
|
|
/// AppError returned `{"message": …}`); serializing one buffer twice
|
|
/// replaces the old per-response deep clone.
|
|
#[derive(Serialize)]
|
|
pub struct ErrorResponse<'a> {
|
|
pub status: &'a str,
|
|
pub error: &'a str,
|
|
pub message: &'a str,
|
|
pub error_type: &'a str,
|
|
}
|
|
|
|
impl AppError {
|
|
/// Create a new AppError with custom status code, message and error type.
|
|
pub fn new(
|
|
status_code: StatusCode,
|
|
message: impl Into<String>,
|
|
error_type: impl Into<Cow<'static, str>>,
|
|
) -> Self {
|
|
Self {
|
|
status_code,
|
|
message: message.into(),
|
|
error_type: error_type.into(),
|
|
}
|
|
}
|
|
|
|
/// Create a 400 Bad Request error.
|
|
pub fn bad_request(message: impl Into<String>) -> Self {
|
|
Self::new(StatusCode::BAD_REQUEST, message, "BadRequest")
|
|
}
|
|
|
|
/// Create a 401 Unauthorized error.
|
|
pub fn unauthorized(message: impl Into<String>) -> Self {
|
|
Self::new(StatusCode::UNAUTHORIZED, message, "Unauthorized")
|
|
}
|
|
|
|
/// Create a 403 Forbidden error.
|
|
pub fn forbidden(message: impl Into<String>) -> Self {
|
|
Self::new(StatusCode::FORBIDDEN, message, "Forbidden")
|
|
}
|
|
|
|
/// Create a 404 Not Found error.
|
|
pub fn not_found(message: impl Into<String>) -> Self {
|
|
Self::new(StatusCode::NOT_FOUND, message, "NotFound")
|
|
}
|
|
|
|
/// Create a 500 Internal Server Error.
|
|
pub fn internal_error(message: impl Into<String>) -> Self {
|
|
Self::new(StatusCode::INTERNAL_SERVER_ERROR, message, "InternalError")
|
|
}
|
|
|
|
/// Create a 405 Method Not Allowed error.
|
|
pub fn method_not_allowed(message: impl Into<String>) -> Self {
|
|
Self::new(StatusCode::METHOD_NOT_ALLOWED, message, "MethodNotAllowed")
|
|
}
|
|
|
|
/// Create a 409 Conflict error.
|
|
pub fn conflict(message: impl Into<String>) -> Self {
|
|
Self::new(StatusCode::CONFLICT, message, "Conflict")
|
|
}
|
|
|
|
/// Create a 423 Locked error (WebDAV).
|
|
pub fn locked(message: impl Into<String>) -> Self {
|
|
Self::new(StatusCode::LOCKED, message, "Locked")
|
|
}
|
|
|
|
/// Create a 415 Unsupported Media Type error.
|
|
pub fn unsupported_media_type(message: impl Into<String>) -> Self {
|
|
Self::new(
|
|
StatusCode::UNSUPPORTED_MEDIA_TYPE,
|
|
message,
|
|
"UnsupportedMediaType",
|
|
)
|
|
}
|
|
|
|
/// Create a 412 Precondition Failed error.
|
|
pub fn precondition_failed(message: impl Into<String>) -> Self {
|
|
Self::new(
|
|
StatusCode::PRECONDITION_FAILED,
|
|
message,
|
|
"PreconditionFailed",
|
|
)
|
|
}
|
|
|
|
/// Create a 413 Payload Too Large error.
|
|
pub fn payload_too_large(message: impl Into<String>) -> Self {
|
|
Self::new(StatusCode::PAYLOAD_TOO_LARGE, message, "PayloadTooLarge")
|
|
}
|
|
}
|
|
|
|
impl From<DomainError> for AppError {
|
|
fn from(err: DomainError) -> Self {
|
|
let status_code = match err.kind {
|
|
ErrorKind::NotFound => StatusCode::NOT_FOUND,
|
|
ErrorKind::AlreadyExists => StatusCode::CONFLICT,
|
|
ErrorKind::InvalidInput => StatusCode::BAD_REQUEST,
|
|
ErrorKind::AccessDenied => StatusCode::FORBIDDEN,
|
|
ErrorKind::Timeout => StatusCode::REQUEST_TIMEOUT,
|
|
ErrorKind::InternalError => StatusCode::INTERNAL_SERVER_ERROR,
|
|
ErrorKind::NotImplemented => StatusCode::NOT_IMPLEMENTED,
|
|
ErrorKind::UnsupportedOperation => StatusCode::METHOD_NOT_ALLOWED,
|
|
ErrorKind::DatabaseError => StatusCode::INTERNAL_SERVER_ERROR,
|
|
ErrorKind::QuotaExceeded => StatusCode::INSUFFICIENT_STORAGE,
|
|
ErrorKind::Conflict => StatusCode::CONFLICT,
|
|
ErrorKind::PreconditionFailed => StatusCode::PRECONDITION_FAILED,
|
|
// 503, not 500: the request was fine and the same request
|
|
// may well succeed shortly. That is what a caller needs to
|
|
// decide whether to retry, and it is what a reverse proxy
|
|
// keys off to avoid caching the failure.
|
|
ErrorKind::TransientBackend => StatusCode::SERVICE_UNAVAILABLE,
|
|
};
|
|
|
|
Self {
|
|
status_code,
|
|
message: err.message,
|
|
error_type: Cow::Borrowed(err.kind.as_str()),
|
|
}
|
|
}
|
|
}
|
|
|
|
impl IntoResponse for AppError {
|
|
fn into_response(self) -> Response {
|
|
let status = self.status_code;
|
|
|
|
// Sanitize 500 Internal Server Error to prevent information leakage.
|
|
// Log the full error server-side for debugging, return a generic
|
|
// message to the client. Other status codes (including 5xx like
|
|
// 501, 503, 507) keep their intentionally user-facing messages.
|
|
let client_message: &str = if status == StatusCode::INTERNAL_SERVER_ERROR {
|
|
tracing::error!(
|
|
error_type = %self.error_type,
|
|
"Internal server error: {}",
|
|
self.message
|
|
);
|
|
"An internal error occurred. Please try again later."
|
|
} else {
|
|
&self.message
|
|
};
|
|
|
|
let status_line = status.to_string();
|
|
let error_response = ErrorResponse {
|
|
status: &status_line,
|
|
error: client_message,
|
|
message: client_message,
|
|
error_type: &self.error_type,
|
|
};
|
|
|
|
let body = Json(&error_response);
|
|
(status, body).into_response()
|
|
}
|
|
}
|