fix(api): always give xrpc error messages

2026-07-22 07:52:22 +00:00 · 2026-03-18 16:54:28 +02:00
parent ebf5af4922
commit c2b8253abe
5 changed files with 197 additions and 130 deletions
@@ -6094,7 +6094,7 @@ dependencies = [

 [[package]]
 name = "tranquil-api"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "anyhow",
 "axum",
@@ -6142,7 +6142,7 @@ dependencies = [

 [[package]]
 name = "tranquil-auth"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "anyhow",
 "base32",
@@ -6165,7 +6165,7 @@ dependencies = [

 [[package]]
 name = "tranquil-cache"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "async-trait",
 "base64 0.22.1",
@@ -6179,7 +6179,7 @@ dependencies = [

 [[package]]
 name = "tranquil-comms"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "async-trait",
 "base64 0.22.1",
@@ -6194,7 +6194,7 @@ dependencies = [

 [[package]]
 name = "tranquil-config"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "confique",
 "serde",
@@ -6202,7 +6202,7 @@ dependencies = [

 [[package]]
 name = "tranquil-crypto"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "aes-gcm",
 "base64 0.22.1",
@@ -6218,7 +6218,7 @@ dependencies = [

 [[package]]
 name = "tranquil-db"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "async-trait",
 "chrono",
@@ -6235,7 +6235,7 @@ dependencies = [

 [[package]]
 name = "tranquil-db-traits"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "async-trait",
 "base64 0.22.1",
@@ -6251,7 +6251,7 @@ dependencies = [

 [[package]]
 name = "tranquil-infra"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "async-trait",
 "bytes",
@@ -6262,7 +6262,7 @@ dependencies = [

 [[package]]
 name = "tranquil-lexicon"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "chrono",
 "hickory-resolver",
@@ -6280,7 +6280,7 @@ dependencies = [

 [[package]]
 name = "tranquil-oauth"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "anyhow",
 "axum",
@@ -6303,7 +6303,7 @@ dependencies = [

 [[package]]
 name = "tranquil-oauth-server"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "axum",
 "base64 0.22.1",
@@ -6336,7 +6336,7 @@ dependencies = [

 [[package]]
 name = "tranquil-pds"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "aes-gcm",
 "anyhow",
@@ -6424,7 +6424,7 @@ dependencies = [

 [[package]]
 name = "tranquil-repo"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "bytes",
 "cid",
@@ -6436,7 +6436,7 @@ dependencies = [

 [[package]]
 name = "tranquil-ripple"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "async-trait",
 "backon",
@@ -6461,7 +6461,7 @@ dependencies = [

 [[package]]
 name = "tranquil-scopes"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "axum",
 "futures",
@@ -6477,7 +6477,7 @@ dependencies = [

 [[package]]
 name = "tranquil-server"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "axum",
 "clap",
@@ -6497,7 +6497,7 @@ dependencies = [

 [[package]]
 name = "tranquil-storage"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "async-trait",
 "aws-config",
@@ -6514,7 +6514,7 @@ dependencies = [

 [[package]]
 name = "tranquil-sync"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "anyhow",
 "axum",
@@ -6536,7 +6536,7 @@ dependencies = [

 [[package]]
 name = "tranquil-types"
-version = "0.4.3"
+version = "0.4.4"
 dependencies = [
 "chrono",
 "cid",
@@ -9,8 +9,7 @@ use std::borrow::Cow;
 #[derive(Debug, Serialize)]
 struct ErrorBody<'a> {
    error: Cow<'a, str>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    message: Option<String>,
+    message: String,
 }

 #[derive(Debug)]
@@ -214,7 +213,7 @@ impl ApiError {
    }
    fn error_name(&self) -> Cow<'static, str> {
        match self {
-            Self::InternalError(_) | Self::DatabaseError => Cow::Borrowed("InternalError"),
+            Self::InternalError(_) | Self::DatabaseError => Cow::Borrowed("InternalServerError"),
            Self::UpstreamFailure | Self::UpstreamUnavailable(_) | Self::UpstreamErrorMsg(_) => {
                Cow::Borrowed("UpstreamError")
            }
@@ -313,22 +312,47 @@ impl ApiError {
            Self::LegacyLoginBlocked => Cow::Borrowed("MfaRequired"),
        }
    }
-    fn message(&self) -> Option<String> {
+    fn message(&self) -> String {
        match self {
-            Self::InternalError(msg)
-            | Self::AuthenticationFailed(msg)
-            | Self::InvalidToken(msg)
-            | Self::ExpiredToken(msg)
-            | Self::OAuthExpiredToken(msg)
-            | Self::RepoNotFound(msg)
-            | Self::BlobNotFound(msg)
-            | Self::InvalidHandle(msg)
-            | Self::HandleNotAvailable(msg)
-            | Self::InvalidSwap(msg)
-            | Self::InsufficientScope(msg)
-            | Self::InvalidCode(msg)
-            | Self::RateLimitExceeded(msg)
-            | Self::ServiceUnavailable(msg) => msg.clone(),
+            Self::InternalError(msg) => msg
+                .clone()
+                .unwrap_or_else(|| "Internal Server Error".into()),
+            Self::AuthenticationFailed(msg) => msg
+                .clone()
+                .unwrap_or_else(|| "Authentication failed".into()),
+            Self::InvalidToken(msg) => {
+                msg.clone().unwrap_or_else(|| "Invalid token".into())
+            }
+            Self::ExpiredToken(msg) | Self::OAuthExpiredToken(msg) => {
+                msg.clone().unwrap_or_else(|| "Token has expired".into())
+            }
+            Self::RepoNotFound(msg) => msg
+                .clone()
+                .unwrap_or_else(|| "Repository not found".into()),
+            Self::BlobNotFound(msg) => {
+                msg.clone().unwrap_or_else(|| "Blob not found".into())
+            }
+            Self::InvalidHandle(msg) => {
+                msg.clone().unwrap_or_else(|| "Invalid handle".into())
+            }
+            Self::HandleNotAvailable(msg) => msg
+                .clone()
+                .unwrap_or_else(|| "Handle not available".into()),
+            Self::InvalidSwap(msg) => {
+                msg.clone().unwrap_or_else(|| "Invalid swap".into())
+            }
+            Self::InsufficientScope(msg) => msg
+                .clone()
+                .unwrap_or_else(|| "Insufficient scope".into()),
+            Self::InvalidCode(msg) => {
+                msg.clone().unwrap_or_else(|| "Invalid code".into())
+            }
+            Self::RateLimitExceeded(msg) => msg
+                .clone()
+                .unwrap_or_else(|| "Rate limit exceeded".into()),
+            Self::ServiceUnavailable(msg) => msg
+                .clone()
+                .unwrap_or_else(|| "Service temporarily unavailable".into()),
            Self::InvalidRequest(msg)
            | Self::UpstreamUnavailable(msg)
            | Self::InvalidPassword(msg)
@@ -336,109 +360,117 @@ impl ApiError {
            | Self::InvalidRecord(msg)
            | Self::NotFoundMsg(msg)
            | Self::UpstreamErrorMsg(msg)
-            | Self::PayloadTooLarge(msg) => Some(msg.clone()),
-            Self::AccountMigrated => Some(
-                "Account has been migrated to another PDS. Repo operations are not allowed."
-                    .to_string(),
-            ),
-            Self::AccountNotVerified => Some(
-                "You must verify at least one notification channel before creating records"
-                    .to_string(),
-            ),
-            Self::NoPasskeys => {
-                Some("No passkeys registered for this account".to_string())
+            | Self::PayloadTooLarge(msg)
+            | Self::InvalidScopes(msg)
+            | Self::InvalidDelegation(msg)
+            | Self::AuthorizationError(msg)
+            | Self::InvalidDid(msg) => msg.clone(),
+            Self::UpstreamError { message, .. } => message
+                .clone()
+                .unwrap_or_else(|| "Upstream error".into()),
+            Self::DatabaseError => "Internal Server Error".into(),
+            Self::AuthenticationRequired => "Authentication required".into(),
+            Self::TokenRequired => "Authentication token required".into(),
+            Self::AccountDeactivated => "Account is deactivated".into(),
+            Self::AccountTakedown => "Account has been taken down".into(),
+            Self::AccountNotFound => "Account not found".into(),
+            Self::RecordNotFound => "Record not found".into(),
+            Self::Forbidden => "Forbidden".into(),
+            Self::InvitesDisabled => "Invite codes are disabled on this server".into(),
+            Self::InvalidCollection => "Invalid collection".into(),
+            Self::InvalidChannel => "Invalid notification channel".into(),
+            Self::TotpAlreadyEnabled => "TOTP is already enabled".into(),
+            Self::TotpNotEnabled => "TOTP is not enabled".into(),
+            Self::DuplicateAppPassword => "An app password with this name already exists".into(),
+            Self::AppPasswordNotFound => "App password not found".into(),
+            Self::SessionNotFound => "Session not found".into(),
+            Self::UpstreamFailure => "Upstream service failed".into(),
+            Self::RepoTakendown => "Repository has been taken down".into(),
+            Self::RepoDeactivated => "Repository is deactivated".into(),
+            Self::AccountMigrated => {
+                "Account has been migrated to another PDS. Repo operations are not allowed.".into()
            }
-            Self::NoChallengeInProgress => Some(
-                "No passkey authentication in progress or challenge expired".to_string(),
-            ),
-            Self::InvalidCredential => Some("Failed to parse credential response".to_string()),
-            Self::NoRegistrationInProgress => Some(
-                "No registration in progress. Call startPasskeyRegistration first.".to_string(),
-            ),
-            Self::RegistrationFailed => {
-                Some("Failed to verify passkey registration".to_string())
+            Self::AccountNotVerified => {
+                "You must verify at least one notification channel before creating records".into()
            }
-            Self::PasskeyNotFound => Some("Passkey not found".to_string()),
-            Self::InvalidId => Some("Invalid ID format".to_string()),
-            Self::InvalidScopes(msg) | Self::InvalidDelegation(msg) => Some(msg.clone()),
-            Self::ControllerNotFound => Some("Controller account not found".to_string()),
+            Self::NoPasskeys => "No passkeys registered for this account".into(),
+            Self::NoChallengeInProgress => {
+                "No passkey authentication in progress or challenge expired".into()
+            }
+            Self::InvalidCredential => "Failed to parse credential response".into(),
+            Self::NoRegistrationInProgress => {
+                "No registration in progress. Call startPasskeyRegistration first.".into()
+            }
+            Self::RegistrationFailed => "Failed to verify passkey registration".into(),
+            Self::PasskeyNotFound => "Passkey not found".into(),
+            Self::InvalidId => "Invalid ID format".into(),
+            Self::ControllerNotFound => "Controller account not found".into(),
            Self::DelegationNotFound => {
-                Some("No active delegation found for this controller".to_string())
+                "No active delegation found for this controller".into()
            }
            Self::InviteCodeRequired => {
-                Some("An invite code is required to create an account".to_string())
+                "An invite code is required to create an account".into()
            }
-            Self::RepoNotReady => Some("Repository not ready".to_string()),
-            Self::PasskeyCounterAnomaly => Some(
-                "Authentication failed: security key counter anomaly detected. This may indicate a cloned key.".to_string(),
-            ),
-            Self::MfaVerificationRequired => Some(
-                "This sensitive operation requires MFA verification".to_string(),
-            ),
-            Self::DeviceNotFound => Some("Device not found".to_string()),
-            Self::NoEmail => Some("Recipient has no email address".to_string()),
-            Self::AuthorizationError(msg) | Self::InvalidDid(msg) => Some(msg.clone()),
+            Self::RepoNotReady => "Repository not ready".into(),
+            Self::PasskeyCounterAnomaly => {
+                "Authentication failed: security key counter anomaly detected. This may indicate a cloned key.".into()
+            }
+            Self::MfaVerificationRequired => {
+                "This sensitive operation requires MFA verification".into()
+            }
+            Self::DeviceNotFound => "Device not found".into(),
+            Self::NoEmail => "Recipient has no email address".into(),
            Self::InvalidSigningKey => {
-                Some("Signing key not found, already used, or expired".to_string())
-            }
-            Self::SetupExpired => {
-                Some("Setup has already been completed or expired".to_string())
-            }
-            Self::InvalidAccount => {
-                Some("This account is not a passkey-only account".to_string())
-            }
-            Self::InvalidRecoveryLink => Some("Invalid recovery link".to_string()),
-            Self::RecoveryLinkExpired => Some("Recovery link has expired".to_string()),
-            Self::MissingEmail => {
-                Some("Email is required when using email verification".to_string())
+                "Signing key not found, already used, or expired".into()
            }
+            Self::SetupExpired => "Setup has already been completed or expired".into(),
+            Self::InvalidAccount => "This account is not a passkey-only account".into(),
+            Self::InvalidRecoveryLink => "Invalid recovery link".into(),
+            Self::RecoveryLinkExpired => "Recovery link has expired".into(),
+            Self::MissingEmail => "Email is required when using email verification".into(),
            Self::MissingDiscordId => {
-                Some("Discord ID is required when using Discord verification".to_string())
+                "Discord ID is required when using Discord verification".into()
            }
            Self::MissingTelegramUsername => {
-                Some("Telegram username is required when using Telegram verification".to_string())
+                "Telegram username is required when using Telegram verification".into()
            }
            Self::MissingSignalNumber => {
-                Some("Signal username is required when using Signal verification".to_string())
+                "Signal username is required when using Signal verification".into()
            }
-            Self::InvalidVerificationChannel => Some("Invalid verification channel".to_string()),
+            Self::InvalidVerificationChannel => "Invalid verification channel".into(),
            Self::SelfHostedDidWebDisabled => {
-                Some("Self-hosted did:web accounts are disabled on this server".to_string())
-            }
-            Self::AccountAlreadyExists => Some("Account already exists".to_string()),
-            Self::HandleNotFound => Some("Unable to resolve handle".to_string()),
-            Self::SubjectNotFound => Some("Subject not found".to_string()),
-            Self::SsoProviderNotFound => Some("Unknown SSO provider".to_string()),
-            Self::SsoProviderNotEnabled => Some("SSO provider is not enabled".to_string()),
-            Self::SsoInvalidAction => {
-                Some("Action must be login, link, or register".to_string())
+                "Self-hosted did:web accounts are disabled on this server".into()
            }
+            Self::AccountAlreadyExists => "Account already exists".into(),
+            Self::HandleNotFound => "Unable to resolve handle".into(),
+            Self::SubjectNotFound => "Subject not found".into(),
+            Self::SsoProviderNotFound => "Unknown SSO provider".into(),
+            Self::SsoProviderNotEnabled => "SSO provider is not enabled".into(),
+            Self::SsoInvalidAction => "Action must be login, link, or register".into(),
            Self::SsoNotAuthenticated => {
-                Some("Must be authenticated to link SSO account".to_string())
+                "Must be authenticated to link SSO account".into()
            }
-            Self::SsoSessionExpired => Some("SSO session expired or invalid".to_string()),
+            Self::SsoSessionExpired => "SSO session expired or invalid".into(),
            Self::SsoAlreadyLinked => {
-                Some("This SSO account is already linked to a different user".to_string())
+                "This SSO account is already linked to a different user".into()
            }
-            Self::SsoLinkNotFound => Some("Linked account not found".to_string()),
+            Self::SsoLinkNotFound => "Linked account not found".into(),
            Self::IdentifierMismatch => {
-                Some("The identifier does not match the verification token".to_string())
+                "The identifier does not match the verification token".into()
+            }
+            Self::UpstreamTimeout => "Upstream service timed out".into(),
+            Self::AdminRequired => "This action requires admin privileges".into(),
+            Self::EmailTaken => "This email address is already registered".into(),
+            Self::HandleTaken => "This handle is already taken".into(),
+            Self::InvalidEmail => "Please provide a valid email address".into(),
+            Self::InvalidInviteCode => "The invite code provided is invalid".into(),
+            Self::DuplicateCreate => "Account creation failed: duplicate request".into(),
+            Self::LegacyLoginBlocked => {
+                "This account requires MFA. Please use an OAuth client that supports TOTP verification.".into()
            }
-            Self::UpstreamError { message, .. } => message.clone(),
-            Self::UpstreamTimeout => Some("Upstream service timed out".to_string()),
-            Self::AdminRequired => Some("This action requires admin privileges".to_string()),
-            Self::EmailTaken => Some("This email address is already registered".to_string()),
-            Self::HandleTaken => Some("This handle is already taken".to_string()),
-            Self::InvalidEmail => Some("Please provide a valid email address".to_string()),
-            Self::InvalidInviteCode => Some("The invite code provided is invalid".to_string()),
-            Self::DuplicateCreate => Some("Account creation failed: duplicate request".to_string()),
-            Self::LegacyLoginBlocked => Some(
-                "This account requires MFA. Please use an OAuth client that supports TOTP verification.".to_string(),
-            ),
            Self::AuthFactorTokenRequired => {
-                Some("A sign in code has been sent to your email address".to_string())
+                "A sign-in code has been sent to your email address".into()
            }
-            _ => None,
        }
    }
    pub fn from_upstream_response(status: StatusCode, body: &[u8]) -> Self {
@@ -10,7 +10,7 @@ use axum::{
    body::Bytes,
    extract::{RawQuery, Request, State},
    handler::Handler,
-    http::{HeaderMap, Method, StatusCode},
+    http::{HeaderMap, Method},
    response::{IntoResponse, Response},
 };
 use futures_util::future::Either;
@@ -335,7 +335,7 @@ async fn proxy_handler(
                Ok(b) => b,
                Err(e) => {
                    error!("Error reading proxy response body: {:?}", e);
-                    return (StatusCode::BAD_GATEWAY, "Error reading upstream response")
+                    return ApiError::UpstreamUnavailable("Error reading upstream response".into())
                        .into_response();
                }
            };
@@ -350,16 +350,16 @@ async fn proxy_handler(
                Ok(r) => r,
                Err(e) => {
                    error!("Error building proxy response: {:?}", e);
-                    (StatusCode::INTERNAL_SERVER_ERROR, "Internal Server Error").into_response()
+                    ApiError::InternalError(None).into_response()
                }
            }
        }
        Err(e) => {
            error!("Error sending proxy request: {:?}", e);
            if e.is_timeout() {
-                (StatusCode::GATEWAY_TIMEOUT, "Upstream Timeout").into_response()
+                ApiError::UpstreamTimeout.into_response()
            } else {
-                (StatusCode::BAD_GATEWAY, "Upstream Error").into_response()
+                ApiError::UpstreamFailure.into_response()
            }
        }
    }
@@ -100,7 +100,7 @@ pub fn app_with_routes(state: AppState, external: ExternalRoutes) -> Router {
        .layer(DefaultBodyLimit::max(
            tranquil_config::get().server.max_blob_size as usize,
        ))
-        .layer(axum::middleware::map_response(rewrite_422_to_400))
+        .layer(axum::middleware::map_response(rewrite_extractor_errors))
        .layer(middleware::from_fn(metrics::metrics_middleware))
        .layer(
            CorsLayer::new()
@@ -150,8 +150,24 @@ pub fn app_with_routes(state: AppState, external: ExternalRoutes) -> Router {
    router
 }

-async fn rewrite_422_to_400(response: axum::response::Response) -> axum::response::Response {
-    if response.status() != StatusCode::UNPROCESSABLE_ENTITY {
+fn is_plain_text(headers: &http::HeaderMap) -> bool {
+    headers
+        .get(http::header::CONTENT_TYPE)
+        .and_then(|v| v.to_str().ok())
+        .is_some_and(|ct| ct.starts_with("text/plain"))
+}
+
+fn should_rewrite_to_xrpc_error(response: &axum::response::Response) -> bool {
+    match response.status() {
+        StatusCode::UNPROCESSABLE_ENTITY => true,
+        StatusCode::BAD_REQUEST => is_plain_text(response.headers()),
+        StatusCode::UNSUPPORTED_MEDIA_TYPE => is_plain_text(response.headers()),
+        _ => false,
+    }
+}
+
+async fn rewrite_extractor_errors(response: axum::response::Response) -> axum::response::Response {
+    if !should_rewrite_to_xrpc_error(&response) {
        return response;
    }
    let (mut parts, body) = response.into_parts();
@@ -173,11 +189,11 @@ async fn rewrite_422_to_400(response: axum::response::Response) -> axum::respons
        .unwrap_or_else(|| {
            String::from_utf8(bytes.to_vec()).unwrap_or_else(|_| "Invalid request body".into())
        });
-    let message = humanize_json_error(&raw);
+    let message = humanize_extraction_error(&raw);

    parts.status = StatusCode::BAD_REQUEST;
    parts.headers.remove(http::header::CONTENT_LENGTH);
-    let error_name = classify_deserialization_error(&raw);
+    let error_name = classify_extraction_error(&raw);
    let new_body = json!({
        "error": error_name,
        "message": message
@@ -188,7 +204,7 @@ async fn rewrite_422_to_400(response: axum::response::Response) -> axum::respons
    )
 }

-fn humanize_json_error(raw: &str) -> String {
+fn humanize_extraction_error(raw: &str) -> String {
    if raw.contains("missing field") {
        raw.split("missing field `")
            .nth(1)
@@ -201,14 +217,21 @@ fn humanize_json_error(raw: &str) -> String {
        "Invalid JSON syntax".to_string()
    } else if raw.contains("Content-Type") || raw.contains("content type") {
        "Content-Type must be application/json".to_string()
+    } else if raw.contains("Failed to parse") || raw.contains("expected ident") {
+        "Invalid JSON in request body".to_string()
+    } else if raw.contains("Failed to deserialize query string") {
+        raw.strip_prefix("Failed to deserialize query string: ")
+            .map(|rest| format!("Invalid query parameter: {}", rest))
+            .unwrap_or_else(|| "Invalid query parameters".into())
    } else {
        raw.to_string()
    }
 }

-fn classify_deserialization_error(raw: &str) -> &'static str {
+fn classify_extraction_error(raw: &str) -> &'static str {
    match raw {
        s if s.contains("invalid handle") => "InvalidHandle",
+        s if s.contains("invalid CID") || s.contains("invalid cid") => "InvalidRequest",
        _ => "InvalidRequest",
    }
 }
@@ -86,6 +86,18 @@ import type {

 const API_BASE = "/xrpc";

+const STATUS_FALLBACK_MESSAGE: Record<number, string> = {
+  400: "Bad request",
+  401: "Authentication required",
+  403: "Forbidden",
+  404: "Not found",
+  429: "Rate limit exceeded",
+  500: "Internal server error",
+  502: "Bad gateway",
+  503: "Service unavailable",
+  504: "Gateway timeout",
+};
+
 export class ApiError extends Error {
  public did?: Did;
  public reauthMethods?: string[];
@@ -96,7 +108,7 @@ export class ApiError extends Error {
    did?: string,
    reauthMethods?: string[],
  ) {
-    super(message);
+    super(message ?? STATUS_FALLBACK_MESSAGE[status] ?? "Request failed");
    this.name = "ApiError";
    this.did = did ? unsafeAsDid(did) : undefined;
    this.reauthMethods = reauthMethods;