From 42c7a7abe855b86a6077a216678dbe4a88482841 Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Fri, 18 Sep 2026 23:28:11 +0800 Subject: [PATCH 1/3] fix(pgwire): complete the numeric mapper and keep internal detail in the log The mapper answered 22 of 81 codes and fell back to XX000 for the rest, so a quota, clone, move-tenant, mirror, sync, storage/WAL, cluster, config, encryption, or shaper-raised code reached every routed surface as an internal fault. The remaining 59 codes now carry the decided class, with six new sqlstate.rs constants (22000, 22P02, 25006, 2BP01, 42P07, XX001); INTERNAL, BRIDGE and DISPATCH keep XX000 as explicit arms. shape_error_to_pg now renders its message through shaping_error_message: an XX000-class failure logs the detail and sends one stable summary, so a manifest path or invariant text cannot travel to a client on an error frame. Tests: completion table (59 codes), internal arms, hygiene pass-through/replace. --- nodedb-types/src/error/sqlstate.rs | 34 ++++ .../control/server/pgwire/types/error_map.rs | 191 +++++++++++++++++- .../server/pgwire/types/internal_message.rs | 54 +++++ nodedb/src/control/server/pgwire/types/mod.rs | 3 + 4 files changed, 281 insertions(+), 1 deletion(-) create mode 100644 nodedb/src/control/server/pgwire/types/internal_message.rs 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..f12e85705 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. @@ -273,6 +280,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 +379,115 @@ 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")); + } +} 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..42b3597bd 100644 --- a/nodedb/src/control/server/pgwire/types/mod.rs +++ b/nodedb/src/control/server/pgwire/types/mod.rs @@ -7,9 +7,11 @@ pub mod error_map; pub mod field; +pub mod internal_message; pub mod parse; pub mod privilege; +pub(crate) use error_map::numeric_code_to_sqlstate; pub use error_map::{ dml_fold_error_to_pg, error_to_pg, error_to_sqlstate, notice_warning, response_status_to_sqlstate, shape_error_to_pg, sqlstate_error, @@ -19,6 +21,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, From 70ab06047c43671e2d65403369647c69f35e42de Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Sun, 20 Sep 2026 22:44:27 +0800 Subject: [PATCH 2/3] fix(pgwire): drop the unused mapper re-export types/mod.rs re-exported numeric_code_to_sqlstate, but the routed gateway calls it by its full path and internal_message uses the module directly, so the re-export was dead and Lint & Check failed on the unused import. --- nodedb/src/control/server/pgwire/types/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/nodedb/src/control/server/pgwire/types/mod.rs b/nodedb/src/control/server/pgwire/types/mod.rs index 42b3597bd..47a892ba0 100644 --- a/nodedb/src/control/server/pgwire/types/mod.rs +++ b/nodedb/src/control/server/pgwire/types/mod.rs @@ -11,7 +11,6 @@ pub mod internal_message; pub mod parse; pub mod privilege; -pub(crate) use error_map::numeric_code_to_sqlstate; pub use error_map::{ dml_fold_error_to_pg, error_to_pg, error_to_sqlstate, notice_warning, response_status_to_sqlstate, shape_error_to_pg, sqlstate_error, From 2f8f4f17c02de42fef0628d5f8148c19f9b7e2ee Mon Sep 17 00:00:00 2001 From: EnRaiha <15997552+EnRaiha@users.noreply.github.com> Date: Sun, 20 Sep 2026 22:37:18 +0800 Subject: [PATCH 3/3] fix(pgwire): answer a real class for the arm-less error variants The blanket INTERNAL_ERROR fallback in error_to_sqlstate flattened 65 of the 101 crate::Error variants, so a statement reading a balance, hitting a period lock, a legal hold, a retention floor, a type guard, or a mirror write got XX000 while the crate's classify() map already carried the class. The fallback now borrows it through classify() and answers from the same numeric table the remote path reads. The bridge exposed one arm doing harm: RetryableSchemaChanged classified as a plan error, which reads as 'fix your SQL' for a condition the lease paths retry. It now classifies as a write conflict (40001), matching its contract. --- .../control/server/pgwire/types/error_map.rs | 110 +++++++++++++++++- nodedb/src/error_classify.rs | 5 +- 2 files changed, 113 insertions(+), 2 deletions(-) diff --git a/nodedb/src/control/server/pgwire/types/error_map.rs b/nodedb/src/control/server/pgwire/types/error_map.rs index f12e85705..5a8986e76 100644 --- a/nodedb/src/control/server/pgwire/types/error_map.rs +++ b/nodedb/src/control/server/pgwire/types/error_map.rs @@ -225,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(), + ), } } @@ -490,4 +497,105 @@ mod tests { 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/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,