Skip to content
This repository

Reraising an HTTP exception from an error handler isn't the same as not trapping. #552

Closed
adrianratnapala opened this Issue · 3 comments

3 participants

adrianratnapala Simon Sapin Armin Ronacher
adrianratnapala

I am not sure if this is a bug, because I don't know what the expected behaviour is. I am reporting it just in case (there should also be a mailing list message from 2012-07-15 about this topic).

behaviour (that might be) expected:

If any exception is trapped and passed to a Flask error handler, but that handler decides to re-raise the exception, the effect should be the same as if no handler had been registered. For example, if an HTTP error handler is registered by their numerical code but then re-raises the exception, it should result in Flask's default HTTP error page for that code, even if TESTING=True.

behaviour observed:

If an HTTP error handler is registered via its numerical code re-raises an exception, the exception is then handled is if it were a non-HTTP exception. If TESTING=True, this results in a stack trace. If TESTING=False, the client gets a "500 Internal Server Error" regardless of what the original exception was.

Simon Sapin

In other words, if an error handler raises an exceeption, Flask should call another error handler? If so, we should be careful not to get stuck in a loop.

adrianratnapala

There should be no risk as long as the implementation doesn't call the original user-installed handler. In fact I don't even know if what I am calling for "calls another error handler", I am saying the result of re-raising an exception within the handler, should be the same as not having an exception handler at all.

Whatever gets called will be the default Flask/Werkzeug behaviour, which should not alow HTTPException (or any thing else) to bubble out. I can't remember exactly now, but I think the patch to Flask.trap_http_exception I sent to the mailing list did this right.


diff --git a/flask/app.py b/flask/app.py
index d30d380..c480814 100644
--- a/flask/app.py
+++ b/flask/app.py
@@ -1210,7 +1210,11 @@ class Flask(_PackageBoundObject):
             handler = self.error_handler_spec[None].get(e.code)
         if handler is None:
             return e
-        return handler(e)
+        try: handler(e)
+        except HTTPException, x:
+                if self.config.get('TRAP_HTTP_EXCEPTIONS') :
+                        raise
+                return x

     def trap_http_exception(self, e):
         """Checks if an HTTP exception should be trapped or not.  By default

(I can put this patch up as a proper pull request, if people actually think issue might be bug).

Both the patch and the context look odd, because they can return exceptions as normal values. I assumed that the caller just turns any return value into a string and returns that string to the client and the way it turns exceptions into strings is by formatting a nice error page. This is what already happens if handler is None, I am just extending it to the case where the handler raises an HTTPException.

Armin Ronacher
Owner

The terminology might be weird but the current behavior is expected. What's the usecase you have?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.