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

Allow to set a custom error handler in router configuration. #21

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Madeorsk
Copy link
Contributor

@Madeorsk Madeorsk commented Dec 18, 2024

Hi! This pull request provides something like not_found_handler but for errors. It also generalizes error response sending in Server: everything now go through the new handle_error which tries to call the error handler, and responds internal server error if the error handler fails. To generalize these calls and the implementation of the handler, I needed to initialize a context instance even before request parsing. This should be taken in account when implementing an error handler, as the request may not be completely initialized if an error happened while parsing it.

  • Add an option for a custom error handler and set a complete default one.
  • Change handler context to allow its initialization without a matched route.
  • Alphabetical order of HTTPError errors.

@mookums
Copy link
Owner

mookums commented Dec 21, 2024

I will take a look at this later today or tomorrow. I'm curious on if you were still planning on building out support for middleware? If yes, are you planning on keeping the error handler like this or doing a layered style?

@Madeorsk
Copy link
Contributor Author

Madeorsk commented Dec 21, 2024

I've implemented middlewares on my development branch, and didn't change how this error handling was made because I wanted to keep a "top-level" error handler which can catch request parsing errors. For "route specific" (or "routes group specific") error handling, middlewares will be required (and will allow to do that).

But my doubts were more about the not found handler. I think that "not found" is just a specific error that could be added to HTTPError, then be catched in middlewares / error handler. It's a bit harder than just setting a not found handler, but is also simplifying real-world applications: I built a test API with my current development branch, and if you ask for and ID which is non-existent, the API should return "not found", so I need to define a custom error handler which catches the not found error returned by the route handler and call the not found handler.

+ Add an option for a custom error handler and set a complete default one.
* Change handler context to allow its initialization without a matched route.
* Alphabetical order of HTTPError errors.
@Madeorsk Madeorsk force-pushed the comptime-router-errors branch from c165662 to d8a593a Compare December 21, 2024 12:45
@Madeorsk
Copy link
Contributor Author

I rebased my branch on the current main.

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