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

Error handler middleware #156

Closed
wants to merge 1 commit into from

Conversation

tmattio
Copy link
Collaborator

@tmattio tmattio commented Jun 2, 2020

This provides error handler middlewares for JSON and HTML.

There are mainly three reasons I implemented this as a middleware.

  • There was already a default HTML page for the error 404, defined in Opium.App. So this middleware can be seen as a generalization of this error page to every error code.

  • The error handler defined in Server_connection will only work when exceptions are raised.
    The exception does not contain the error code, and reporting errors other than exceptions are reserved to Httpaf internal uses (functions are not exposed)
    so if users want to rely on the Server_connection error handler, they are limited to using the internal server error, which seems pretty limited.

    I think most of the time, users will return an HTTP response with an error code instead of raising an exception. If there is no error handler, they will have to write their own.
    Most applications will end up having to do this at some point, but following the design goal "A programmer should be instantly productive when starting out.", I thought it would be nice to provide good defaults.

  • A lot of applications have different types of entry points. It is common to provide JSON endpoints as well as HTML endpoints in the same application.
    The error handlers will be different depending on the content type of the endpoint, and a middleware provides this flexibility as it can be applied to specific routes.

I also provided a way to easily override the error handling of specific status code with #96 in mind. One can override an error with the following syntax:

let custom_handler = function
  | `Forbidden -> Some (Response.of_string "Denied!")
  | _ -> None
;;

let error_handler = Middleware.error_handler ~custom_handler ()

which, IMHO, is a bit more user friendly than implementing a custom Httpaf error handler.

Error handling is an important part of a web framework and I think it's crucial that we find a good balance between convention, productivity, and flexibility. So I'm happy to experiment on other solutions to provide good defaults if the middleware solution seems weird to you.

@tmattio tmattio mentioned this pull request Jun 2, 2020
2 tasks
@tmattio
Copy link
Collaborator Author

tmattio commented Jun 5, 2020

Closing this as it seems there are better ways to manage errors than a middleware.

@tmattio tmattio closed this Jun 5, 2020
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.

1 participant