Implications of running statements directly in IO

[torgeirsh]

Since "Session a" is isomorphic to "Connection -> IO (Either QueryError a)", it's relatively simple to integrate it with a custom application monad when using plain Hasql. With Hasql.Pool things get a bit more complicated, because the "use" function says it relies on the QueryError to determine if a connection is lost. What will happen if an application handles the QueryError in a different way, e.g. by logging it, and then "use" is only called when there's no QueryError (i.e. the return value of the IO action inside the Session passed to "use" is always "Right a")? Would a temporary connection problem then poison the pool and eventually make it impossible for the application to connect, or is that not an issue?

[nikita-volkov]

That's an interesting problem you're highlighting here. It raises questions in the direction of the MonadError instance on Session. Possibly there shouldn't be one 🤔, because it allows the user to handle the error within the session. The usefulness of that is questionable and this issue highlights the problems.

Any way, if your session fails with QueryError but you restore from the error inside the session it will indeed cause the connection to stay in the pool.

One thing to note here is that the latest release now spawns a maximal connection liftetime option. That one will at least cause all connections to get freed up after the period that you specify, so your pool won't stay poisoned forever. Still it's best to avoid handling QueryError within Session I guess.

[torgeirsh]

Somewhat unintuitively, it's not the MonadError instance that allows for bypassing the error handling, but the MonadReader instance. The "run" function converts a session to "Connection -> IO (Either QueryError a)", so inside the session passed to hasql-pool's "use" function, you only need a Connection (and liftIO) in order to end up with "Session (Either QueryError a)", which for "use" looks like a success, regardless of what the Either is.

I'm not sure if there's a quick solution that fits within the current design, but since this only affects hasql-pool, and not plain hasql, I hope the MonadReader instance can remain, as it's very useful for interoperability with things like unliftio and effect systems.

[nikita-volkov]

You're right. The MonadReader instance can also be seen at cause here. Actually both seem questionable to me. But regardless I understand that many solutions are reliant on them now and I agree that since "hasql-pool" is merely one of the dependendants of the "hasql" library and is not an integral part of it, it wouldn't be correct for it to affect the design of "hasql". However both instances have long been seen questionable even without the issue at hand and I do see the current issue as one of the proofs of that.

Currently I see 2 ways to address the issue:

1. Add a specialized Session in hasql-pool, which would lack the mentioned instances.
2. Extend the documentation on use to notify the user about the effects of handling the QueryError.

I think for now it would be best to go with the second option.