pimoroni / phew

MIT License
204 stars 44 forks source link

Fixing behavior issue when route raising Exception #21

Open staytime opened 2 years ago

staytime commented 2 years ago

Background

Consider following code

@server.route("/", methods=("GET", ))
def landing_page(request):
  # some error
  foo = 0
  foo.set()
  return "OK?"

Presently _handle_request will return not thing, Leading client hang until timeout.

Change

This pull request change are following

  1. Fixing undesired behavior by return a error page.
  2. Adding error message.
  3. Adding decorator "errorhandler" for customizing in user code.
ccrighton commented 4 months ago

@staytime @Gadgetoid This PR is heading in the right direction. However, Phew is a framework that is used by other applications. The behaviour in this PR while an improvement does not give control into the hands of the application developer.

Note that I'ved used my branch of Phew ccrighton/phew in the examples below.

Ideally, we would:

1) Provide default unhandled exception handler that returns a 500 Internal Server Error (much like this PR)

2) Provide a decorator that allows the developer to create their own handlers for different exceptions and provide their own default exception handler. This PR provides the decorator but does not support handling different exceptions with different handlers. It's possible that we just start with the generic case and expand it. However, it would be better to support the parameter in the errorhandler from the outset even if we just do something like @phew_app.errorhandler(Exception)

phew_app = server.Phew()

@phew_app.errorhandler(HTTPException)
def handle_exception(e):
   return render_template("500_generic.html", e=e), 500

3) Support API errors as JSON. Could also be an extension to initial capability.

4) Support a set of exception classes that can be raised in route handlers that will automatically generate a response object.

Again could be done after initial implementation without breaking anything.

class ImATeapot(HTTPException):
    code = 418
    description = 'I'm a teapot'

@phew_app.route("/", methods=["GET"])
def index(request)
    ...
    raise ImATeapot("an error occured")    

5) Documentation and examples. Very important we add this with the initial implementation.

6) Some other stuff that will come out in the wash when implementation is done :-).

It would be worth considering adding an abort(code) method that raises an HTTPException based on the given status code. This is supported by Flask. They also support code or exception e.g. errorhandler(404) and errorhandler(NotFound) are the same.