search

rightfold / purescript-postgresql-client

https://pursuit.purescript.org/packages/purescript-postgresql-client
BSD 3-Clause "New" or "Revised" License
35 stars 20 forks source link

More fine-grained error handling #25

Closed akheron closed 5 years ago

akheron commented 5 years ago

It would be nice to be able to separate e.g. integrity errors from other errors like connection or SQL syntax errors.

In many cases, integrity errors are expected and recoverable, for example if there's a UNIQUE constraint on some column.

node-postgres's error objects contain a code property, which seems to contain a PostgreSQL error code (e.g. '23xxx' for integrity errors, '42601' for syntax error, etc.), or a human-readable errno ('ECONNREFUSED') in case of connection errors.

paluh commented 5 years ago

@akheron This for sure great idea to return "exceptions" in a more semantic manner and we probably should try to tackle it here at the FFI level of this library. I'm just not sure how to implement them and what are the potential consequences of this addition for the current really simple API.

Aff provides Error type which is not really extensible and is not meant to be used "for control-flow exceptions" (second answer by Nate here: https://github.com/slamdata/purescript-aff/issues/136) so we probably should handle (all or a subset of?) exceptions by returning Either from our "query functions".

Should we use ExceptT to wrap our Aff (Either e a) as a "library" monad? It seems that it can complicate a bit interaction with outside "application monad", but maybe it is probably just a question of single lmap...

Do we want to provide both: extended error handling API and simplified (current) version of functions for interaction with Postgres?

Regarding implementation details here is my proposition: Probably error representation should be just closed sum. We want to somehow pass its constructors to FFI. We want to compose them with Left so they would be "ready to use" on the FFI side I think. In such a case we should pass Right constructor there as well so FFI could wrap successful query result too.

I'm going to prototype something this week to simplify further discussion. Of course I'm really open for any PRs or suggestions!

What do you think?

akheron commented 5 years ago

I love libraries that don't have runtime errors. I introduced myself to FP with Elm, and there it's just impossible to have runtime errors in your programs. So I actually like it when I have to handle the errors each time, as it forces me to think about what to do if an error occurs.

I'm not sure about others, though :)

Should we use ExceptT to wrap our Aff (Either e a) as a "library" monad?

I'm not sure what you mean with this. Having ExceptT PostgreSQL.Error (Aff a) as the monad for all database querying functions? Does it provide some benefits instead of having just Aff (Either PostgreSQL.Error a)?

A closed sum as the error representation sounds good, and I guess passing all the constructors to JS is the only possible way to do it.

paluh commented 5 years ago

I love libraries that don't have runtime errors. I introduced myself to FP with Elm, and there it's just impossible to have runtime errors in your programs.

Yeah, that is really valuable property of a strongly typed FP (I really love that Purescript tracks functions partiality in its type too). As a side note I want to point out that we are facing another kind of problem here which sqltopurs is trying to solve. We are not able to verify at compile time that provided SQL is consistent with declared Query i o type. I'm not using sqltopurs currently (because of problems with Perl), but I think that we should explore this idea even further. I think that we can try to reimplement sqltopurs and build a command line tool in Purescript which can use output of purs --codegen corefn (we have already battle tested purescript-corefn library for this purpose) and validate all SQL string literals with queries (which probably can be found by type Database.Postgresql.Query) against postgresql database like it is done in sqltopurs currently...

Sorry for digression... maybe I should just open a separate issue and describe this idea there ;-)

Does it provide some benefits instead of having just Aff (Either PostgreSQL.Error a)?

Maybe you are right. There is no benefit by wrapping it in ExceptT in the current state of affair. Users can easily wrap our Aff in ExceptT if they like to.