Native client discards the server's SQLSTATE: every server error becomes ErrorCode(9000) Internal, and retriability is lost

[laksamanakeris]

Version / build tested against

origin/main @ 2828760

Deployment mode

Origin — single node (local)

Engine(s) involved

Not engine-specific / unsure

Summary

nodedb-client's native protocol connection reads only the message field of a server error frame and drops the code field beside it, so every server-returned error reaches the caller as ErrorCode(9000) / ErrorDetails::Internal regardless of what the server actually classified it as. The SQLSTATE is computed by the server, is carried on the wire, and is documented on the frame type; it is discarded on arrival.

The consequence is not only a less useful error string. Every predicate on NodeDbError derives from ErrorDetails, so folding everything into Internal also makes is_retriable(), is_not_found(), is_client_error() and friends report the wrong answer. A cross-shard OCC abort is sent as 40001 (serialization_failure) precisely so the client retries it, and the client reports it as a non-retriable internal fault, so a retry loop abandons a transaction that was designed to be retried.

Client-side errors are unaffected, which localises the fault: connecting to a dead port still yields SyncConnectionFailed / ErrorCode(3000) / retriable = true. Only errors that arrive in a server frame are flattened.

Steps to reproduce

-- Server: origin/main @ 28287607a, single node on loopback, fresh data dir,
-- config with [auth] mode = "trust". Driven through nodedb-client:
--     ConnectionBuilder::new(addr).username("admin").build()?
-- printing e.code(), e.is_retriable() and e.details() for each statement.

CREATE TABLE repro_t (id BIGINT PRIMARY KEY, v BIGINT);   -- OK
INSERT INTO repro_t (id, v) VALUES (1, 1);                -- OK

SELECT * FROM does_not_exist;
-- server sends SQLSTATE 42P01 "collection 'does_not_exist' not found"
-- client reports  ErrorCode(9000)  Internal { component: "unspecified", ... }
--                 <- WRONG, expected ErrorCode(1100) / CollectionNotFound

DROP TABLE does_not_exist;
-- server sends SQLSTATE 42P01 "collection 'does_not_exist' does not exist"
-- client reports  ErrorCode(9000)  Internal { ... }        <- WRONG

CREATE TABLE repro_t (id BIGINT PRIMARY KEY);
-- server sends SQLSTATE 42P07 "table 'repro_t' already exists"
-- client reports  ErrorCode(9000)  Internal { ... }        <- WRONG

The raw wire values above were read by logging ErrorPayload.code at the point the client currently discards it, so they are the bytes the server actually sent rather than an inference from the pgwire port.

Expected behavior

A server error frame carries the server's own classification of the failure, so the client should reconstruct a typed NodeDbError from it: 42P01 should surface as ErrorCode::COLLECTION_NOT_FOUND with ErrorDetails::CollectionNotFound, is_not_found() true, and the server's message preserved. A SQLSTATE with no corresponding ErrorDetails variant should keep behaving exactly as it does today, as an internal error.

Actual behavior

Every server-returned error, whatever its SQLSTATE, arrives as:

code      = ErrorCode(9000)
details   = Internal { component: "unspecified", detail: "<the real message>" }
retriable = false

Root cause

The server computes a SQLSTATE (nodedb/src/control/server/native/dispatch/conversion.rs, error_to_native, plus ddl_result_to_native which forwards DdlError.sqlstate verbatim), and the wire type carries it with a doc comment naming this exact example:

// nodedb-types/src/protocol/frames.rs
pub struct ErrorPayload {
    /// SQLSTATE-style error code (e.g., "42P01" for undefined table).
    pub code: String,
    /// Human-readable error message.
    pub message: String,
}

Two functions in nodedb-client/src/native/connection/mod.rs, check_error and response_to_query_result, each do:

let msg = resp.error.map(|e| e.message)      // e.code discarded here
    .unwrap_or_else(|| "unknown error".into());
return Err(NodeDbError::internal(msg));      // everything becomes Internal

.map(|e| e.message) keeps the message and drops e.code. Nothing anywhere in the tree maps a SQLSTATE back to an ErrorDetails, so even a caller willing to do the work has no vocabulary to do it with, despite nodedb-types/src/error/sqlstate.rs already defining the constants for both directions.

Worth stating plainly, because it is easy to read this as intentional: a redaction measure would do the opposite of this. It would suppress the internal message and keep the coarse code. This keeps the full internal message, including raw parser output, and drops the harmless five-character classifier.

How it survived

The test covering this branch constructs NativeResponse::error(1, "42P01", "not found") and asserts only that the message survives. Nothing asserts the code, so the missing mapping never failed a test.

Provenance

Stated only because it explains the gap, not to attribute it:

• Introduced in 22d2c87ca (2026-03-24), feat(client): add native protocol client with connection pooling, a single-parent commit on main. The repo's first PR (fix(timeseries): correct ILP shard routing and scan column output #1) was opened 2026-03-29, five days later, so this code was never reviewed.
• The server's SQLSTATE mapping predates it: 77d964688 (2026-03-17), a week earlier. ErrorPayload.code already carried its 42P01 doc comment when the client was written.
• The only later commit touching these lines is 25f6cea14, a file-splitting refactor that moved them verbatim.

This looks like an oversight in an unreviewed initial implementation rather than a deliberate trade-off.

Two adjacent findings, out of scope for this issue

Both were measured while reproducing the above. Filing separately if maintainers agree they are worth tracking.

1. error_to_native types fewer errors than the pgwire path does. Its match has arms for seven crate::Error variants and sends XX000 for everything else, including PlanError and UndefinedFunction, whose own doc comments say the pgwire layer renders them as 42601 and 42883. Measured on the same statements: SELEKT 1 and SELECT no_such_fn(1) come back as 42601 and 42883 over pgwire but as XX000 over the native protocol. So the native wire carries strictly less type information than pgwire for the same query, and fixing the client alone will not type those two classes.

2. 42P07 has no ErrorDetails equivalent. The server correctly sends 42P07 for a duplicate CREATE TABLE, but there is no "already exists" variant to map it onto, so it will keep falling back to Internal after the client fix.

Note also that a duplicate primary key is not an instance of this bug: the server itself sends XX000 for it, because RejectedConstraint reaches error_to_native's catch-all arm. The client folding that to Internal is correct.

What actually happened? (check all that are true)

• A workaround exists (rewrite the query, avoid one path, etc.)

Proposed severity

SEV-3 — Medium: feature wrong, but operational and a workaround exists

Reproducibility

Always — every attempt

Last known-good version / commit (if a regression)

Environment & logs

macOS arm64 (Darwin 25.5.0), debug build of origin/main @ 28287607a.

Two things that otherwise cost time when reproducing:

• Trust auth is off by default, so a default-config server rejects the client's trust handshake. An [auth] section with mode = "trust" is needed, and AuthConfig has several fields with no serde defaults (mode, superuser_name, min_password_length, max_failed_logins, lockout_duration_secs, idle_timeout_secs, max_connections_per_user, password_expiry_days, audit_retention_days); the server reports them one at a time.
• Debug builds overflow the thread stack on some statements. RUST_MIN_STACK=67108864 avoids it. This is a debug-build artifact, not a server defect.

Before submitting

• I searched existing issues and this is not a duplicate.
• I reproduced this on a released tag or a current main build (not a stale local branch).
• This is not a security vulnerability (those go to a private advisory).