tmyers273 / error-context

An error wrapper, bringing anyhow like errors to thiserror
2 stars 1 forks source link

Trait bound is not satisfied if two errors implement the same #[from] #2

Open GCastilho opened 4 weeks ago

GCastilho commented 4 weeks ago

So, I don't know if I'm doing something wrong, but if I have two errors that implement the same #[from] trait I get a compilation error and can't use neither

Here's an example:

#[derive(Debug, Error)]
enum InnerA {
    #[error("InnerA Sqlx")]
    Sqlx(#[from] sqlx::Error),
}

impl_context!(OuterA(InnerA));

#[derive(Debug, Error)]
enum InnerB {
    #[error("InnerB Sqlx")]
    Sqlx(#[from] sqlx::Error),
}

impl_context!(OuterB(InnerB));

async fn outer_a(pool: &mut Pool<Sqlite>) -> Result<(), OuterA> {
    let mut tx = pool.begin().await?;
    sqlx::query!("CREATE TABLE IF NOT EXISTS some_table (id INTEGER PRIMARY KEY AUTOINCREMENT)")
        .execute(&mut *tx)
        .await
        .context("outer_a")?; // <---- the trait bound `std::result::Result<SqliteQueryResult, sqlx::Error>: thiserror_context::Context<_, (), _>` is not satisfied

    Ok(())
}

image

The error occur even if they are in different modules

I've made a repository to reproduce it: https://github.com/GCastilho/thiserror-context-trait-not-satisfied-reproduction. Instructions on README

Please let me know if I can help with anything. Thank you

GCastilho commented 3 weeks ago

I've implemented a test case on a branch of mine that reproduces this build error in an even simpler way

tmyers273 commented 3 weeks ago

Hey @GCastilho,

Thanks for the detailed bug report!

Looking at your branch, it looks like the anyhow import may be causing some issues there. Link. The latest commit seems to be working with that particular test case.

However, I was able to reproduce the original sqlx version you posted.

TLDR: Adding an explicit conversion should get things compiling. I know it's not the most ergonomic, but I wasn't able to figure out a way to avoid it in a case like this.

async fn outer_a(pool: &mut Pool<Sqlite>) -> Result<(), OuterA> {
    let mut tx = pool.begin().await?;
    sqlx::query!(
        "CREATE TABLE IF NOT EXISTS some_table (id INTEGER PRIMARY KEY AUTOINCREMENT)"
    )
    .execute(&mut *tx)
    .await
    // .map_err(InnerA::from) // <---- uncomment this line
    .context("outer_a")?; // <---- the trait bound `std::result::Result<SqliteQueryResult, sqlx::Error>: thiserror_context::Context<_, (), _>` is not satisfied

    Ok(())
}

It looks like there were 2 factors contributing to the error:

  1. the sqlx response was Result<DB::QueryResult, _>, while the function response was Result<(), _>. That caused the Ok variant type of each Result to be differen, hiding the real error.
  2. the compiler can't infer if the sqlx error should be converted to InnerA or InnerB.
GCastilho commented 3 weeks ago

It seems that using the .map_err(InnerA::from) is the only way to make the code to compile

I've investigated a little further and the problem lies in the Into<$out> part image If there are multiple Into<$out> implementations then the compiler can't know if you're referring to InnerA or InnerB (as you mentioned)

This seems to be caused by the ? not being smart enough to understand that the return type, OuterA, implies that InnerA is the only valid implementation for Context here

I've update my branch with a modified test case where the error occur, for completion sake, image but I don't think there is anything the lib can do to fix this without an explicit conversion

Thanks for the quick patch, I managed to solve my original problem with it and the explicit conversion

I think the issue can be closed too