Database error text not being shown in admin UI

[ethagnawl]

I'm seeing some (potentially) unexpected behavior when encountering database-level input validation errors.

For example, if I have a table (app_user) whose name column has a uniqueness constraint on it, an admin entering a duplicate name is met with a 500 HTTP response whose "Internal Server Error" body is displayed to the user upon submitting the form. This isn't very helpful to a non-technical user.

Is this the expected behavior and the burden is on the implementer to handle the form validation and error messaging or should the error message returned to the UI contain the contents of the database's error message? From looking through endpoints.py, I think it should be the latter (400 response with {"message": ...} body) but I'm not certain.

[dantownsend]

Unique constraints aren't handled well currently - it should return a proper error message, as you say.

Most of the Piccolo Admin backend is built upon a class called PiccoloCRUD which maps HTTP methods to SQL queries. We rely heavily on Pydantic for input validation. But Pydantic can't validate unique constraints - we need to rely on the database. This is the relevant method on PiccoloCRUD.

We need to catch the database errors, and returns something more useful.

[sinisaos]

We can use generic exception to catch database errors on post_single, put_single and patch_single in Piccolo API crud endpoints like this:

except Exception as e:
    return Response(
        f"Unable to save the resource: {e}", status_code=400
    )

Result is

or in Postgres with biggest alert box

[dantownsend]

@sinisaos Thanks, that could work.

Ideally we would be able to just catch certain exceptions, for example unique errors. There's a PR open in Piccolo, but I haven't finished it yet. The idea is it will map the asyncpg or aiosqlite exceptions into generic Piccolo exceptions (for example UniqueException). For now though, this might be our best option.

[sinisaos]

@dantownsend Like you said that would be ideal because they would have the same error message for different db drivers. Once you finish with Piccolo PR you can mention me in the comments and I can do PR in Piccolo API and Piccolo Admin to implement this feature. Something like this

from piccolo.engine.exceptions import UniqueConstraintError # or different name of exception
...
except UniqueConstraintError as exception:
    return Response(
        f"Unable to save the resource: {exception}", status_code=400
    )
...

[dantownsend]

@sinisaos Cool, thanks - that looks good.

If I don't manage to get that PR closed soon, we'll go with your original solution.

[ethagnawl]

Thanks for the prompt response, @sinisaos and @dantownsend! Any of the above would make for a much better UX.

[dantownsend]

Sorry, progress on this stalled. Going with something similar to @sinisaos solution (https://github.com/piccolo-orm/piccolo_api/pull/187). It will now show a proper error message if a unique constraint fails rather than just a 500.

[dantownsend]

In piccolo_api>=0.47.0 this should be fixed.

I'll create some follow up tasks, as there's still more we can do with error messages in Piccolo Admin.