launchbadge / sqlx

🧰 The Rust SQL Toolkit. An async, pure Rust SQL crate featuring compile-time checked queries without a DSL. Supports PostgreSQL, MySQL, and SQLite.
Apache License 2.0
13.47k stars 1.28k forks source link

Feat: better database error support #2102

Closed saiintbrisson closed 1 year ago

saiintbrisson commented 2 years ago

Is your feature request related to a problem? Please describe. I've been adding support for SQLx to an error handling crate and I got stuck when processing database errors because the crate aims to be agnostic, and I'd have to manually compare SQLSTATEs to check if the error is a unique violation, check violation, etc. It'd be nice if we had some solution akin to Diesel's DatabaseErrorKind.

While it's possible to do this via DatabaseError::kind, this handling is something that I've had to do multiple times in the past, and removing the burden off of the user would be nice.

Describe the solution you'd like The solution boils down to mapping the SQLSTATE returned by the DB to an enum variant. We'd need constants with the SQLSTATEs for the drivers already supported (PostgreSQL, MySQL, SQLite, and MSSQL). The function that would perform this mapping would be implemented via DatabaseError and returns an Option<Enum> (drivers may not support the same SQLSTATEs):

enum ErrorKind {
  UniqueViolation,
}

trait DatabaseError {
  // ...
  fn kind(&self) -> Option<ErrorKind>;
}

impl DatabaseError for PgDatabaseError {
  // ...
  fn kind(&self) -> Option<ErrorKind> {
    match self.code()? {
      "23505" => Some(ErrorKind::UniqueViolation),
      _ => None,
    }
  }
}

Describe alternatives you've considered Create a feature for each driver SQLx provides support for and manually comparing the error codes for each database on the crate I'm working with.

I'm keen on working on this, guidance is welcome!

saiintbrisson commented 2 years ago

There is also the table function which is local to the PostgreSQL back-end, and it'd be great to be able to access it in a generic way, but I'm asking for too much now 🥷

abonander commented 2 years ago

As context for how we handle this in our applications (as SQLx is a core component of many Launchbadge projects), we usually have a little extension trait over Result like this: https://github.com/launchbadge/realworld-axum-sqlx/blob/main/src/http/error.rs#L173

It's much easier to make this ergonomic in a non-generic context like an application where the final error type is known ahead of time. And it only handles constraint violations because that's really the only kind of error we care to recover from.

That interface wouldn't adapt to MySQL very well, however, as the error response doesn't include the constraint name as a separate field. I suppose it wouldn't be too difficult to parse it from the error message, although in this case I would reverse it and do a substring search for the constraint name in the error message.

saiintbrisson commented 2 years ago

That's how I do it as well, and it works fine when I know which driver is being used. I've parsed the error message in the past, but I can't guarantee it will have the same message features across all back-ends. Also, the constraint name isn't required (in my case at least), only the SQLSTATE kind.

The problem I'm aiming to tackle is when we add support for SQLx in agnostic code (for instance, this is the crate I'm working on). In my case, I'll have to add some logic that accounts for the possible drivers the code may be using.

abonander commented 2 years ago

I could see possibly introducing a kind() method and enum, though I think there should also be some convenience methods like:

fn is_constraint_violation(&self) -> bool;

fn is_unique_violation(&self) -> bool;

fn is_check_violation(&self) -> bool;

added to the DatabaseError trait.

saiintbrisson commented 2 years ago

I'll sketch out a rough idea in a PR and mention you for ideas. Are you fine with this?

abonander commented 2 years ago

Yeah, feel free.

saiintbrisson commented 1 year ago

2109 was merged and will be available in the next minor release (0.7).