digital-society-coop / axum-sqlx-tx

Request-scoped sqlx transactions for axum
MIT License
45 stars 11 forks source link

How to use a Tx in a middleware fn? #23

Closed agasparovic closed 8 months ago

agasparovic commented 10 months ago

The documentation mentions:

A transaction begins the first time the extractor is used for a request, and is then stored in request extensions for use by other middleware/handlers.

It seems like this means that the transaction should be able to be used by middleware as well as the main request handler.

But if I try to add a Tx as a parameter to a middleware fn (and register it via middleware::from_fn), I get an OverlappingExtractors error. I believe this is because I have the Tx in scope when calling next.run(request).await on a handler that also extracts Tx.

Is there any way to have request handlers that use Tx, while also using the Tx from a middleware function?

connec commented 9 months ago

Thank you for the issue! It's an interesting find – I did expect that this use case would work but I hadn't tested it.

The error occurs because the middleware's Tx is in scope when the request handler is invoked by next.run(request).await, which attempts to extract Tx again and finds that the transaction is already taken.

Under hood, the transaction is essentially stored in an Option (technically Slot) in the axum_sqlx_tx::Layer. When the Tx extractor runs, it takes the transaction from the Option, and it gets put back when the Tx is dropped (e.g. when the handler is done).

It's possible to make some progress by explicitly dropping tx before invoking the handler. However it's not possible to re-extract after invoking the handler as the request is moved.

I spent a bit of time looking at options to make further progress, but I wasn't able to find anything that worked. Tx behaves a bit like it is borrowed from the Request, and so it is invalidated when the Request is moved into next.run. Unfortunately this is not visible to the compiler, as extractors can't actually borrow from the request.

It's possible that an explicit release/try_acquire mechanism could make this work, e.g.

async fn my_middleware<B>(tx: Tx, request: Request<B>, next: Next<B>) -> Response {
    // do something with tx
    // ...

    // relinquish our access to the tx, this should take ownership and change the type so it can't be used
    let tx = tx.release();

    // call the next handler, which could happily extract the tx again
    let response = next.run(handler).await;

    // and now we can attempt to reacquire the tx - it could fail if the handler explicitly committed
    // (or otherwise "lost" the `Tx`, e.g. with mem::forget)
    let tx = tx.try_acquire().unwrap();

    // do more with tx
    // ...

    response
}

Can I ask what your use case for this is?

agasparovic commented 9 months ago

Thanks for the explanation! My use case is for implementing row-level security. I'd like to call set_config inside each transaction to set the current app user for the transaction, after middleware that extracts the current user, but before each request handler so I don't have to do it manually.

connec commented 9 months ago

For that use case, it will work to explicitly drop the tx handle in the middleware before calling the handler.

I'll add a test to cover that behaviour.

agasparovic commented 9 months ago

Thanks! I can confirm that dropping the tx handle works.