Best practices on handling DB errors independent of DB used

[rahulsabbineni]

I've been prototyping an API using starlette with SQLAlchemy and had a question about how to handle DB errors with the databases module.

# SQLAlchemy table definition
class Users(Base):
    __tablename__ = "users"
    id = Column(Integer, primary_key=True)
    username = Column(String(length=20), unique=True)
    password_hash = Column(String(length=80))

# Endpoint
@validate_schema(CREATE_USER_SCHEMA)
async def create_user(request):
    data = await request.json()
    query = Users.__table__.insert().values(username=data["username"], password_hash=hash_password(data["password"]))
    try:
        await database.execute(query)
    except exc.IntegrityError:
        return JSONResponse({"message": "User already exists."})
    return JSONResponse({"message": "Created user."})

When I try to create a user with a username that already exists, violating the UNIQUE constraint specified in the table definition, I receive a sqlite3.IntegrityError: UNIQUE constraint failed: users.username (venv/lib/python3.8/site-packages/aiosqlite/core.py:153: IntegrityError). This is not caught by the code above, which uses sqlalchemy.exc.IntegrityError.

I like to run tests in SQLite, and use Postgres in production; in other APIs I've built (with plain SQLAlchemy), sqlalchemy.exc generally does the trick regardless of the DB backend used.

Q: Is there a good way to catch errors independent of the DB used? For now, I'm catching both the SQLite and Postgres exceptions manually, but I was wondering what best practices for this case would look like using the databases module.

Thanks for creating starlette and databases, and sorry in advance if this is a duplicate.

[tomchristie]

Right now there's not a good answer here, no.
It would make sense for databases to wrap up any known exception types inside it's own top-level wrapper.

[gvbgduh]

I wonder if it persists with aiomysql and aiopg, but it's probably due to the fact the sqlalchemy isn't aware of these drivers hence about the exception classes.

I would be nice indeed to wrap it as the databases exception.

1. We can try to catch base exceptions or those that are known per backend implementation and raise the wrapped one.
2. Or raise the one from sqlalchemy as far as the mapping between exceptions.

But I have mixed feelings about the second option.

Update: or actually raise the sqlalchemy exception based on the context of the databases generic exception that'll be raised within the particular backend implementations.

[jruizaranguren]

Given that every backend already depends on sqlalchemy, it make sense that exceptions raised by Connection derived class have a common error interface. If decoupling from sqlalchemy is not in the roadmap, then using sqlalchemy errors seems a sensible approach.

@tomchristie, Where is the "top-level" wrapper you mentioned in code?

[divbzero]

@jruizaranguren As far as I can tell, a “top-level” wrapper for errors does not exist in the code right now. Tom was simply stating that it would make sense to implement this wrapper in the future.

[tomchristie]

@divbzero Correct.

[skewty]

Any progress made or milestone set for this issue?

This issue is likely affecting many people as one exception is seen in production and another is seen in testing.

Is the desired path forward to create a databases.DatabaseError or use a sqlalchemy.DatabaseError? (perhaps this hasn't been decided yet)

My vote goes more towards an import from databases even if I am just importing an alias to an sqlalchemy exception.

[willsmith28]

I came across this issue trying to write tests for a personal project. I created a fork where I attempt to catch the 8 main python DB API exceptions in the backends and re-raise them as the equivalent SQLAlchemy exception. Here is a diff against this repo for convenience.

Unfortunately asyncpg doesn't implement the standard DB API exceptions one-to-one so I have made my best attempt at mapping the most generic exceptions, but there might be something I missed.

If there is any interest in this code I would be happy to make a pull request or any recommendations about improvements. There is an included test that shows the correct IntegrityError is being raised but I am unsure of how to construct test cases for the other except clauses to raise the test coverage.

If this isn't the direction you want to take then this can just serve as a proposal. Thank you for the work you do!

[skewty]

@willsmith28 I had a look through the diff in your fork and I would welcome such a merge. (I am not a member of this team though) It would be a breaking change though which might hold it up. IMO it should be okay to break stuff like this because this project still has an alpha status. Projects using this would hopefully be in active development stages and the issue caused would be easy to fix with a more robust end result that is easier to test.

[Feijo]

Any progress on this? I'm having the same problem with Production vs Unit Tests database throwing different errors.

[mreschke]

I have the same issue. I have no choice but to just catch all errors, or in the case of catching IntegrityError I have to SELECT first to see if already exists. Definitely need unified errors across these backends.

[ioparaskev]

I also have the same issue basically because this forces you to use the same database type or have all database drivers install and expect all possible exceptions