Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLR exceptions #54

Merged
merged 13 commits into from
Jul 8, 2023
Merged

CLR exceptions #54

merged 13 commits into from
Jul 8, 2023

Conversation

pimbrouwers
Copy link
Owner

  • Removes Result from execution pipeline.
  • Introduces SqlType help functions.

@pimbrouwers pimbrouwers changed the title Clr exceptions CLR exceptions Jul 8, 2023
@pimbrouwers pimbrouwers merged commit 8c0b0a1 into master Jul 8, 2023
1 check passed
@sbacquet
Copy link
Contributor

sbacquet commented Jul 27, 2023

This is a major breaking change.
What is the rationale behind ?

@pimbrouwers
Copy link
Owner Author

pimbrouwers commented Jul 27, 2023 via email

@sbacquet
Copy link
Contributor

sbacquet commented Jul 28, 2023

Indeed, which is why it was a major package number bump.

Ok, but I guess branch 9.x will not be maintained ;)

there was hardly a salient use case for handling errors monadically

Are you talking about DB execution specifically, or in general ?

If the DB execution fails, what are you reasonably going to do other than log/report the error?

DB execution is just a step in monadic pipelines.
Throwing exceptions is not idiomatic and breaks the pipeline.
To me, returning Result<'T, System.Exception> is the best way to manage errors in F#.

Anyway for Donald it's not a big deal : I'll surround the calls to Db.exec and Db.Async.exec with try/with and return Error exception.

@pimbrouwers
Copy link
Owner Author

pimbrouwers commented Jul 28, 2023 via email

@sbacquet
Copy link
Contributor

Donald is working like a charm, thanks for publishing this nice library :)
I'll adapt my code to version 10, no worries.

@pimbrouwers
Copy link
Owner Author

pimbrouwers commented Jul 28, 2023 via email

@sbacquet
Copy link
Contributor

Hello - sorry to bother you
I'm migrating my code to version 10 with CLR exceptions.
The exceptions that I'm catching are of type : AggregateException -> Inner = Donald.DbExecutionException -> Inner = OracleException
That's a lot of layers. Do you know why an AggregateException is thrown and how to avoid it ?
Thanks !

@sbacquet
Copy link
Contributor

sbacquet commented Sep 21, 2023

OK the issue is within Async.AwaitTask, which does not unwrap properly the exception thrown in another thread.
It's actively discussed here and should be fixed some day in F#.

To confirm the issue, the following test fails :

    [<Fact>]
    member _.``SELECT records async should fail`` () =
        let sql = "
            SELECT author_id, full_name
            FROM   fake_author"

        let query () =
            try
                conn
                |> Db.newCommand sql
                |> Db.Async.query Author.FromReader
                |> Async.AwaitTask
                |> Async.RunSynchronously
                |> ignore
            with ex ->
                ex |> ignore

        query |> should throw typeof<DbExecutionException>

which is just the async version of test SELECT records should fail.

@sbacquet
Copy link
Contributor

sbacquet commented Sep 21, 2023

I've ended up with the following adapter :

let rec donaldExToError (ex:Exception) =
    match ex with
    | :? Donald.DbConnectionException as ex ->
        Error ex.InnerException
    | :? Donald.DbExecutionException as ex ->
        Error ex.InnerException
    | :? Donald.DbTransactionException as ex ->
        Error ex.InnerException
    | :? Donald.DbReaderException as ex ->
        Error ex.InnerException
    | :? AggregateException as ex ->
        donaldExToError ex.InnerException
    | ex ->
        Error ex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants