awtkns / fastapi-crudrouter

A dynamic FastAPI router that automatically creates CRUD routes for your models
https://fastapi-crudrouter.awtkns.com
MIT License
1.42k stars 157 forks source link

Non-informative error on creation #154

Open ghilesmeddour opened 2 years ago

ghilesmeddour commented 2 years ago

Hi,

HTTPException(422, "Key already exists") if raised for every IntegrityError on creation. This can be confusing. I was wondering whether it would be better to use CRUDGenerator _raise method to have a more specific error message.

Ghiles

xkortex commented 2 years ago

I just got bit by this. I was wracking my brain trying to figure out how I could be getting Key already exists errors with a completely empty database. So I debugged the _create function and found out the error it was eating was actually sqlalchemy.exc.IntegrityError: (psycopg2.errors.NotNullViolation) null value in column "created_on" violates not-null constraint (I had forgot to set server_default=sa.sql.func.now() for the column).

I agree, doing a self._raise(e) just like the _update method would be an easy fix here.

rodg commented 1 year ago

Yeah I just ran into this as well, going to +1 having the option to just raise the errors up. I get the point mentioned in other issues (like #66 ), but at the same time I find this package useful for stuff that never sees the light of day (quick tests, internal tools) so having a bonus option on the create route to have it just print the real error would be very helpful. If we could just toggle a boolean to have the "less safe but more useful" errors that would be a good change.

Something more ideal might be having the ability to pass in an error handling function to the router? I don't think it'd be 100% necessary but nice error handling customization would be cool IMO.

rodg commented 1 year ago

In the mean time this little hack seems to work (not exhaustively tested):

from typing import Callable, Any

from fastapi_crudrouter import SQLAlchemyCRUDRouter as _SQLACRUDRouter
from sqlalchemy.orm import Session
from sqlalchemy.ext.declarative import DeclarativeMeta as Model
from sqlalchemy.exc import IntegrityError

from fastapi import Depends

CALLABLE = Callable[..., Model]

class SQLAlchemyCRUDRouter(_SQLACRUDRouter):
    def _create(self, *args: Any, **kwargs: Any) -> CALLABLE:
        def route(
            model: self.create_schema,  # type: ignore
            db: Session = Depends(self.db_func),
        ) -> Model:
            try:
                db_model: Model = self.db_model(**model.dict())
                db.add(db_model)
                db.commit()
                db.refresh(db_model)
                return db_model
            except IntegrityError as e:
                db.rollback()
                self._raise(e)

        return route