ocaml-caqti icon indicating copy to clipboard operation
ocaml-caqti copied to clipboard

More detailed error types

Open aantron opened this issue 3 years ago • 7 comments

For example, upon a UNIQUE constraint error, Caqti exposes a Caqti_error.t which looks like this after show:

Response from <sqlite3:db.sqlite> failed: CONSTRAINT. Query: <snip>

The actual value is

`Response_failed _

The msg field of _ could be either...

  • Caqti_error.Msg, which carries a string, and is therefore not useful for pattern matching. The string would have to be parsed instead.
  • Caqti_driver_sqlite3.Rc carrying, presumably, Sqlite3.CONSTRAINT. This would be useful, but:
    • Caqti_driver_sqlite3.Rc is not exposed AFAICT.
    • We actually want something portable to all the database drivers, not just SQLite3.
    • It doesn't specify that the constraint violation was of a UNIQUE constraint.

Is there a way to get more detail in the error types? Since at the moment, all we can reasonably match on is `Response_failed _, all we essentially can know is that "some error occurred."

On the subject of detail on CONSTRAINT, by contrast, @thangngoc89 reports in https://github.com/aantron/dream/issues/86 that sqlite3 cli gives errors like

Error: UNIQUE constraint failed: user.email

Ideally, we would have a detailed database-agnostic error type, and translate database-specific errors in a thorough way into it.

aantron avatar Jul 04 '21 14:07 aantron

Yes, the database specific errors are only exposed as strings today. It would be possible to reveal them, but the user would have to link against each specific driver. So, I agree a better solution would be to translate the errors into an agnostic type. I also think it would be useful to classify the errors based on the kind of issue, the recipient of the error, and whether something can be done about it (like retry in case of timeout or connection failure).

I only had a brief look this far and considering the number of errors for MariaDB and PostgreSQL it looks like a major task to classify and unite them. In comparison Sqlite3 seems to have few enumerated conditions, maybe even insufficient for what you suggest? But focusing on the former two, there is a common "SQLSTATE". It's not exposed in the PostgreSQL API, but the documentation suggests it present, so we can make a PR for that. In MariaDB the error is represented as int * string. I wonder if that string is the SQLSTATE, otherwise there is at least a translation. Many of the conditions, however, translates to the fall-back HY000. So, I am not sure if we can rely on the SQLSTATE classification.

On the side, I am a bit suspicious of using errors to detect unique constraint or other violations as a normal control flow. Though, maybe there isn't an easy alternative which works across database systems, except paying the overhead of querying up-front within the same transaction.

paurkedal avatar Jul 06 '21 16:07 paurkedal

If you look at SQLITE3 documentation, you can see that error codes are documented clearly with subclasses. I don’t think sqlite3 driver exposed the subclasses

https://www.sqlite.org/rescode.html#constraint_unique

thangngoc89 avatar Jul 06 '21 17:07 thangngoc89

On the side, I am a bit suspicious of using errors to detect unique constraint or other violations as a normal control flow.

Isn't that actually the accepted way of doing this? You set a unique constraint and the database keeps it. Getting a unique violation tells you exactly what you need to know

tcoopman avatar Jul 06 '21 18:07 tcoopman

To outline the task:

  • [x] Extend PostgreSQL bindings to expose the SQLSTATE, possibly supplemented by a variant. Update: This is already implemented in the error_field method.
  • [ ] Extend the MariaDB bindings to expose the SQLSTATE. We have the error code from the result. andrenth/ocaml-mariadb#42. Update: While it may be nice to have, the chosen approach does not rely on SQLSTATE.
  • [ ] Extend the Sqlite3 bindings to expose the refined error codes. mmottl/sqlite3-ocaml#49.
  • [x] Decide on how the errors will be exposed in Caqti and whether and how to try to unify them.
  • [ ] Implement it in Caqti. Update: Missing extended error codes for sqlite.

paurkedal avatar Jul 17 '21 14:07 paurkedal

My rough thoughts on the Caqti interface is to expose them as a field in query_error, or possibly accessible only by functions so that the representation can depend on the driver. What we can easily expose is an optional SQLSTATE in raw form and maybe an optional driver-dependent integer error code as a limited-use fall-back. As for the less easy but much nicer API, I consider making a grant variant containing all cases for all drivers and then creating variant aliases of groups of cases which one is interested in matching. What I don't like about this approach is that it exposes too much of the specifics of each database system, but on the other hand I am not sure how feasible it will be to map them into a really unified set of errors, given that many of the errors are really specific to the database system.

paurkedal avatar Jul 17 '21 14:07 paurkedal

On the side, I am a bit suspicious of using errors to detect unique constraint or other violations as a normal control flow.

Isn't that actually the accepted way of doing this? You set a unique constraint and the database keeps it. Getting a unique > violation tells you exactly what you need to know

@tcoopman It probably is. I usually try to avoid triggering errors, which has been useful in cases when I want to run a while series of updates in a transaction, since an error would abort the transaction. But that sometimes involves additional queries, within the transaction, or the use of not universally available ON CONFLICT clauses. And in case of concurrent operation, the transaction could fail, in which case a more detailed error will also be needed to handle it gracefully.

paurkedal avatar Jul 17 '21 14:07 paurkedal

I'm reconsidering my approach here. PostgreSQL provides SQLSTATE at a useful level, but MariaDB maps many errors to too generic classes, esp. to a fallback HY000, so I think we would need to redo the mapping to make it useful. Sqlite3 has no SQLSTATE support. But, creating a mapping for MariaDB and Sqlite3 turned out to be a tiring process with limited success. I pushed some limited mapping to https://github.com/paurkedal/ocaml-caqti/tree/error-sqlstate in case anyone wants to see a meagre start.

My thinking now is that if we're not going for anything close to the full SQLSTATE, we may as well make something which is more ergonomic for OCaml code. So, I made about the same mapping to a variant type, https://github.com/paurkedal/ocaml-caqti/tree/error-cause. Example:

(match Caqti_error.cause error with
 | `Foreign_key_violation -> handle_foreign_key_violation ()
 | #integrity_constraint_violation -> handle_other_constraint_violation ()
 | _ -> handle_other_error ())

We don't have the extended sqlite3 error codes yet, so any constraint violation ends up on the #integrity_constraint_violation branch. I am also unsure about some of the MariaDB error codes, since the manual only states the error messages and not the precise circumstances when the error occurs.

paurkedal avatar Feb 17 '22 16:02 paurkedal