diff --git a/nodedb-types/src/error/sqlstate.rs b/nodedb-types/src/error/sqlstate.rs index cc8a8899c..38950ffbd 100644 --- a/nodedb-types/src/error/sqlstate.rs +++ b/nodedb-types/src/error/sqlstate.rs @@ -49,9 +49,18 @@ pub const CANNOT_DROP_DEFAULT_DATABASE: AmbiguousSqlstate = AmbiguousSqlstate("0 // ── Class 22 — Data Exception ──────────────────────────────────────────────── +/// `22000` — `data_exception` (a value-level failure whose specific subclass +/// the producer does not name — the array family) +pub const DATA_EXCEPTION: &str = "22000"; + /// `22003` — `numeric_value_out_of_range` pub const NUMERIC_VALUE_OUT_OF_RANGE: &str = "22003"; +/// `22P02` — `invalid_text_representation` (a payload that cannot be decoded +/// into the representation the reader expected — the response shaper's +/// `SERIALIZATION` code) +pub const INVALID_TEXT_REPRESENTATION: &str = "22P02"; + /// `22012` — `division_by_zero` (`/` or `%` with a zero divisor — /// raised at runtime instead of evaluating to `NULL`) pub const DIVISION_BY_ZERO: &str = "22012"; @@ -109,11 +118,23 @@ pub const TYPE_GUARD_VIOLATION: &str = "23608"; /// status column; a misconfigured column name, not a locked period. pub const PERIOD_LOCK_MISCONFIGURED: &str = "23609"; +// ── Class 25 — Invalid Transaction State ───────────────────────────────────── + +/// `25006` — `read_only_sql_transaction` (a write against a mirror that is +/// read-only by design) +pub const READ_ONLY_SQL_TRANSACTION: &str = "25006"; + // ── Class 28 — Invalid Authorization Specification ─────────────────────────── /// `28000` — `invalid_authorization_specification` (no valid credentials) pub const INVALID_AUTHORIZATION: &str = "28000"; +// ── Class 2B — Dependent Privilege Descriptors Still Exist ─────────────────── + +/// `2BP01` — `dependent_objects_still_exist` (the built-in `default` database +/// cannot be dropped) +pub const DEPENDENT_OBJECTS_STILL_EXIST: &str = "2BP01"; + // ── Class 3D — Invalid Catalog Name ────────────────────────────────────────── /// `3D000` — `invalid_catalog_name` (the selected database does not exist) @@ -133,6 +154,9 @@ pub const SERIALIZATION_FAILURE: &str = "40001"; /// `42501` — `insufficient_privilege` pub const INSUFFICIENT_PRIVILEGE: &str = "42501"; +/// `42P07` — `duplicate_table` (the named object already exists) +pub const DUPLICATE_TABLE: &str = "42P07"; + /// `42601` — `syntax_error` pub const SYNTAX_ERROR: &str = "42601"; @@ -332,6 +356,10 @@ pub const IO_ERROR: &str = "58030"; /// `XX000` — `internal_error` pub const INTERNAL_ERROR: &str = "XX000"; +/// `XX001` — `data_corrupted` (a segment or encrypted page failed its +/// integrity check) +pub const DATA_CORRUPTED: &str = "XX001"; + #[cfg(test)] mod tests { use super::*; @@ -390,6 +418,12 @@ mod tests { STALE_READ_NOT_LEADER, CONNECTION_FAILURE, IO_ERROR, + DATA_EXCEPTION, + INVALID_TEXT_REPRESENTATION, + READ_ONLY_SQL_TRANSACTION, + DEPENDENT_OBJECTS_STILL_EXIST, + DUPLICATE_TABLE, + DATA_CORRUPTED, ]; for code in &codes { assert_eq!( diff --git a/nodedb/src/control/server/pgwire/types/error_map.rs b/nodedb/src/control/server/pgwire/types/error_map.rs index fbd48e481..5a8986e76 100644 --- a/nodedb/src/control/server/pgwire/types/error_map.rs +++ b/nodedb/src/control/server/pgwire/types/error_map.rs @@ -40,8 +40,15 @@ pub fn error_to_pg(err: &crate::Error) -> PgWireError { /// client reads, with the SQLSTATE its numeric code maps to. A per-row /// sequence accessor refusal (`42704`, `55000`) or a division by zero /// (`22012`) keeps its class instead of collapsing to `XX000`. +/// +/// An `XX000`-class failure describes server state, so its detail stays in +/// the server log and the client reads one stable summary +/// ([`super::shaping_error_message`]); every other class keeps its message. pub fn shape_error_to_pg(e: &nodedb_types::NodeDbError) -> PgWireError { - sqlstate_error(numeric_code_to_sqlstate(e.code()), e.message()) + sqlstate_error( + numeric_code_to_sqlstate(e.code()), + &super::shaping_error_message(e.code(), e.message()), + ) } /// Map a NodeDB `Error` to a PostgreSQL SQLSTATE code + message. @@ -218,7 +225,14 @@ pub fn error_to_sqlstate(err: &crate::Error) -> (&'static str, &'static str, Str numeric_code_to_sqlstate(e.code()), e.message().to_string(), ), - _ => ("ERROR", sqlstate::INTERNAL_ERROR, err.to_string()), + // A variant with no arm of its own carries its class on the error: + // `classify` borrows it (the crate's one Error-to-NodeDbError map), + // then this table answers instead of a blanket `XX000`. + _ => ( + "ERROR", + numeric_code_to_sqlstate(crate::error_classify::classify(err).code()), + err.to_string(), + ), } } @@ -273,6 +287,76 @@ pub(crate) fn numeric_code_to_sqlstate(code: nodedb_types::error::ErrorCode) -> Ec::NOT_LEADER => sqlstate::DATABASE_DROPPED, // Mirrors the `CloneWriteRequiresMaterialize` arm. Ec::CLONE_WRITE_REQUIRES_MATERIALIZE => sqlstate::CLONE_WRITE_REQUIRES_MATERIALIZE.0, + // ── Completion of the classified set ────────────────────────────── + // + // The families below existed in the `crate::Error` table but not + // here, so a remote or shaper-raised code in one of them collapsed to + // XX000 at every routed surface. + // + // Constraint family — mirrors the Data Plane table. + Ec::APPEND_ONLY_VIOLATION => sqlstate::APPEND_ONLY_VIOLATION, + Ec::BALANCE_VIOLATION => sqlstate::BALANCE_VIOLATION, + Ec::INSUFFICIENT_BALANCE => sqlstate::CHECK_VIOLATION, + Ec::PERIOD_LOCKED => sqlstate::PERIOD_LOCKED, + Ec::PERIOD_LOCK_MISCONFIGURED => sqlstate::PERIOD_LOCK_MISCONFIGURED, + Ec::PREVALIDATION_REJECTED => sqlstate::CHECK_VIOLATION, + Ec::RETENTION_VIOLATION => sqlstate::RETENTION_VIOLATION, + Ec::LEGAL_HOLD_ACTIVE => sqlstate::LEGAL_HOLD_ACTIVE, + Ec::STATE_TRANSITION_VIOLATION => sqlstate::STATE_TRANSITION_VIOLATION, + Ec::TRANSITION_CHECK_VIOLATION => sqlstate::TRANSITION_CHECK_VIOLATION, + Ec::TYPE_GUARD_VIOLATION => sqlstate::TYPE_GUARD_VIOLATION, + Ec::TYPE_MISMATCH => sqlstate::DATATYPE_MISMATCH, + // The shaper's own code: a payload that cannot be decoded into the + // shape the projection requires. + Ec::SERIALIZATION => sqlstate::INVALID_TEXT_REPRESENTATION, + Ec::CODEC => sqlstate::INVALID_TEXT_REPRESENTATION, + Ec::ARRAY => sqlstate::DATA_EXCEPTION, + Ec::OVERFLOW => sqlstate::NUMERIC_VALUE_OUT_OF_RANGE, + // Read-path absence. + Ec::NOT_FOUND => sqlstate::NO_DATA, + Ec::DATABASE_NOT_FOUND => sqlstate::INVALID_CATALOG_NAME, + // Quota and admission. + Ec::QUOTA_OVERCOMMIT => sqlstate::QUOTA_OVERCOMMIT, + Ec::TENANT_QUOTA_EXCEEDED | Ec::DATABASE_QUOTA_EXCEEDED => sqlstate::QUOTA_EXCEEDED, + Ec::TENANT_VECTOR_DIM_EXCEEDED | Ec::TENANT_GRAPH_DEPTH_EXCEEDED => { + sqlstate::PROGRAM_LIMIT_EXCEEDED + } + Ec::SERVER_OVERLOAD => sqlstate::SERVER_OVERLOAD, + Ec::MIGRATION_IN_PROGRESS | Ec::COLLECTION_DRAINING => sqlstate::CANNOT_CONNECT_NOW, + // Clone family. + Ec::ALREADY_EXISTS => sqlstate::DUPLICATE_TABLE, + Ec::CANNOT_CLONE_MIRROR => sqlstate::FEATURE_NOT_SUPPORTED, + Ec::CLONE_DEPENDENCY => sqlstate::OBJECT_NOT_IN_PREREQUISITE_STATE, + Ec::CLONE_DEPTH_EXCEEDED => sqlstate::CLONE_DEPTH_EXCEEDED, + Ec::CLONE_PREDATES_QUERY_TIME => sqlstate::CLONE_PREDATES_QUERY_TIME, + // Move-tenant family. + Ec::MOVE_TENANT_DRAIN_TIMEOUT => sqlstate::QUERY_CANCELED, + Ec::MOVE_TENANT_PREFLIGHT_FAILED => sqlstate::MOVE_TENANT_PREFLIGHT_FAILED, + Ec::MOVE_TENANT_SNAPSHOT_FAILED + | Ec::MOVE_TENANT_CUTOVER_FAILED + | Ec::MOVE_TENANT_ALREADY_AT_TARGET => sqlstate::OBJECT_NOT_IN_PREREQUISITE_STATE, + // Mirror / read-only family. + Ec::MIRROR_READ_ONLY => sqlstate::READ_ONLY_SQL_TRANSACTION, + Ec::MIRROR_NOT_PROMOTED => sqlstate::OBJECT_NOT_IN_PREREQUISITE_STATE, + Ec::STALE_READ_NOT_LEADER => sqlstate::STALE_READ_NOT_LEADER, + Ec::CANNOT_DROP_DEFAULT_DATABASE => sqlstate::DEPENDENT_OBJECTS_STILL_EXIST, + // Credentials and cluster connectivity. + Ec::AUTH_EXPIRED | Ec::BACKUP_KEY_MISMATCH => sqlstate::INVALID_AUTHORIZATION, + Ec::BACKUP_TENANT_MISMATCH => sqlstate::BACKUP_TENANT_MISMATCH, + Ec::HANDSHAKE_FAILED + | Ec::SYNC_CONNECTION_FAILED + | Ec::SHAPE_SUBSCRIPTION_FAILED + | Ec::NODE_UNREACHABLE + | Ec::CLUSTER => sqlstate::CONNECTION_FAILURE, + Ec::SYNC_DELTA_REJECTED => sqlstate::SERIALIZATION_FAILURE, + // Storage and durability. + Ec::STORAGE | Ec::COLD_STORAGE | Ec::WAL => sqlstate::IO_ERROR, + Ec::SEGMENT_CORRUPTED | Ec::ENCRYPTION => sqlstate::DATA_CORRUPTED, + Ec::CONFIG => sqlstate::OBJECT_NOT_IN_PREREQUISITE_STATE, + Ec::SQL_NOT_ENABLED => sqlstate::FEATURE_NOT_SUPPORTED, + // These three stay internal by decision: the explicit arms document + // that the fallback is intended, not an unmapped code. + Ec::INTERNAL | Ec::BRIDGE | Ec::DISPATCH => sqlstate::INTERNAL_ERROR, _ => sqlstate::INTERNAL_ERROR, } } @@ -302,3 +386,216 @@ pub fn response_status_to_sqlstate( } } } + +#[cfg(test)] +mod tests { + use super::*; + use nodedb_types::error::ErrorCode as Ec; + + /// The completion table: every code the mapper previously left on + /// the `XX000` fallback now answers with its decided class. + #[test] + fn completion_codes_keep_their_class() { + let cases = [ + (Ec::APPEND_ONLY_VIOLATION, "23601"), + (Ec::BALANCE_VIOLATION, "23602"), + (Ec::PERIOD_LOCKED, "23603"), + (Ec::STATE_TRANSITION_VIOLATION, "23604"), + (Ec::TRANSITION_CHECK_VIOLATION, "23605"), + (Ec::RETENTION_VIOLATION, "23606"), + (Ec::LEGAL_HOLD_ACTIVE, "23607"), + (Ec::TYPE_GUARD_VIOLATION, "23608"), + (Ec::PERIOD_LOCK_MISCONFIGURED, "23609"), + (Ec::INSUFFICIENT_BALANCE, "23514"), + (Ec::PREVALIDATION_REJECTED, "23514"), + (Ec::TYPE_MISMATCH, "42804"), + (Ec::SERIALIZATION, "22P02"), + (Ec::CODEC, "22P02"), + (Ec::ARRAY, "22000"), + (Ec::OVERFLOW, "22003"), + (Ec::NOT_FOUND, "02000"), + (Ec::DATABASE_NOT_FOUND, "3D000"), + (Ec::QUOTA_OVERCOMMIT, "53400"), + (Ec::TENANT_QUOTA_EXCEEDED, "53400"), + (Ec::DATABASE_QUOTA_EXCEEDED, "53400"), + (Ec::TENANT_VECTOR_DIM_EXCEEDED, "54000"), + (Ec::TENANT_GRAPH_DEPTH_EXCEEDED, "54000"), + (Ec::SERVER_OVERLOAD, "57P03"), + (Ec::MIGRATION_IN_PROGRESS, "57P03"), + (Ec::COLLECTION_DRAINING, "57P03"), + (Ec::ALREADY_EXISTS, "42P07"), + (Ec::CANNOT_CLONE_MIRROR, "0A000"), + (Ec::SQL_NOT_ENABLED, "0A000"), + (Ec::CLONE_DEPENDENCY, "55000"), + (Ec::CLONE_DEPTH_EXCEEDED, "54011"), + (Ec::CLONE_PREDATES_QUERY_TIME, "22023"), + (Ec::MOVE_TENANT_DRAIN_TIMEOUT, "57014"), + (Ec::MOVE_TENANT_PREFLIGHT_FAILED, "55P02"), + (Ec::MOVE_TENANT_SNAPSHOT_FAILED, "55000"), + (Ec::MOVE_TENANT_CUTOVER_FAILED, "55000"), + (Ec::MOVE_TENANT_ALREADY_AT_TARGET, "55000"), + (Ec::MIRROR_READ_ONLY, "25006"), + (Ec::MIRROR_NOT_PROMOTED, "55000"), + (Ec::STALE_READ_NOT_LEADER, "55P03"), + (Ec::CANNOT_DROP_DEFAULT_DATABASE, "2BP01"), + (Ec::AUTH_EXPIRED, "28000"), + (Ec::BACKUP_KEY_MISMATCH, "28000"), + (Ec::BACKUP_TENANT_MISMATCH, "22023"), + (Ec::HANDSHAKE_FAILED, "08006"), + (Ec::SYNC_CONNECTION_FAILED, "08006"), + (Ec::SHAPE_SUBSCRIPTION_FAILED, "08006"), + (Ec::NODE_UNREACHABLE, "08006"), + (Ec::CLUSTER, "08006"), + (Ec::SYNC_DELTA_REJECTED, "40001"), + (Ec::STORAGE, "58030"), + (Ec::COLD_STORAGE, "58030"), + (Ec::WAL, "58030"), + (Ec::SEGMENT_CORRUPTED, "XX001"), + (Ec::ENCRYPTION, "XX001"), + (Ec::CONFIG, "55000"), + ]; + for (code, expected) in cases { + assert_eq!( + numeric_code_to_sqlstate(code), + expected, + "code {} must keep its decided class", + code.0 + ); + } + } + + /// The three codes that are internal by decision keep the fallback, as an + /// explicit arm rather than an implied one. + #[test] + fn internal_codes_stay_internal() { + for code in [Ec::INTERNAL, Ec::BRIDGE, Ec::DISPATCH] { + assert_eq!(numeric_code_to_sqlstate(code), sqlstate::INTERNAL_ERROR); + } + } + + /// An `XX000`-class shaping failure renders the stable summary; the detail + /// stays in the log (enforced inside the helper `shape_error_to_pg` uses). + #[test] + fn shaping_internal_detail_is_not_rendered() { + let error = nodedb_types::NodeDbError::internal( + "manifest at /var/lib/nodedb/segment-42 is corrupt", + ); + let message = super::super::shaping_error_message(error.code(), error.message()); + + assert_eq!(message, "internal error while shaping the response"); + assert!(!message.contains("segment-42")); + } + + /// A client-class shaping failure keeps its actionable message. + #[test] + fn shaping_client_message_passes_through() { + let error = nodedb_types::NodeDbError::serialization( + "cell", + "column \"ts\" holds an integer where a timestamp is required", + ); + let message = super::super::shaping_error_message(error.code(), error.message()); + + assert!(message.contains("timestamp")); + } + + /// A variant with no arm of its own still carries a class: the fallback + /// borrows it through `classify` and this table answers it. + #[test] + fn arm_less_variants_keep_their_class_through_the_fallback() { + use crate::Error; + + let cases = vec![ + ( + Error::AppendOnlyViolation { + collection: "c".into(), + detail: "d".into(), + }, + sqlstate::APPEND_ONLY_VIOLATION, + ), + ( + Error::BalanceViolation { + collection: "c".into(), + detail: "d".into(), + }, + sqlstate::BALANCE_VIOLATION, + ), + ( + Error::InsufficientBalance { + collection: "c".into(), + key: "k".into(), + detail: "d".into(), + }, + sqlstate::CHECK_VIOLATION, + ), + ( + Error::PeriodLocked { + collection: "c".into(), + detail: "d".into(), + }, + sqlstate::PERIOD_LOCKED, + ), + ( + Error::RetentionViolation { + collection: "c".into(), + detail: "d".into(), + }, + sqlstate::RETENTION_VIOLATION, + ), + ( + Error::LegalHoldActive { + collection: "c".into(), + detail: "d".into(), + }, + sqlstate::LEGAL_HOLD_ACTIVE, + ), + ( + Error::StateTransitionViolation { + collection: "c".into(), + detail: "d".into(), + }, + sqlstate::STATE_TRANSITION_VIOLATION, + ), + ( + Error::TransitionCheckViolation { + collection: "c".into(), + detail: "d".into(), + }, + sqlstate::TRANSITION_CHECK_VIOLATION, + ), + ( + Error::TypeGuardViolation { + collection: "c".into(), + detail: "d".into(), + }, + sqlstate::TYPE_GUARD_VIOLATION, + ), + ( + Error::TypeMismatch { + collection: "c".into(), + key: "k".into(), + detail: "d".into(), + }, + sqlstate::DATATYPE_MISMATCH, + ), + ( + Error::MirrorReadOnly { + database: "db".into(), + }, + sqlstate::READ_ONLY_SQL_TRANSACTION, + ), + ( + // Retryable in the lease paths, so the fallback must not read + // as a plan/syntax error now that it answers through `classify`. + Error::RetryableSchemaChanged { + descriptor: "users".into(), + }, + sqlstate::SERIALIZATION_FAILURE, + ), + ]; + + for (err, expected) in cases { + let (_severity, state, _message) = error_to_sqlstate(&err); + assert_eq!(state, expected, "{err:?}"); + } + } +} diff --git a/nodedb/src/control/server/pgwire/types/internal_message.rs b/nodedb/src/control/server/pgwire/types/internal_message.rs new file mode 100644 index 000000000..e2415085b --- /dev/null +++ b/nodedb/src/control/server/pgwire/types/internal_message.rs @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: BUSL-1.1 + +//! Client-facing message hygiene for rendered failures. +//! +//! An internal (`XX000`-class) failure describes server state — a manifest +//! path, an invariant, an unmapped bridge fault. That detail belongs in the +//! server log; the client receives a stable summary, because the class and +//! the numeric code already say everything the client can act on. Every other +//! class keeps its message: it names what the caller must change. + +use nodedb_types::error::{ErrorCode, sqlstate}; + +use super::error_map::numeric_code_to_sqlstate; + +/// The client-facing text for a failure rendered at a protocol edge. +/// +/// Codes the mapper sends to `XX000` (`INTERNAL`, `BRIDGE`, `DISPATCH`) +/// become one generic summary and the detail is logged with its code. Any +/// other class passes through unchanged. +pub fn shaping_error_message(code: ErrorCode, message: impl Into) -> String { + let message = message.into(); + if numeric_code_to_sqlstate(code) == sqlstate::INTERNAL_ERROR { + tracing::error!(%code, detail = %message, "internal failure rendered to a client"); + "internal error while shaping the response".to_owned() + } else { + message + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn internal_class_detail_is_replaced() { + let message = shaping_error_message( + ErrorCode::INTERNAL, + "manifest at /var/lib/nodedb/segment-42 is corrupt", + ); + + assert_eq!(message, "internal error while shaping the response"); + assert!(!message.contains("segment-42")); + } + + #[test] + fn client_class_message_passes_through() { + let message = shaping_error_message( + ErrorCode::SERIALIZATION, + "cell \"ts\" holds an integer where a timestamp is required", + ); + + assert!(message.contains("timestamp")); + } +} diff --git a/nodedb/src/control/server/pgwire/types/mod.rs b/nodedb/src/control/server/pgwire/types/mod.rs index b15b76920..47a892ba0 100644 --- a/nodedb/src/control/server/pgwire/types/mod.rs +++ b/nodedb/src/control/server/pgwire/types/mod.rs @@ -7,6 +7,7 @@ pub mod error_map; pub mod field; +pub mod internal_message; pub mod parse; pub mod privilege; @@ -19,6 +20,7 @@ pub use field::{ int2_field, int4_field, int8_field, json_field, jsonb_field, text_field, timestamp_field, timestamptz_field, type_name_to_pgwire, varchar_field, }; +pub use internal_message::shaping_error_message; pub use parse::parse_role; pub use privilege::{ require_cluster_admin, require_database_owner, require_database_owner_or_higher, diff --git a/nodedb/src/error_classify.rs b/nodedb/src/error_classify.rs index afc808472..f756dba90 100644 --- a/nodedb/src/error_classify.rs +++ b/nodedb/src/error_classify.rs @@ -177,8 +177,11 @@ pub(crate) fn classify(e: &Error) -> NodeDbError { Error::InvalidLimitValue { clause, value } => { NodeDbError::invalid_limit_value(*clause, value.clone()) } + // Retryable, like a write conflict: the descriptor-lease paths already + // retry this variant, so it must not reach a client as a plan/syntax + // error, which reads as "fix your SQL". Error::RetryableSchemaChanged { descriptor } => { - NodeDbError::plan_error(format!("retryable schema change on {descriptor}")) + NodeDbError::write_conflict("catalog", descriptor.clone()) } Error::RetryableLeaderChange { group_id,