Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: better exceptions and logging and startup #31

Merged
merged 1 commit into from
Dec 21, 2023

Conversation

tiptenbrink
Copy link
Collaborator

I was taking a look at the exception logging and found that the stacktrace is not properly logged.

Ideally, we would not also show a lot of the unnecessary Uvicorn/Starlette/FastAPI stacktrace, but I did some exploration and it's not easy at all to remove this if you do want to have the stacktrace of your application. It's best we just live with it.

Throwing an AppError in the application will now log the error and lead to a 500 response.

Other improvements:

  • All queries are now performed with execute_catch (renamed form execute_catch_conn) to allow better handling of exceptions. I added a new type of DbError, ProgrammingError based on the SQLAlchemy exception type. This occurs for instance when a table is undefined.
  • I added some better error messages for common problems when running the backend, like an incorrect config, or an empty database (this uses the ProgrammingError) or if the database isn't on. It's mostly just heuristics, like I only added the ProgrammingError check for the JWK loading since in practice this is the first time we load something from the DB. Maybe good to replace that in the future with something more structured.
  • I added a new command to the dev.nu script, running nu dev.nu backend will now start the backend.
  • I renamed StoreError to StoreObjectError and ensured all errors in the store module inherit from a basic StoreError.
  • Connections are now terminated when recreating a database.
  • I added some additional documentation to some functions that return errors and made the language clearer on the startup lock.

Copy link
Contributor

@SenneDrent SenneDrent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@SenneDrent SenneDrent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@tiptenbrink tiptenbrink merged commit 0c6fd7e into main Dec 21, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants