Skip to content

Error conversion friction: from_error, source chain, bail!/ensure!, typed details #93

@iainmcgin

Description

@iainmcgin

Returning errors from a handler today requires explicit conversion at every fallible call site. ConnectError only has two From impls (std::io::Error and http::Error), so the common pattern in real handlers is:

async fn get_user(&self, _ctx: RequestContext, req: OwnedGetUserRequestView) -> ServiceResult<User> {
    let id = parse_id(req.id)
        .map_err(|e| ConnectError::invalid_argument(e.to_string()))?;
    let row = self.db.fetch(id).await
        .map_err(|e| ConnectError::internal(e.to_string()))?;
    let user = row_to_user(row)
        .map_err(|e| ConnectError::internal(e.to_string()))?;
    Response::ok(user)
}

Three .map_err(|e| ConnectError::xxx(e.to_string()))? lines for one happy path. tonic users have the same friction with Status, but tonic at least ships Status::from_error(Box<dyn Error>). connect-go avoids it entirely via errors.As — a handler returns plain error, and the framework finds an embedded *connect.Error in the chain or defaults to CodeUnknown.

There is also no ConnectError::source() — the original error is stringified into message and lost, so middleware/logging cannot inspect the underlying cause without re-parsing the message.

Proposed direction

Tier 1 — from_error, source, bail!/ensure!

Add ConnectError::from_error(impl Into<Box<dyn Error + Send + Sync>>), mirroring tonic::Status::from_error but walking the source() chain for an embedded ConnectError first (the analogue of connect-go's errors.As):

impl ConnectError {
    /// Build a `ConnectError` from any error.
    ///
    /// If the source chain contains a `ConnectError`, it is returned (preserving
    /// its code, message, and details). Otherwise produces an `Internal` error
    /// with `err.to_string()` as the message and the original error retained
    /// as the `source()`.
    pub fn from_error(err: impl Into<Box<dyn std::error::Error + Send + Sync>>) -> Self;
}

This requires adding source: Option<Box<dyn Error + Send + Sync>> to ConnectError, which is independently valuable: it gives an internal/external split where message is what the client sees and source is what the operator logs (the same split dropshot::HttpError uses by design). impl std::error::Error for ConnectError should expose it via fn source().

Add bail! and ensure! macros for the early-return cases:

ensure!(!req.name.is_empty(), ErrorCode::InvalidArgument, "name is required");
let id = req.id.unwrap_or_else(|| bail!(ErrorCode::InvalidArgument, "id is required"));

After Tier 1, the handler above becomes:

async fn get_user(&self, _ctx: RequestContext, req: OwnedGetUserRequestView) -> ServiceResult<User> {
    ensure!(req.id.is_some(), ErrorCode::InvalidArgument, "id is required");
    let id = parse_id(req.id).map_err(ConnectError::from_error)?;
    let row = self.db.fetch(id).await.map_err(ConnectError::from_error)?;
    let user = row_to_user(row).map_err(ConnectError::from_error)?;
    Response::ok(user)
}

Better, but ? still doesn't work directly. That's Tier 2.

Tier 2 — ? ergonomics

A blanket impl<E: Error + Send + Sync + 'static> From<E> for ConnectError would make ? work directly, but it conflicts with downstream impl From<MyError> for ConnectError impls (overlap), and needs a sealed NotConnectError marker to avoid colliding with the reflexive From<ConnectError> for ConnectError. It also encodes a policy decision — every unmapped error becomes Internal — that production codebases usually want to override per error variant anyway.

Recommendation: do not ship a blanket From. Instead, document the recommended pattern in the guide:

// Define one From impl for your service's error type.
impl From<AppError> for ConnectError {
    fn from(e: AppError) -> Self {
        match e {
            AppError::NotFound(_) => ConnectError::not_found(e.to_string()),
            AppError::Validation(_) => ConnectError::invalid_argument(e.to_string()),
            AppError::Db(_) => ConnectError::from_error(e),
        }
    }
}

// ... and ? works everywhere in handlers that return ServiceResult.

This is honest about the design space and gives users code-specific mapping for free, which a blanket Internal mapping cannot. from_error covers the "I genuinely don't care about the code" cases.

If demand for the blanket exists after Tier 1 ships, it can be added behind a from-error feature flag with a documented coherence caveat.

Tier 3 — typed error details

ErrorDetail is currently a raw { type_url: String, value: Option<base64-string>, debug } struct. To attach a google.rpc.BadRequest you must compute the type URL and base64-encode the proto bytes by hand. Both connect-go (connect.NewErrorDetail(msg proto.Message)) and tonic (tonic_types::StatusExt) do the Any packing for you.

Add typed pack/unpack helpers:

impl ConnectError {
    /// Pack a proto message as a google.protobuf.Any error detail.
    pub fn with_detail_message<M: Message>(self, msg: &M) -> Result<Self, ConnectError>;

    /// Extract all details of a given message type.
    pub fn details_of<M: Message>(&self) -> Result<Vec<M>, ConnectError>;
}

This depends on buffa exposing the message's fully-qualified name for the type.googleapis.com/<full-name> URL.

Optionally, a connectrpc-types companion crate with pre-generated google.rpc.* (BadRequest, RetryInfo, QuotaFailure, ErrorInfo, PreconditionFailure, ...) types, mirroring tonic-types, so users don't have to vendor those protos.

Sequencing

  • Tier 1 is small and unambiguous; suitable for a 0.5 milestone.
  • Tier 2 is a documentation change.
  • Tier 3 depends on buffa surfacing message full names; track separately.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions