Future returned by transaction is not Send

[mattg23]

Describe the bug Using transactions is not Send.

Reproduction I am using the client in an async axum handler with the following code:

pub async fn query_json(client: &edgedb_tokio::Client, query: &str) -> anyhow::Result<String, anyhow::Error> {
    let ro_client = client
        .with_transaction_options(edgedb_tokio::TransactionOptions::default().read_only(true));

    let result = ro_client
        .transaction(|mut tx| async move { tx.query_json(query, &()).await })
        .await;

    match result {
        Ok(result) => Ok(result.into()),
        Err(e) => Err(anyhow::Error::new(e)),
    }
}

The rust compiler complains about:

error: future cannot be sent between threads safely
   --> reos-server\src\main.rs:245:1
    |
245 | async fn query_json(
    | ^^^^^ future returned by `query_json` is not `Send`
    |
    = help: the trait `Sync` is not implemented for `dyn StdError`
note: future is not `Send` as this value is used across an await
   --> C:\Users\XXXXXXXXX\.cargo\registry\src\github.com-1ecc6299db9ec823\edgedb-tokio-0.3.0\src\transaction.rs:101:65
    |
93  |                 for e in e.chain() {
    |                     - has type `&dyn StdError` which is not `Send`
...
101 |                                 sleep((rule.backoff)(iteration)).await;
    |                                                                 ^^^^^^ await occurs here, with `e` maybe used later
...
106 |                 }
    |                 - `e` is later dropped here
note: required by a bound in `__axum_macros_check_query_json_future::check`
   --> reos-server\src\main.rs:245:1
    |
245 | async fn query_json(
    | ^^^^^ required by this bound in `__axum_macros_check_query_json_future::check`

For more information about this error, try `rustc --explain E0277`.

I've moved the check for the SHOULD_RETRY tag, such that the &dyn StdError can be dropped before the await. This change https://github.com/mattg23/edgedb-rust/commit/20d0e467e3359995bca6ad399d8dfa534c38b851 works for me. The behavior of the code should be identical to how it was before.

I'm quite new to rust and edgedb so I wanted to open the issue and not just send a PR. Any feedback is welcome.

Versions (please complete the following information):

• OS: Win11
• EdgeDB version: 2.7+81de4dd
• EdgeDB CLI version: EdgeDB CLI 2.2.6+af56767
• Rust compiler version: rustc 1.65.0 (897e37553 2022-11-02)
• Cargo version: cargo 1.65.0 (4bc8f24d3 2022-10-20)
• EdgeDB Rust versions: edgedb-tokio = "0.3.0" & my fork

[tailhook]

Generally I agree with the solution. But I think it can be made a bit simpler, and also I think we should insert compile-time assertions to ensure that this bug doesn't appear later and in other places. I had no chance to take a look at this yet, though.

[mattg23]

My plan is to work on this project between christmas and new year again. If you have any ideas / pointers, I can try improving it.

[tailhook]

Well, I can't come up with something significantly better (except maybe let..else but that's too early probably). Just fix formatting (spaces around else and indentation of the second line of log).

Assertion can be done by adding a function that enforce types, but otherwise unused in the code:

#[allow(dead_code)]
fn _assertions() {
    let cli: Client = unimplemented!();
    assert_send_sync(cli.transaction(|tx| _));
}
fn assert_send_sync<T: Send+Sync>(_: T) {}

[mattg23]

Sounds good, I'll prep a PR after christmas.

[tailhook]

Fixed in #199