diff --git a/nodedb-client/src/native/connection/mod.rs b/nodedb-client/src/native/connection/mod.rs index 05cf17858..34e8ecad8 100644 --- a/nodedb-client/src/native/connection/mod.rs +++ b/nodedb-client/src/native/connection/mod.rs @@ -13,9 +13,9 @@ use std::sync::atomic::{AtomicU64, Ordering}; use nodedb_types::error::{NodeDbError, NodeDbResult}; use nodedb_types::protocol::{ AuthMethod, CAP_COLUMNAR, CAP_CRDT, CAP_FTS, CAP_GRAPHRAG, CAP_SPATIAL, CAP_STREAMING, - CAP_TIMESERIES, FRAME_HEADER_LEN, HelloAckFrame, HelloFrame, Limits, MAX_FRAME_SIZE, - NativeRequest, NativeResponse, OpCode, PROTO_VERSION, RequestFields, ResponseStatus, - TextFields, + CAP_TIMESERIES, ErrorPayload, FRAME_HEADER_LEN, HelloAckFrame, HelloFrame, Limits, + MAX_FRAME_SIZE, NativeRequest, NativeResponse, OpCode, PROTO_VERSION, RequestFields, + ResponseStatus, TextFields, }; use nodedb_types::result::QueryResult; use tokio::io::{AsyncReadExt, AsyncWriteExt}; @@ -449,24 +449,30 @@ fn io_err(e: std::io::Error) -> NodeDbError { NodeDbError::sync_connection_failed(format!("I/O: {e}")) } +/// Rebuild the typed error a server error frame describes. +/// +/// The frame carries the server's own SQLSTATE alongside the message; reading +/// only the message would discard the one machine-matchable fact in it and +/// leave every server-side failure indistinguishable from an internal fault. +/// A frame with no payload at all has no classification to recover, so it +/// falls back to `fallback` as an internal error. +fn error_from_frame(error: Option, fallback: &str) -> NodeDbError { + error.map_or_else( + || NodeDbError::internal(fallback), + |e| NodeDbError::from_sqlstate(&e.code, &e.message), + ) +} + fn check_error(resp: NativeResponse) -> NodeDbResult<()> { if resp.status == ResponseStatus::Error { - let msg = resp - .error - .map(|e| e.message) - .unwrap_or_else(|| "unknown error".into()); - return Err(NodeDbError::internal(msg)); + return Err(error_from_frame(resp.error, "unknown error")); } Ok(()) } fn response_to_query_result(resp: NativeResponse) -> NodeDbResult { if resp.status == ResponseStatus::Error { - let msg = resp - .error - .map(|e| e.message) - .unwrap_or_else(|| "query failed".into()); - return Err(NodeDbError::internal(msg)); + return Err(error_from_frame(resp.error, "query failed")); } Ok(QueryResult { columns: resp.columns.unwrap_or_default(), @@ -498,11 +504,67 @@ mod tests { assert_eq!(qr.rows[0][0].as_i64(), Some(42)); } + /// An error frame must surface the server's classification, not just its + /// prose: asserting only on the message is what let the SQLSTATE be + /// dropped here unnoticed. #[test] fn response_to_query_result_error() { let resp = NativeResponse::error(1, "42P01", "not found"); let err = response_to_query_result(resp).unwrap_err(); + assert!(format!("{err}").contains("not found")); + assert_eq!( + err.code(), + nodedb_types::error::ErrorCode::COLLECTION_NOT_FOUND + ); + assert!(err.is_not_found()); + assert!(!err.is_internal()); + } + + /// The same reconstruction happens on the non-query path, which has its + /// own copy of the error branch. + #[test] + fn check_error_reconstructs_sqlstate() { + let resp = NativeResponse::error(1, "42501", "authorization denied on t"); + let err = check_error(resp).unwrap_err(); + + assert!(err.is_auth_denied()); + assert_eq!(err.message(), "authorization denied on t"); + } + + /// Retriability is the behavioural consequence of the classification: a + /// serialization failure the client folds to `Internal` reads as + /// permanent, so a retrying caller gives up on a transaction that would + /// have succeeded. + #[test] + fn serialization_failure_frame_is_retriable() { + let resp = NativeResponse::error(1, "40001", "transaction aborted"); + let err = response_to_query_result(resp).unwrap_err(); + + assert!(err.is_retriable()); + } + + /// Non-regression: a SQLSTATE with no mapped variant behaves exactly as + /// every server error did before the mapping existed. + #[test] + fn unmapped_sqlstate_frame_stays_internal() { + let resp = NativeResponse::error(1, "XX000", "boom"); + let err = response_to_query_result(resp).unwrap_err(); + + assert!(err.is_internal()); + assert!(format!("{err}").contains("boom")); + } + + /// An error frame with no payload has no SQLSTATE to recover, so it keeps + /// the call site's own fallback text. + #[test] + fn error_frame_without_payload_uses_fallback() { + let mut resp = NativeResponse::error(1, "42P01", "not found"); + resp.error = None; + let err = response_to_query_result(resp).unwrap_err(); + + assert!(err.is_internal()); + assert!(format!("{err}").contains("query failed")); } #[test] diff --git a/nodedb-types/src/error/ctors/from_sqlstate.rs b/nodedb-types/src/error/ctors/from_sqlstate.rs new file mode 100644 index 000000000..aa851fb1c --- /dev/null +++ b/nodedb-types/src/error/ctors/from_sqlstate.rs @@ -0,0 +1,319 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Reconstruct a typed [`NodeDbError`] from a SQLSTATE received over the wire. +//! +//! The native protocol's `ErrorPayload` carries a five-character SQLSTATE and a +//! human-readable message. The SQLSTATE is the server's own classification of +//! the failure, so a client that reads only the message throws away every +//! machine-matchable fact about the error and leaves callers with nothing to +//! branch on. +//! +//! # Fidelity limits +//! +//! A SQLSTATE classifies; it does not carry operands. Variants with payload +//! fields (`CollectionNotFound { collection }`, `UndefinedFunction { name }`) +//! are therefore rebuilt with those fields empty: the failing name lives in the +//! message, which is preserved verbatim, but it is not recoverable as +//! structured data. The variant, the [`ErrorCode`], and every predicate derived +//! from them ([`NodeDbError::is_retriable`], [`NodeDbError::is_not_found`], …) +//! are exact. An empty field reads as "not transmitted"; guessing the operand +//! out of the message text would be worse than leaving it blank. +//! +//! Codes are mapped only where a SQLSTATE and an [`ErrorDetails`] variant +//! correspond one-to-one. Three groups are deliberately left unmapped: +//! +//! - **Overloaded codes.** `55P03` is both `LOCK_NOT_AVAILABLE` (no leader, +//! retriable) and `STALE_READ_NOT_LEADER` (redirect to source, not +//! retriable); `57P03` is both a draining collection and server overload; +//! `53400` is both quota-exceeded and quota-overcommit. Picking either side +//! would misreport retriability for the other. +//! - **Load-bearing payloads.** `57P04` maps to `NotLeader { leader_addr }` and +//! `54001` to `FanOutExceeded { shards_touched, limit }`. Unlike a name, a +//! redirect address or a shard count is acted on rather than displayed, and +//! an empty or zero value would be a fabricated instruction. +//! - **Codes with no variant.** `42P07` (duplicate table) and `0A000` +//! (feature not supported) have no `ErrorDetails` equivalent today. +//! +//! Everything unmapped falls through to [`NodeDbError::internal`], which is +//! exactly what every server error produced before this mapping existed. + +use super::super::code::ErrorCode; +use super::super::details::ErrorDetails; +use super::super::sqlstate; +use super::super::types::NodeDbError; + +impl NodeDbError { + /// Rebuild a typed error from a server-sent SQLSTATE and message. + /// + /// `message` is preserved verbatim on mapped codes. Unmapped codes produce + /// the same [`NodeDbError::internal`] this function replaced, so a server + /// emitting a SQLSTATE no client knows yet is no worse off than before. + pub fn from_sqlstate(sqlstate: &str, message: &str) -> Self { + let Some((code, details)) = classify(sqlstate) else { + return Self::internal(message); + }; + Self { + code, + message: message.to_owned(), + details, + cause: None, + } + } +} + +/// Map a SQLSTATE onto the code/details pair that classifies it, or `None` +/// when no variant corresponds. +fn classify(sqlstate: &str) -> Option<(ErrorCode, ErrorDetails)> { + let empty = String::new; + let pair = match sqlstate { + // ── Data exception ── + sqlstate::NUMERIC_VALUE_OUT_OF_RANGE => ( + ErrorCode::OVERFLOW, + ErrorDetails::Overflow { + collection: empty(), + }, + ), + sqlstate::DIVISION_BY_ZERO => (ErrorCode::DIVISION_BY_ZERO, ErrorDetails::DivisionByZero), + + // ── Integrity constraint violation ── + // + // The generic class-23 codes all describe the same client-visible + // condition: a write the collection's constraints refused. + sqlstate::INTEGRITY_CONSTRAINT_VIOLATION + | sqlstate::NOT_NULL_VIOLATION + | sqlstate::FOREIGN_KEY_VIOLATION + | sqlstate::UNIQUE_VIOLATION + | sqlstate::CHECK_VIOLATION => ( + ErrorCode::CONSTRAINT_VIOLATION, + ErrorDetails::ConstraintViolation { + collection: empty(), + }, + ), + // NodeDB's class-23 extensions each have a dedicated variant. + sqlstate::APPEND_ONLY_VIOLATION => ( + ErrorCode::APPEND_ONLY_VIOLATION, + ErrorDetails::AppendOnlyViolation { + collection: empty(), + }, + ), + sqlstate::BALANCE_VIOLATION => ( + ErrorCode::BALANCE_VIOLATION, + ErrorDetails::BalanceViolation { + collection: empty(), + }, + ), + sqlstate::PERIOD_LOCKED => ( + ErrorCode::PERIOD_LOCKED, + ErrorDetails::PeriodLocked { + collection: empty(), + }, + ), + sqlstate::STATE_TRANSITION_VIOLATION => ( + ErrorCode::STATE_TRANSITION_VIOLATION, + ErrorDetails::StateTransitionViolation { + collection: empty(), + }, + ), + sqlstate::TRANSITION_CHECK_VIOLATION => ( + ErrorCode::TRANSITION_CHECK_VIOLATION, + ErrorDetails::TransitionCheckViolation { + collection: empty(), + }, + ), + sqlstate::RETENTION_VIOLATION => ( + ErrorCode::RETENTION_VIOLATION, + ErrorDetails::RetentionViolation { + collection: empty(), + }, + ), + sqlstate::LEGAL_HOLD_ACTIVE => ( + ErrorCode::LEGAL_HOLD_ACTIVE, + ErrorDetails::LegalHoldActive { + collection: empty(), + }, + ), + sqlstate::TYPE_GUARD_VIOLATION => ( + ErrorCode::TYPE_GUARD_VIOLATION, + ErrorDetails::TypeGuardViolation { + collection: empty(), + }, + ), + + // ── Authorization ── + // + // The server sends 28000 both for an expired bearer token and for an + // unauthenticated request; `AuthExpired` covers both as a client error + // whose remedy is to authenticate again. + sqlstate::INVALID_AUTHORIZATION => (ErrorCode::AUTH_EXPIRED, ErrorDetails::AuthExpired), + sqlstate::INSUFFICIENT_PRIVILEGE => ( + ErrorCode::AUTHORIZATION_DENIED, + ErrorDetails::AuthorizationDenied { resource: empty() }, + ), + + // ── Transaction rollback ── + // + // Restores retriability: `WriteConflict` is in `is_retriable`, so a + // serialization failure once again tells the caller to retry. + sqlstate::SERIALIZATION_FAILURE => ( + ErrorCode::WRITE_CONFLICT, + ErrorDetails::WriteConflict { + collection: empty(), + document_id: empty(), + }, + ), + + // ── Syntax error or access rule violation ── + sqlstate::SYNTAX_ERROR => (ErrorCode::BAD_REQUEST, ErrorDetails::BadRequest), + sqlstate::CANNOT_COERCE => ( + ErrorCode::TYPE_MISMATCH, + ErrorDetails::TypeMismatch { + collection: empty(), + }, + ), + sqlstate::UNDEFINED_TABLE => ( + ErrorCode::COLLECTION_NOT_FOUND, + ErrorDetails::CollectionNotFound { + collection: empty(), + }, + ), + sqlstate::UNDEFINED_FUNCTION => ( + ErrorCode::UNDEFINED_FUNCTION, + ErrorDetails::UndefinedFunction { name: empty() }, + ), + + // ── Insufficient resources ── + sqlstate::OUT_OF_MEMORY => ( + ErrorCode::MEMORY_EXHAUSTED, + ErrorDetails::MemoryExhausted { engine: empty() }, + ), + sqlstate::TOO_MANY_CONNECTIONS => ( + ErrorCode::RATE_EXCEEDED, + ErrorDetails::RateExceeded { gate: empty() }, + ), + + // ── Operator intervention ── + sqlstate::QUERY_CANCELED => (ErrorCode::DEADLINE_EXCEEDED, ErrorDetails::DeadlineExceeded), + + _ => return None, + }; + Some(pair) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The headline case: the SQLSTATE the server computes for a missing + /// collection must survive the trip, not collapse into `Internal`. + #[test] + fn undefined_table_maps_to_collection_not_found() { + let e = NodeDbError::from_sqlstate("42P01", "collection 'users' not found"); + + assert_eq!(e.code(), ErrorCode::COLLECTION_NOT_FOUND); + assert!(matches!( + e.details(), + ErrorDetails::CollectionNotFound { .. } + )); + assert!(e.is_not_found()); + assert!(!e.is_internal()); + } + + /// The message is the only place the failing operand survives, so it is + /// carried through untouched rather than re-derived from the variant. + #[test] + fn message_is_preserved_verbatim_on_a_mapped_code() { + let e = NodeDbError::from_sqlstate("42P01", "collection 'users' not found"); + + assert_eq!(e.message(), "collection 'users' not found"); + } + + /// The non-regression guarantee: a SQLSTATE this mapping does not know + /// behaves exactly as every server error did before it existed. + #[test] + fn unmapped_code_falls_back_to_internal() { + let mapped = NodeDbError::from_sqlstate("XX000", "boom"); + let baseline = NodeDbError::internal("boom"); + + assert_eq!(mapped.code(), ErrorCode::INTERNAL); + assert!(mapped.is_internal()); + assert_eq!(mapped.message(), baseline.message()); + assert!(matches!( + mapped.details(), + ErrorDetails::Internal { component, detail } + if component == "unspecified" && detail == "boom" + )); + } + + /// A code that is real but deliberately unmapped (overloaded across two + /// variants with different retriability) must take the same fallback. + #[test] + fn deliberately_unmapped_code_falls_back_to_internal() { + assert!(NodeDbError::from_sqlstate("55P03", "no leader").is_internal()); + assert!(NodeDbError::from_sqlstate("57P04", "not leader").is_internal()); + } + + /// Retriability is derived from `ErrorDetails`, so mapping the variant is + /// what restores it: a serialization failure is retriable again. + #[test] + fn serialization_failure_is_retriable_again() { + let e = NodeDbError::from_sqlstate("40001", "transaction aborted"); + + assert_eq!(e.code(), ErrorCode::WRITE_CONFLICT); + assert!(e.is_retriable()); + } + + /// A deadline is the other genuinely retriable server-side condition. + #[test] + fn query_canceled_is_retriable() { + assert!( + NodeDbError::from_sqlstate("57014", "query cancelled due to timeout").is_retriable() + ); + } + + /// A client error must not be reported as retriable, or the caller would + /// spin on a request that can never succeed. + #[test] + fn client_errors_are_not_retriable() { + for code in ["42P01", "42601", "42883", "42501"] { + let e = NodeDbError::from_sqlstate(code, "nope"); + assert!(!e.is_retriable(), "{code} must not be retriable"); + assert!(e.is_client_error(), "{code} must be a client error"); + } + } + + /// Payload fields are left empty rather than filled from the message: the + /// variant classifies, the message carries the operand. + #[test] + fn payload_fields_are_empty_not_guessed() { + let e = NodeDbError::from_sqlstate("42883", "function no_such_fn(...) does not exist"); + + assert!(matches!( + e.details(), + ErrorDetails::UndefinedFunction { name } if name.is_empty() + )); + assert_eq!(e.message(), "function no_such_fn(...) does not exist"); + } + + /// Every SQLSTATE the mapping claims resolves to a distinct, matching code. + #[test] + fn spot_check_mapped_codes() { + let cases = [ + ("22012", ErrorCode::DIVISION_BY_ZERO), + ("23505", ErrorCode::CONSTRAINT_VIOLATION), + ("28000", ErrorCode::AUTH_EXPIRED), + ("42501", ErrorCode::AUTHORIZATION_DENIED), + ("42601", ErrorCode::BAD_REQUEST), + ("42883", ErrorCode::UNDEFINED_FUNCTION), + ("53200", ErrorCode::MEMORY_EXHAUSTED), + ("53300", ErrorCode::RATE_EXCEEDED), + ("57014", ErrorCode::DEADLINE_EXCEEDED), + ]; + for (sqlstate, expected) in cases { + assert_eq!( + NodeDbError::from_sqlstate(sqlstate, "m").code(), + expected, + "{sqlstate} mapped to the wrong code" + ); + } + } +} diff --git a/nodedb-types/src/error/ctors/mod.rs b/nodedb-types/src/error/ctors/mod.rs index 77076a2b1..b18d69732 100644 --- a/nodedb-types/src/error/ctors/mod.rs +++ b/nodedb-types/src/error/ctors/mod.rs @@ -8,7 +8,9 @@ //! - [`read_query_auth`] — 1100 read path, 1200 query, 2000 auth. //! - [`sync_infra`] — 3000 sync, 4000 storage, 4200 serialization, 5000 config, //! 6000 cluster, 7000 memory, 8000 encryption, 9000 internal/bridge/dispatch. +//! - [`from_sqlstate`] — rebuild a typed error from a SQLSTATE sent by a peer. +pub mod from_sqlstate; pub mod mirror; pub mod move_tenant; pub mod read_query_auth;