diff --git a/MIGRATING-0.3.md b/MIGRATING-0.3.md index a054eb8..188eab4 100644 --- a/MIGRATING-0.3.md +++ b/MIGRATING-0.3.md @@ -2,7 +2,7 @@ This is the consolidated migration guide for the v0.3.0 bundle of breaking and additive changes. Each section corresponds to a bundle PR; the guide grows as each PR lands. The bundle ships as one major bump after the last PR merges. -> Each bundle PR uses `chore:` Conventional Commit prefix to defer release-please from cutting an early version. After all PRs merge, a single `feat!:` commit with a `BREAKING CHANGE:` footer triggers v0.3.0. +> Each bundle PR uses `chore:` Conventional Commit prefix to defer release-please from cutting an early version. After all PRs merge, a single `feat:` commit with a `BREAKING CHANGE:` footer triggers v0.3.0. --- @@ -26,7 +26,7 @@ The public `hyperdb_api::Error` type was redesigned into a flat enum per the [Mi ```rust pub enum Error { // Connection / transport - Connection { message: String, source: Option }, + Connection { message: String, source: Option, sqlstate: Option }, Authentication(String), Tls(String), @@ -38,9 +38,9 @@ pub enum Error { Io(std::io::Error), // Lifecycle - Closed(String), + Closed { message: String, sqlstate: Option }, Timeout(String), - Cancelled(String), + Cancelled { message: String, sqlstate: Option }, // Type / value Conversion(String), @@ -52,6 +52,7 @@ pub enum Error { InvalidTableDefinition(String), NotFound(String), AlreadyExists(String), + InvalidOperation(String), // added in Follow-up B // Column / row mapping Column { name: String, kind: ColumnErrorKind }, @@ -97,15 +98,18 @@ Error::invalid_name("..."); Error::invalid_table_definition("..."); Error::not_found("..."); Error::already_exists("..."); +Error::invalid_operation("..."); ``` Pattern-matching uses the PascalCase variant names (e.g. `Error::Conversion(msg)`); only construction switches to snake_case. Forward-compatibility for new struct-variant fields relies on going through these constructors — `#[non_exhaustive]` on individual struct variants is forbidden by Rust E0639. ### Behavioral note: SQLSTATE on non-server errors -`Error::sqlstate()` now returns `Some(...)` only for [`Error::Server`]. Previously it could return `Some` for any wrapped `client::Error` whose internal type carried a SQLSTATE code, including some `Cancelled`, `Closed`, and `Connection` paths that arrived from the server with codes like `57014` (`query_canceled`), `57P03` (`cannot_connect_now`), or `08*` connection-class codes. +> **Updated by Follow-up C below.** v0.3.0 ships with structured SQLSTATE on `Server`, `Connection`, `Closed`, and `Cancelled`. Use `Error::sqlstate()` on any of these variants to retrieve the code; or destructure the variant directly to read it. -After v0.3.0 those SQLSTATE codes are folded into the variant's message string (still visible to humans via `Display`) but are not surfaced by `Error::sqlstate()`. If you branch on those codes, parse them out of the message string or open a follow-up issue requesting structured SQLSTATE on `Connection`/`Closed`/`Cancelled`/`Timeout` variants. +`Error::sqlstate()` returns `Some(...)` for [`Error::Server`] (Query-class codes), [`Error::Connection`] (typically `08*`), [`Error::Closed`] (typically `57P0*` shutdown codes), and [`Error::Cancelled`] (typically `57014` `query_canceled`) when the underlying server provided a code. Other variants always return `None`. + +If you have callers that previously parsed SQLSTATE out of the message string for `Cancelled` / `Closed` / `Connection`, switch them to destructuring or `Error::sqlstate()` — see the Follow-up C section below for recipes. ### Migration recipes @@ -373,6 +377,118 @@ The proc-macro lives in a new `hyperdb-api-derive` workspace crate (Rust require --- +## Follow-up C — Structured SQLSTATE on `Cancelled` / `Closed` / `Connection` + +Reverses the v0.3.0 "non-Server SQLSTATE drops to message" caveat. SQLSTATE codes that arrive via cancellation (`57014`), connection-class (`08*`), or close-class wire errors (`57P01` admin shutdown, `57P02` crash shutdown) are now exposed structurally via the variant's `sqlstate` field, and `Error::sqlstate()` returns them too — no more parsing the message string. + +### Variant shape changes + +```rust +// Before +Error::Cancelled(String) +Error::Closed(String) +Error::Connection { message: String, source: Option } + +// After +Error::Cancelled { message: String, sqlstate: Option } +Error::Closed { message: String, sqlstate: Option } +Error::Connection { message: String, source: Option, sqlstate: Option } +``` + +`Error::Cancelled` and `Error::Closed` are now struct variants instead of tuple variants. Match arms that destructured them as tuples must switch to struct-pattern syntax. + +### Migration recipes + +**Pattern-match for the message** — before: + +```rust +match err { + Error::Cancelled(msg) => log::warn!("cancelled: {msg}"), + Error::Closed(msg) => log::warn!("closed: {msg}"), + other => return Err(other), +} +``` + +after: + +```rust +match err { + Error::Cancelled { message, sqlstate } => log::warn!("cancelled: {message} ({sqlstate:?})"), + Error::Closed { message, sqlstate } => log::warn!("closed: {message} ({sqlstate:?})"), + other => return Err(other), +} +``` + +If you only need the message, use `..` to elide other fields: `Error::Cancelled { message, .. }`. + +**Read SQLSTATE structurally** — new in Follow-up C: + +```rust +if let Error::Cancelled { sqlstate: Some(code), .. } = &err { + if code == "57014" { /* user cancellation, distinct from timeout */ } +} + +// Or via the helper: +match err.sqlstate() { + Some("08006") => /* connection failure */, + Some("57014") => /* query_canceled */, + Some("57P01") => /* admin shutdown */, + _ => {} +} +``` + +### Constructors + +The existing `Error::cancelled(msg)`, `Error::closed(msg)`, and `Error::connection(msg)` keep working — they default `sqlstate: None`. Three new constructors carry SQLSTATE: + +```rust +Error::cancelled_with_sqlstate(message, sqlstate); // both impl Into +Error::closed_with_sqlstate(message, sqlstate); +Error::connection_with_sqlstate(message, sqlstate); +``` + +`Error::connection_with_io(message, io_err)` is unchanged — it still defaults `sqlstate: None`. If you have an `io::Error` cause *and* a SQLSTATE, construct via the struct-expression form (forward-compatibility caveat: future field additions may break it; prefer adding a constructor if this combination becomes common). + +--- + +## Follow-up B — `Error::InvalidOperation` for caller-API misuse + +A new `Error::InvalidOperation(String)` variant separates caller-API misuse from `Error::Internal`. `Error::Internal` is now reserved for true library invariant violations the caller could not have triggered; misuse of caller-facing methods (mixing two mutually exclusive insertion modes, calling `insert_record_batches()` before `insert_data()`, etc.) returns `Error::InvalidOperation`. + +### Affected sites + +`ArrowInserter` state-machine errors that were previously `Error::Internal` are now `Error::InvalidOperation`: + +- Mixing `insert_data()` / `insert_record_batches()` / `insert_raw()` with `insert_batch()` (and vice versa). +- Calling `insert_record_batches()` before `insert_data()` has sent the schema. +- Calling `insert_data()` after the schema has already been sent. + +### Migration recipe + +Match arms that previously caught `Error::Internal { .. }` for any of these caller-misuse cases must now match `Error::InvalidOperation(_)`: + +```rust +// Before +match err { + Error::Internal { message } if message.starts_with("Cannot mix") => /* user-API misuse */, + Error::Internal { .. } => /* invariant violation */, + other => return Err(other), +} + +// After +match err { + Error::InvalidOperation(_) => /* user-API misuse, caller bug */, + Error::Internal { .. } => /* library invariant violation, hyperdb-api bug */, + other => return Err(other), +} +``` + +### Constructor + +`Error::invalid_operation(message: impl Into)`. `String`-shaped tuple variant, matching the `Error::Conversion` / `Error::Config` / `Error::FeatureNotSupported` pattern — no `.to_string()` ceremony needed. + +--- + ## #70 (continued) — Ergonomic constructors across all workspace error types The same ergonomic-constructor pattern was applied to every error type in the workspace that user code might construct, so call sites no longer need `.to_string()` ceremony for string-literal arguments. diff --git a/hyperdb-api/src/arrow_inserter.rs b/hyperdb-api/src/arrow_inserter.rs index 8c55f43..a1e3cce 100644 --- a/hyperdb-api/src/arrow_inserter.rs +++ b/hyperdb-api/src/arrow_inserter.rs @@ -301,7 +301,7 @@ impl<'conn> ArrowInserter<'conn> { } if self.insert_mode == Some(InsertMode::BatchIpc) { - return Err(Error::internal( + return Err(Error::invalid_operation( "Cannot mix insert_data() with insert_batch(). \ Use either raw IPC methods (insert_data/insert_record_batches) \ or RecordBatch methods (insert_batch), not both.", @@ -309,7 +309,7 @@ impl<'conn> ArrowInserter<'conn> { } if self.schema_sent { - return Err(Error::internal( + return Err(Error::invalid_operation( "Arrow schema was already sent. Use insert_record_batches() for subsequent chunks without schema, \ or use insert_data() only once with the complete Arrow IPC stream.", )); @@ -386,7 +386,7 @@ impl<'conn> ArrowInserter<'conn> { } if self.insert_mode == Some(InsertMode::BatchIpc) { - return Err(Error::internal( + return Err(Error::invalid_operation( "Cannot mix insert_record_batches() with insert_batch(). \ Use either raw IPC methods (insert_data/insert_record_batches) \ or RecordBatch methods (insert_batch), not both.", @@ -394,7 +394,7 @@ impl<'conn> ArrowInserter<'conn> { } if !self.schema_sent { - return Err(Error::internal( + return Err(Error::invalid_operation( "No Arrow schema has been sent yet. Call insert_data() first with a complete \ Arrow IPC stream that includes the schema.", )); @@ -436,9 +436,9 @@ impl<'conn> ArrowInserter<'conn> { /// /// # Errors /// - /// - Returns [`Error::Internal`] if a previous `insert_batch` call already - /// locked the inserter into `RecordBatch` IPC mode — raw IPC and - /// `RecordBatch` paths cannot be mixed. + /// - Returns [`Error::InvalidOperation`] if a previous `insert_batch` + /// call already locked the inserter into `RecordBatch` IPC mode — + /// raw IPC and `RecordBatch` paths cannot be mixed. /// - Returns [`Error::FeatureNotSupported`] / [`Error::Server`] if the lazy COPY /// session fails to open. /// - Returns [`Error::Server`] / [`Error::Io`] if the server rejects @@ -449,7 +449,7 @@ impl<'conn> ArrowInserter<'conn> { } if self.insert_mode == Some(InsertMode::BatchIpc) { - return Err(Error::internal( + return Err(Error::invalid_operation( "Cannot mix insert_raw() with insert_batch(). \ Use either raw IPC methods (insert_data/insert_record_batches/insert_raw) \ or RecordBatch methods (insert_batch), not both.", @@ -611,9 +611,9 @@ impl<'conn> ArrowInserter<'conn> { /// /// # Errors /// - /// - Returns [`Error::Internal`] if a previous raw-IPC call locked this - /// inserter into the other mode — raw IPC and `RecordBatch` paths - /// cannot be mixed. + /// - Returns [`Error::InvalidOperation`] if a previous raw-IPC call + /// locked this inserter into the other mode — raw IPC and + /// `RecordBatch` paths cannot be mixed. /// - Returns [`Error::FeatureNotSupported`] / [`Error::Server`] if the lazy COPY /// session cannot be opened. /// - Returns [`Error::Conversion`] wrapping the underlying Arrow IPC @@ -630,7 +630,7 @@ impl<'conn> ArrowInserter<'conn> { /// `Some`, so the unwrap is unreachable. pub fn insert_batch(&mut self, batch: &arrow::record_batch::RecordBatch) -> Result<()> { if self.insert_mode == Some(InsertMode::RawIpc) { - return Err(Error::internal( + return Err(Error::invalid_operation( "Cannot mix insert_batch() with raw IPC methods. \ Use either RecordBatch methods (insert_batch) \ or raw IPC methods (insert_data/insert_record_batches/insert_raw), not both.", diff --git a/hyperdb-api/src/error.rs b/hyperdb-api/src/error.rs index ed8ae48..ffd9b2b 100644 --- a/hyperdb-api/src/error.rs +++ b/hyperdb-api/src/error.rs @@ -42,16 +42,24 @@ pub enum Error { /// I/O). Carries the underlying [`std::io::Error`] when one is /// available; the type is erased at the wire-protocol boundary in /// `hyperdb-api-core`, so `source` is `None` for errors that - /// originated there. + /// originated there. `sqlstate` is set when the server provided a + /// connection-class SQLSTATE (e.g. `08001`, `08006`, `57P03`). /// - /// Construct via [`Self::connection`] or [`Self::connection_with_io`]. - #[error("connection error: {message}")] + /// Construct via [`Self::connection`], [`Self::connection_with_io`], + /// or [`Self::connection_with_sqlstate`]. + #[error( + "connection error{}: {message}", + sqlstate.as_ref().map(|s| format!(" ({s})")).unwrap_or_default(), + )] Connection { /// Human-readable description. message: String, /// Underlying I/O error, if available. #[source] source: Option, + /// `PostgreSQL` SQLSTATE code, if the server provided one + /// (typically `08*` connection-class codes). + sqlstate: Option, }, /// Authentication failed. @@ -97,17 +105,38 @@ pub enum Error { Io(#[from] std::io::Error), // ---- Lifecycle ----------------------------------------------------- - /// Operation attempted on a closed connection. - #[error("connection closed: {0}")] - Closed(String), + /// Operation attempted on a closed connection. `sqlstate` is set + /// when the server provided one (typically `57P01` admin shutdown + /// or `57P02` crash shutdown). Construct via [`Self::closed`] or + /// [`Self::closed_with_sqlstate`]. + #[error( + "connection closed{}: {message}", + sqlstate.as_ref().map(|s| format!(" ({s})")).unwrap_or_default(), + )] + Closed { + /// Human-readable description. + message: String, + /// `PostgreSQL` SQLSTATE code, if the server provided one. + sqlstate: Option, + }, /// Operation timed out. #[error("operation timed out: {0}")] Timeout(String), - /// Operation was cancelled. - #[error("operation cancelled: {0}")] - Cancelled(String), + /// Operation was cancelled. `sqlstate` is set when the server + /// provided one (typically `57014` `query_canceled`). Construct via + /// [`Self::cancelled`] or [`Self::cancelled_with_sqlstate`]. + #[error( + "operation cancelled{}: {message}", + sqlstate.as_ref().map(|s| format!(" ({s})")).unwrap_or_default(), + )] + Cancelled { + /// Human-readable description. + message: String, + /// `PostgreSQL` SQLSTATE code, if the server provided one. + sqlstate: Option, + }, // ---- Type / value -------------------------------------------------- /// Type or value conversion failed (out-of-range numeric, malformed @@ -144,6 +173,15 @@ pub enum Error { #[error("already exists: {0}")] AlreadyExists(String), + /// Caller-API misuse: a method was called in an invalid sequence + /// or combination (e.g. mixing two mutually exclusive insertion + /// modes on a single inserter, calling a method after the resource + /// has been finalized). Distinct from [`Self::Internal`], which is + /// reserved for true library invariant violations the caller could + /// not have triggered. Construct via [`Self::invalid_operation`]. + #[error("invalid operation: {0}")] + InvalidOperation(String), + // ---- Column / row mapping ------------------------------------------ /// Structured error for named-column access in row decoding. Used /// by `FromRow` impls and `Row::try_get` / `Row::get_by_name` to @@ -169,13 +207,15 @@ pub enum Error { }, // ---- Internal ------------------------------------------------------ - /// Internal invariant violation. Used as a default for state - /// assertions that should be unreachable in correct callers; - /// callers generally cannot recover beyond logging and bailing. + /// Internal invariant violation — a state the library believes + /// should be unreachable. Callers cannot trigger this from the + /// public API in well-formed code; reaching it indicates a bug + /// inside `hyperdb-api`. Recovery is generally impossible beyond + /// logging and bailing. /// - /// Construction of this variant should be rare — every site is a - /// candidate for either a more specific variant or removal once - /// the assertion is proven unreachable. + /// For caller-API misuse (e.g. mixing two mutually exclusive + /// methods, using a finalized resource), prefer + /// [`Self::InvalidOperation`]. /// /// Construct via [`Self::internal`]. #[error("internal error: {message}")] @@ -218,12 +258,14 @@ impl Error { } /// Constructs an [`Self::Connection`] error with no underlying I/O - /// source. Prefer this over struct-expression syntax to remain - /// source-compatible if new fields are added in a minor release. + /// source and no SQLSTATE. Prefer this over struct-expression + /// syntax to remain source-compatible if new fields are added in a + /// minor release. pub fn connection(message: impl Into) -> Self { Error::Connection { message: message.into(), source: None, + sqlstate: None, } } @@ -235,6 +277,20 @@ impl Error { Error::Connection { message: message.into(), source: Some(source), + sqlstate: None, + } + } + + /// Constructs an [`Self::Connection`] error carrying a SQLSTATE + /// code (typically `08*` connection-class) and no I/O source. + pub fn connection_with_sqlstate( + message: impl Into, + sqlstate: impl Into, + ) -> Self { + Error::Connection { + message: message.into(), + source: None, + sqlstate: Some(sqlstate.into()), } } @@ -293,9 +349,21 @@ impl Error { Error::Protocol(message.into()) } - /// Constructs an [`Self::Closed`] error. + /// Constructs an [`Self::Closed`] error with no SQLSTATE. pub fn closed(message: impl Into) -> Self { - Error::Closed(message.into()) + Error::Closed { + message: message.into(), + sqlstate: None, + } + } + + /// Constructs an [`Self::Closed`] error carrying a SQLSTATE code + /// (typically `57P01` admin shutdown or `57P02` crash shutdown). + pub fn closed_with_sqlstate(message: impl Into, sqlstate: impl Into) -> Self { + Error::Closed { + message: message.into(), + sqlstate: Some(sqlstate.into()), + } } /// Constructs an [`Self::Timeout`] error. @@ -303,9 +371,24 @@ impl Error { Error::Timeout(message.into()) } - /// Constructs an [`Self::Cancelled`] error. + /// Constructs an [`Self::Cancelled`] error with no SQLSTATE. pub fn cancelled(message: impl Into) -> Self { - Error::Cancelled(message.into()) + Error::Cancelled { + message: message.into(), + sqlstate: None, + } + } + + /// Constructs an [`Self::Cancelled`] error carrying a SQLSTATE + /// code (typically `57014` `query_canceled`). + pub fn cancelled_with_sqlstate( + message: impl Into, + sqlstate: impl Into, + ) -> Self { + Error::Cancelled { + message: message.into(), + sqlstate: Some(sqlstate.into()), + } } /// Constructs an [`Self::Conversion`] error. @@ -343,6 +426,11 @@ impl Error { Error::AlreadyExists(message.into()) } + /// Constructs an [`Self::InvalidOperation`] error. + pub fn invalid_operation(message: impl Into) -> Self { + Error::InvalidOperation(message.into()) + } + /// Returns the error message in human-readable form. Equivalent to /// `self.to_string()`. #[must_use] @@ -350,8 +438,14 @@ impl Error { self.to_string() } - /// Returns the `PostgreSQL` SQLSTATE code if this is a - /// [`Self::Server`] error that carries one, otherwise `None`. + /// Returns the `PostgreSQL` SQLSTATE code if this error carries + /// one, otherwise `None`. + /// + /// Returns `Some(...)` for [`Self::Server`] (Query-class codes), + /// [`Self::Connection`] (typically `08*`), [`Self::Closed`] + /// (typically `57P0*` shutdown codes), and [`Self::Cancelled`] + /// (typically `57014` `query_canceled`) when the underlying server + /// provided a code. /// /// SQLSTATE codes are 5-character strings — see the [`PostgreSQL` /// errcodes appendix][1]. @@ -360,7 +454,10 @@ impl Error { #[must_use] pub fn sqlstate(&self) -> Option<&str> { match self { - Error::Server { sqlstate, .. } => sqlstate.as_deref(), + Error::Server { sqlstate, .. } + | Error::Connection { sqlstate, .. } + | Error::Closed { sqlstate, .. } + | Error::Cancelled { sqlstate, .. } => sqlstate.as_deref(), _ => None, } } @@ -381,11 +478,12 @@ impl Error { // "DETAIL: ..." and "HINT: ..." lines from those fields, so using // `chain` would duplicate the detail text. // -// SQLSTATE: `client::Error::sqlstate()` may return `Some` for non-Query -// kinds (e.g. SQLSTATE 57014 query_canceled comes back as Cancelled). -// The flat enum only carries `sqlstate` on `Server`, so SQLSTATE codes -// from non-Query kinds are folded into the message via `chain` rather -// than surfaced via `Error::sqlstate()`. Documented in MIGRATING-0.3. +// SQLSTATE: `client::Error::sqlstate()` may return `Some` for any +// kind. After Follow-up C, the flat enum carries `sqlstate` on +// `Server`, `Connection`, `Closed`, and `Cancelled` so callers can +// match on it programmatically (e.g. SQLSTATE 57014 `query_canceled` +// arrives via Cancelled and is now exposed structurally). Other +// variants still drop SQLSTATE — folded into the message via `chain`. impl From for Error { fn from(err: hyperdb_api_core::client::Error) -> Self { use hyperdb_api_core::client::ErrorKind as CoreKind; @@ -401,6 +499,7 @@ impl From for Error { CoreKind::Connection => Error::Connection { message: chain, source: None, + sqlstate, }, CoreKind::Authentication => Error::Authentication(chain), // Use unchained `message` here: detail/hint are passed as @@ -419,11 +518,18 @@ impl From for Error { CoreKind::Io => Error::Connection { message: chain, source: None, + sqlstate, }, CoreKind::Config => Error::Config(chain), CoreKind::Timeout => Error::Timeout(chain), - CoreKind::Cancelled => Error::Cancelled(chain), - CoreKind::Closed => Error::Closed(chain), + CoreKind::Cancelled => Error::Cancelled { + message: chain, + sqlstate, + }, + CoreKind::Closed => Error::Closed { + message: chain, + sqlstate, + }, CoreKind::Conversion => Error::Conversion(chain), CoreKind::FeatureNotSupported => Error::FeatureNotSupported(chain), CoreKind::Other => Error::Internal { message: chain }, @@ -533,14 +639,23 @@ mod tests { } #[test] - fn sqlstate_returns_some_only_for_server() { + fn sqlstate_returns_some_for_server_connection_closed_cancelled() { + // Server still surfaces SQLSTATE. let server = Error::server(Some("42P04".to_string()), "db exists", None, None); assert_eq!(server.sqlstate(), Some("42P04")); - // Non-Server variants must return None even if the SQLSTATE - // would have been present in the underlying client::Error. - // Documented behavior: only Server-variant SQLSTATEs surface - // through Error::sqlstate() in the flat enum. + // Connection / Closed / Cancelled now surface SQLSTATE + // structurally (Follow-up C). + let conn = Error::connection_with_sqlstate("connect failed", "08006"); + assert_eq!(conn.sqlstate(), Some("08006")); + + let closed = Error::closed_with_sqlstate("admin shutdown", "57P01"); + assert_eq!(closed.sqlstate(), Some("57P01")); + + let cancelled = Error::cancelled_with_sqlstate("user cancel", "57014"); + assert_eq!(cancelled.sqlstate(), Some("57014")); + + // Variants without sqlstate field return None. assert_eq!(Error::Conversion("...".into()).sqlstate(), None); assert_eq!( Error::Internal { @@ -549,7 +664,9 @@ mod tests { .sqlstate(), None ); - assert_eq!(Error::Cancelled("...".into()).sqlstate(), None); + + // Cancelled with no SQLSTATE returns None too. + assert_eq!(Error::cancelled("user cancel").sqlstate(), None); } #[test] @@ -606,4 +723,14 @@ mod tests { let err = Error::internal("invariant violated"); assert_eq!(err.to_string(), "internal error: invariant violated"); } + + #[test] + fn invalid_operation_constructor_round_trip() { + let err = Error::invalid_operation("cannot mix insert_data with insert_batch"); + assert_eq!( + err.to_string(), + "invalid operation: cannot mix insert_data with insert_batch" + ); + assert!(matches!(err, Error::InvalidOperation(_))); + } } diff --git a/hyperdb-api/src/process.rs b/hyperdb-api/src/process.rs index 52d40b9..858b0a0 100644 --- a/hyperdb-api/src/process.rs +++ b/hyperdb-api/src/process.rs @@ -341,16 +341,16 @@ impl HyperProcess { // Create callback listener on ephemeral port // This is the "dead man's switch" - when this connection is closed, Hyper shuts down let callback_listener = TcpListener::bind("127.0.0.1:0") - .map_err(|e| Error::internal(format!("Failed to create callback listener: {e}")))?; + .map_err(|e| Error::connection_with_io("Failed to create callback listener", e))?; let callback_port = callback_listener .local_addr() - .map_err(|e| Error::internal(format!("Failed to get callback port: {e}")))? + .map_err(|e| Error::connection_with_io("Failed to get callback port", e))? .port(); // Set a timeout for accepting the callback connection callback_listener.set_nonblocking(false).map_err(|e| { - Error::internal(format!("Failed to set callback listener to blocking: {e}")) + Error::connection_with_io("Failed to set callback listener to blocking", e) })?; // Check if user wants to disable default parameters @@ -384,7 +384,7 @@ impl HyperProcess { // Create a temp directory for the socket let temp_dir = std::env::temp_dir().join(format!("hyper-{}", std::process::id())); std::fs::create_dir_all(&temp_dir).map_err(|e| { - Error::internal(format!("Failed to create socket directory: {e}")) + Error::connection_with_io("Failed to create socket directory", e) })?; temp_dir }; @@ -610,11 +610,10 @@ impl HyperProcess { // Start the process let child = cmd.spawn().map_err(|e| { - Error::internal(format!( - "Failed to start Hyper server at {}: {}", - hyperd_path.display(), - e - )) + Error::connection_with_io( + format!("Failed to start Hyper server at {}", hyperd_path.display()), + e, + ) })?; // Wait for Hyper to connect back to our callback listener @@ -727,16 +726,17 @@ impl HyperProcess { thread::sleep(Duration::from_millis(50)); } Err(e) => { - return Err(Error::internal(format!( - "Failed to accept callback connection: {e}" - ))); + return Err(Error::connection_with_io( + "Failed to accept callback connection", + e, + )); } } }; // Set stream back to blocking for reading stream.set_nonblocking(false).map_err(|e| { - Error::internal(format!("Failed to set callback stream to blocking: {e}")) + Error::connection_with_io("Failed to set callback stream to blocking", e) })?; // Set read timeout @@ -746,7 +746,7 @@ impl HyperProcess { // Protocol: [1 byte length][N bytes descriptor string] let mut len_buf = [0u8; 1]; stream.read_exact(&mut len_buf).map_err(|e| { - Error::internal(format!("Failed to read endpoint length from Hyper: {e}")) + Error::connection_with_io("Failed to read endpoint length from Hyper", e) })?; let len = len_buf[0] as usize; @@ -756,9 +756,7 @@ impl HyperProcess { let mut descriptor_buf = vec![0u8; len]; stream.read_exact(&mut descriptor_buf).map_err(|e| { - Error::internal(format!( - "Failed to read endpoint descriptor from Hyper: {e}" - )) + Error::connection_with_io("Failed to read endpoint descriptor from Hyper", e) })?; let descriptor = String::from_utf8(descriptor_buf) @@ -1101,13 +1099,13 @@ impl HyperProcess { // Then force kill let _ = child.kill(); break child.wait().map_err(|e| { - Error::internal(format!("Failed to wait for hyperd: {e}")) + Error::connection_with_io("Failed to wait for hyperd", e) }); } thread::sleep(Duration::from_millis(100)); } Err(e) => { - break Err(Error::internal(format!("Failed to wait for hyperd: {e}"))) + break Err(Error::connection_with_io("Failed to wait for hyperd", e)) } } } @@ -1115,7 +1113,7 @@ impl HyperProcess { // Wait indefinitely child .wait() - .map_err(|e| Error::internal(format!("Failed to wait for hyperd: {e}"))) + .map_err(|e| Error::connection_with_io("Failed to wait for hyperd", e)) }; wait_result?; diff --git a/hyperdb-mcp/src/error.rs b/hyperdb-mcp/src/error.rs index cb67e9e..d71fd57 100644 --- a/hyperdb-mcp/src/error.rs +++ b/hyperdb-mcp/src/error.rs @@ -211,9 +211,9 @@ impl From for McpError { // so the engine recycles. is_connection_lost above already // catches most of these via message; this is a fallback. hyperdb_api::Error::Connection { .. } - | hyperdb_api::Error::Closed(_) + | hyperdb_api::Error::Closed { .. } | hyperdb_api::Error::Timeout(_) - | hyperdb_api::Error::Cancelled(_) => McpError::new(ErrorCode::ConnectionLost, msg), + | hyperdb_api::Error::Cancelled { .. } => McpError::new(ErrorCode::ConnectionLost, msg), _ => McpError::new(ErrorCode::InternalError, msg), }