Api encouraging incorrect implementation

[turboMaCk]

This PR rotted a lot and should probably be closed: https://github.com/Holmusk/postgresql-simple-named/pull/28

Anyway I would like to instead open an issue to discuss the core problem it was attempting to address.

This library exposes 2 types of errors:

1. WithNamedError is used for errors that come from implementation of this library (named argument not found)
2. IO exceptions that happen within postgresql-simple

My claim is that one never wants to handle the 1 (it's implementation error not legitimate runtime error in most cases). But almost always wants to handle some subset of 2.

The problem

Putting WithNamedError thing aside lets clarify why IO exceptions matter.

Imagine a table with UNIQUE constrained. For obvious reasons when inserting data to the table there is an expected error that could happen - violation of unique constrained. This error most likely needs to be clearly communicated to caller - for instance the specific 400 response with information that name is taken in some form should be rendered by HTTP server.

The solution that this library encourages is to write 2 queries (why is explained bellow). One to check if given unique column doesn't already exist and then performing the insert. This solution is of course incorrect. In presence of concurrency some other thread could create conflicting entry in between the select and insert.

The correct solution is to perform insert and catch unique constrained violation. So what does it take to do this?

With postgresql-simple one works with IO so catching this means catching exception in IO monad. However with postgresql-simple-named this IO is lifted to LiftIO m => m. To catch the exception one needs to use construct that could catch it in the m (lifted IO context). For instance using Control.Exception.Lifted which is not part base or implement similar exception handler to this one. I claim that this is no good because good API should make correct implementation simpler not harder.

Proposed solutions

There are multiple potential solutions. I have some ideas myself but first I would like to see what others think.

Currently there is an issue for one possible approach https://github.com/Holmusk/postgresql-simple-named/issues/27

[jhrcek]

I feel that nowadays people are using unliftio or similar "lifter IO" lib to catch exceptions, which already makes it possible to catch exceptions thrown by this library relatively easily. Of course there's still the downside that exceptions are not visible in types. I find it unfortunate that this library is adding MonadError as separate route through which exceptions can be thrown, but I'd like to avoid the trap of exposing more exceptions via this library's MonadError .

[turboMaCk]

Well this issue is not really about exposing exceptions via MonadError. The issue you've created is https://github.com/Holmusk/postgresql-simple-named/issues/27 but I intentionally created this one to avoid talking about general problem in terms of one specific solution.

find it unfortunate that this library is adding MonadError as separate route through which exceptions can be thrown

I agree with this. Especially since the errors thrown in MonadError are implementation errors (at least in most cases) one doesn't want to handle anyway. So api makes it easy to handle things you don't want to handle and sort of obscures things you might need to handle.

I think there might be some solutions that avoid issues with exposing more exceptions in MonadError. For instance:

• we can just ditch monad error and io lifting (the low tech solution) completely to avoid confusion
• we can ditch just MonadError/WithNamedError so it's clear everything is thrown in lifted io
• we can provide primitive function to catch specific io exception from the library
• we can provide function that would turn result of queries to Either err a where err is constructed based on thing passed into the function which will represent catches of handled io exceptions to some error type.

to name a few which immediately come to my head.

in the end I would like to be able to write something along (pseudo code):

executeNamed [sql| query |] [ param =? foo ]
  `catching` \IoExceptionThing -> NameTakenError
  `catching` \AnotherIOExceptionThing -> AliasTakenError

[nitinprakash96]

I wonder if https://github.com/haskellari/postgresql-simple/pull/108 is going to be any helpful for handling different errors arising out of postgres-simple? @turboMaCk