search

paurkedal / ocaml-caqti

Cooperative-threaded access to relational data
https://paurkedal.github.io/ocaml-caqti/index.html
GNU Lesser General Public License v3.0
309 stars 36 forks source link

Assert vs. Exception with message vs. Caqti_error.t #43

Closed joseferben closed 3 years ago

joseferben commented 4 years ago

I had an issue when I ran into assert_single_use by using Lwt.all ... to run a list of queries on a connection where it just fails without error message. Would you accept a PR where I change that assert to throw an exception containing an error message that gives a hint, that it is not allowed to run multiple queries at the some time on one connection?

Would it even make sense maybe to get rid of all asserts and return Caqti_error.t?

paurkedal commented 4 years ago

Yes, a PR for raising a an exception with a more informative message would be appreciated. I suggest using Failure for this.

With respect to Caqti_error.t, this should not be used here. Caqti uses the result type to indicate runtime errors, i.e. errors which are outside the control of the application, and which should be handled. On the other hand, failing the single-use test is a bug in the application, and should not be handled, other than to report it a such.

paurkedal commented 3 years ago

A more informative exception was already added in bb08bfb80012013264ca0b76ceeaf3fad961c7b7, so I'll close this.