programatik29 / tokio-rusqlite

Asynchronous handle for rusqlite library.
MIT License
89 stars 21 forks source link

Revert to application type errors #20

Closed aalowlevel closed 1 year ago

aalowlevel commented 1 year ago

Hello. It's always problematic when i want to write async wrappers and i am not interested to work with neither tokio_rusqlite type errors nor rusqlite type errors:

    /** Initialization of the program (async) */
    pub async fn init(connection: Arc<tokio_rusqlite::Connection>) -> Result<Init, MyError> {
        connection
            .call(|connection| crate::config::init(connection)) // this wants to return `MyError` as well.
            // But the new version only wants to work with its error type. I don't want to return `tokio_rusqlite::Error`
            .await
    }

I just want to work like i used to in 0.3.0. Just go back to the old style.

programatik29 commented 1 year ago

Thanks for feedback. The problem is we added close support and that requires call returning error.

aalowlevel commented 1 year ago

You might consider a callback for close function as well. Well, I don't know. In this way of design, the call function forces the app error type to be wrapped into the error of its type .

programatik29 commented 1 year ago

I'll try to figure out a way.

czocher commented 1 year ago

@programatik29 we can provide two interfaces since clearly both have their pros and cons? Maybe hide the close method behind a feature flag? What do you think?

programatik29 commented 1 year ago

@programatik29 we can provide two interfaces since clearly both have their pros and cons? Maybe hide the close method behind a feature flag? What do you think?

Maybe we can just add a panicking call method and solve this. If close is never called, it would never panic. With proper documentation I think this might be a good solution.

czocher commented 1 year ago

Sure, any specific name you have in mind?

programatik29 commented 1 year ago

Sure, any specific name you have in mind?

@Czocher Naming it call_unwrap and documenting panic behavior should be fine.