Merge pull request #8 from wasdacraic/error-handling

Error handling

Author: Chris Connelly

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "axum-sqlx-tx"
 description = "Request-scoped SQLx transactions for axum"
-version = "0.1.2"
+version = "0.2.0"
 license = "MIT"
 repository = "https://github.com/wasdacraic/axum-sqlx-tx/"
 edition = "2021"
@@ -28,8 +28,10 @@ features = ["all-databases", "runtime-tokio-rustls"]
 
 [dependencies]
 axum-core = "0.1.2"
+bytes = "1.1.0"
 futures-core = "0.3.21"
 http = "0.2.6"
+http-body = "0.4.4"
 parking_lot = "0.12.0"
 sqlx = { version = "0.5.11", default-features = false }
 thiserror = "1.0.30"

examples/example.rs

@@ -2,7 +2,7 @@
 
 use std::error::Error;
 
-use axum::{error_handling::HandleErrorLayer, response::IntoResponse, routing::get, Json};
+use axum::{response::IntoResponse, routing::get, Json};
 use http::StatusCode;
 
 // OPTIONAL: use a type alias to avoid repeating your database type
@@ -22,16 +22,8 @@ async fn main() -> Result<(), Box<dyn Error>> {
     // Standard axum app setup
     let app = axum::Router::new()
         .route("/numbers", get(list_numbers).post(generate_number))
-        .layer(
-            // Apply the Tx middleware
-            tower::ServiceBuilder::new()
-                .layer(HandleErrorLayer::new(|error: sqlx::Error| async move {
-                    // The transaction is committed by the middleware so an error is possible
-                    // and must be converted into a response.
-                    DbError(error)
-                }))
-                .layer(axum_sqlx_tx::Layer::new(pool.clone())),
-        );
+        // Apply the Tx middleware
+        .layer(axum_sqlx_tx::Layer::new(pool.clone()));
 
     let server = axum::Server::bind(&([0, 0, 0, 0], 0).into()).serve(app.into_make_service());
 

src/layer.rs

@@ -1,8 +1,13 @@
 //! A [`tower_layer::Layer`] that enables the [`Tx`](crate::Tx) extractor.
 
+use std::marker::PhantomData;
+
+use axum_core::response::IntoResponse;
+use bytes::Bytes;
 use futures_core::future::BoxFuture;
+use http_body::{combinators::UnsyncBoxBody, Body};
 
-use crate::tx::TxSlot;
+use crate::{tx::TxSlot, Error};
 
 /// A [`tower_layer::Layer`] that enables the [`Tx`] extractor.
 ///
@@ -15,8 +20,9 @@ use crate::tx::TxSlot;
 ///
 /// [`Tx`]: crate::Tx
 /// [request extensions]: https://docs.rs/http/latest/http/struct.Extensions.html
-pub struct Layer<DB: sqlx::Database> {
+pub struct Layer<DB: sqlx::Database, E = Error> {
     pool: sqlx::Pool<DB>,
+    _error: PhantomData<E>,
 }
 
 impl<DB: sqlx::Database> Layer<DB> {
@@ -28,54 +34,72 @@ impl<DB: sqlx::Database> Layer<DB> {
     /// If you want to access the pool outside of a transaction, you should add it also with
     /// [`axum::Extension`].
     ///
+    /// To use a different type than [`Error`] to convert commit errors into responses, see
+    /// [`new_with_error`](Self::new_with_error).
+    ///
     /// [`axum::Extension`]: https://docs.rs/axum/latest/axum/extract/struct.Extension.html
     pub fn new(pool: sqlx::Pool<DB>) -> Self {
-        Self { pool }
+        Self::new_with_error(pool)
+    }
+
+    /// Construct a new layer with a specific error type.
+    ///
+    /// See [`Layer::new`] for more information.
+    pub fn new_with_error<E>(pool: sqlx::Pool<DB>) -> Layer<DB, E> {
+        Layer {
+            pool,
+            _error: PhantomData,
+        }
     }
 }
 
-impl<DB: sqlx::Database, S> tower_layer::Layer<S> for Layer<DB> {
-    type Service = Service<DB, S>;
+impl<DB: sqlx::Database, S, E> tower_layer::Layer<S> for Layer<DB, E> {
+    type Service = Service<DB, S, E>;
 
     fn layer(&self, inner: S) -> Self::Service {
         Service {
             pool: self.pool.clone(),
             inner,
+            _error: self._error,
         }
     }
 }
 
 /// A [`tower_service::Service`] that enables the [`Tx`](crate::Tx) extractor.
 ///
 /// See [`Layer`] for more information.
-pub struct Service<DB: sqlx::Database, S> {
+pub struct Service<DB: sqlx::Database, S, E = Error> {
     pool: sqlx::Pool<DB>,
     inner: S,
+    _error: PhantomData<E>,
 }
 
 // can't simply derive because `DB` isn't `Clone`
-impl<DB: sqlx::Database, S: Clone> Clone for Service<DB, S> {
+impl<DB: sqlx::Database, S: Clone, E> Clone for Service<DB, S, E> {
     fn clone(&self) -> Self {
         Self {
             pool: self.pool.clone(),
             inner: self.inner.clone(),
+            _error: self._error,
         }
     }
 }
 
-impl<DB: sqlx::Database, S, ReqBody, ResBody> tower_service::Service<http::Request<ReqBody>>
-    for Service<DB, S>
+impl<DB: sqlx::Database, S, E, ReqBody, ResBody> tower_service::Service<http::Request<ReqBody>>
+    for Service<DB, S, E>
 where
     S: tower_service::Service<
         http::Request<ReqBody>,
         Response = http::Response<ResBody>,
         Error = std::convert::Infallible,
     >,
     S::Future: Send + 'static,
-    ResBody: Send,
+    E: From<Error> + IntoResponse,
+    ResBody: Body<Data = Bytes> + Send + 'static,
+    ResBody::Error: Into<Box<dyn std::error::Error + Send + Sync + 'static>>,
 {
-    type Response = S::Response;
-    type Error = sqlx::Error;
+    type Response = http::Response<UnsyncBoxBody<ResBody::Data, axum_core::Error>>;
+    type Error = S::Error;
     type Future = BoxFuture<'static, Result<Self::Response, Self::Error>>;
 
     fn poll_ready(
@@ -94,19 +118,18 @@ where
             let res = res.await.unwrap(); // inner service is infallible
 
             if res.status().is_success() {
-                transaction.commit().await?;
+                if let Err(error) = transaction.commit().await {
+                    return Ok(E::from(Error::Database { error }).into_response());
+                }
             }
 
-            Ok(res)
+            Ok(res.map(|body| body.map_err(axum_core::Error::new).boxed_unsync()))
         })
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use axum::error_handling::HandleErrorLayer;
-    use tower::ServiceBuilder;
-
     use super::Layer;
 
     // The trait shenanigans required by axum for layers are significant, so this "test" ensures
@@ -117,11 +140,7 @@ mod tests {
 
         let app = axum::Router::new()
             .route("/", axum::routing::get(|| async { "hello" }))
-            .layer(
-                ServiceBuilder::new()
-                    .layer(HandleErrorLayer::new(|_: sqlx::Error| async {}))
-                    .layer(Layer::new(pool)),
-            );
+            .layer(Layer::new(pool));
 
         axum::Server::bind(todo!()).serve(app.into_make_service());
     }

src/lib.rs

@@ -19,30 +19,20 @@
 //!
 //! To use the [`Tx`] extractor, you must first add [`Layer`] to your app:
 //!
-//! ```no_run
-//! # use axum::error_handling::HandleErrorLayer;
+//! ```
 //! # async fn foo() {
 //! let pool = /* any sqlx::Pool */
 //! # sqlx::SqlitePool::connect(todo!()).await.unwrap();
 //! let app = axum::Router::new()
 //!     // .route(...)s
-//!     .layer(
-//!         tower::ServiceBuilder::new()
-//!             // The transaction is committed by the middleware so an error is possible and must
-//!             // be converted into a response
-//!             .layer(HandleErrorLayer::new(|error: sqlx::Error| async move {
-//!                 http::StatusCode::INTERNAL_SERVER_ERROR
-//!             }))
-//!             // Now we can add the middleware
-//!             .layer(axum_sqlx_tx::Layer::new(pool)),
-//!     );
+//!     .layer(axum_sqlx_tx::Layer::new(pool));
 //! # axum::Server::bind(todo!()).serve(app.into_make_service());
 //! # }
 //! ```
 //!
 //! You can then simply add [`Tx`] as an argument to your handlers:
 //!
-//! ```no_run
+//! ```
 //! use axum_sqlx_tx::Tx;
 //! use sqlx::Sqlite;
 //!
@@ -65,6 +55,56 @@
 //! you have multiple `Tx` arguments in a single handler, or call `Tx::from_request` multiple times
 //! in a single middleware.
 //!
+//! ## Error handling
+//!
+//! `axum` requires that middleware do not return errors, and that the errors returned by extractors
+//! implement `IntoResponse`. By default, [`Error`](Error) is used by [`Layer`] and [`Tx`] to
+//! convert errors into HTTP 500 responses, with the error's `Display` value as the response body,
+//! however it's generally not a good practice to return internal error details to clients!
+//!
+//! To make it easier to customise error handling, both [`Layer`] and [`Tx`] have a second generic
+//! type parameter, `E`, that can be used to override the error type that will be used to convert
+//! the response.
+//!
+//! ```
+//! use axum::response::IntoResponse;
+//! use axum_sqlx_tx::Tx;
+//! use sqlx::Sqlite;
+//!
+//! struct MyError(axum_sqlx_tx::Error);
+//!
+//! // Errors must implement From<axum_sqlx_tx::Error>
+//! impl From<axum_sqlx_tx::Error> for MyError {
+//!     fn from(error: axum_sqlx_tx::Error) -> Self {
+//!         Self(error)
+//!     }
+//! }
+//!
+//! // Errors must implement IntoResponse
+//! impl IntoResponse for MyError {
+//!     fn into_response(self) -> axum::response::Response {
+//!         // note that you would probably want to log the error or something
+//!         (http::StatusCode::INTERNAL_SERVER_ERROR, "internal server error").into_response()
+//!     }
+//! }
+//!
+//! // Change the layer error type
+//! # async fn foo() {
+//! # let pool: sqlx::SqlitePool = todo!();
+//! let app = axum::Router::new()
+//!     // .route(...)s
+//!     .layer(axum_sqlx_tx::Layer::new_with_error::<MyError>(pool));
+//! # axum::Server::bind(todo!()).serve(app.into_make_service());
+//! # }
+//!
+//! // Change the extractor error type
+//! async fn create_user(mut tx: Tx<Sqlite, MyError>, /* ... */) {
+//!     /* ... */
+//! }
+//! ```
+//!
+//! # Examples
+//!
 //! See [`examples/`][examples] in the repo for more examples.
 //!
 //! [examples]: https://github.com/wasdacraic/axum-sqlx-tx/tree/master/examples
@@ -77,5 +117,61 @@ mod tx;
 
 pub use crate::{
     layer::{Layer, Service},
-    tx::{Error, Tx},
+    tx::Tx,
 };
+
+/// Possible errors when extracting [`Tx`] from a request.
+///
+/// `axum` requires that the `FromRequest` `Rejection` implements `IntoResponse`, which this does
+/// by returning the `Display` representation of the variant. Note that this means returning
+/// configuration and database errors to clients, but you can override the type of error that
+/// `Tx::from_request` returns using the `E` generic parameter:
+///
+/// ```
+/// use axum::response::IntoResponse;
+/// use axum_sqlx_tx::Tx;
+/// use sqlx::Sqlite;
+///
+/// struct MyError(axum_sqlx_tx::Error);
+///
+/// // The error type must implement From<axum_sqlx_tx::Error>
+/// impl From<axum_sqlx_tx::Error> for MyError {
+///     fn from(error: axum_sqlx_tx::Error) -> Self {
+///         Self(error)
+///     }
+/// }
+///
+/// // The error type must implement IntoResponse
+/// impl IntoResponse for MyError {
+///     fn into_response(self) -> axum::response::Response {
+///         (http::StatusCode::INTERNAL_SERVER_ERROR, "internal server error").into_response()
+///     }
+/// }
+///
+/// async fn handler(tx: Tx<Sqlite, MyError>) {
+///     /* ... */
+/// }
+/// ```
+#[derive(Debug, thiserror::Error)]
+pub enum Error {
+    /// Indicates that the [`Layer`](crate::Layer) middleware was not installed.
+    #[error("required extension not registered; did you add the axum_sqlx_tx::Layer middleware?")]
+    MissingExtension,
+
+    /// Indicates that [`Tx`] was extracted multiple times in a single handler/middleware.
+    #[error("axum_sqlx_tx::Tx extractor used multiple times in the same handler/middleware")]
+    OverlappingExtractors,
+
+    /// A database error occurred when starting the transaction.
+    #[error(transparent)]
+    Database {
+        #[from]
+        error: sqlx::Error,
+    },
+}
+
+impl axum_core::response::IntoResponse for Error {
+    fn into_response(self) -> axum_core::response::Response {
+        (http::StatusCode::INTERNAL_SERVER_ERROR, self.to_string()).into_response()
+    }
+}