Registering a handler for HTTPException has no effect #941

mdeous · 2014-01-03T18:58:20Z

When registering a handler for werkzeug.exceptions.HTTPException, it has no effect when an HTTP error is raised.

Assume the following handler:

@app.errorhandler(HTTPException)
def http_err_handler(error):
    response = jsonify({
        "success": False, 
        "message": error.name
    })
    response.status_code = error.code
    return response

When requesting a page for which no route exists, a JSON response should be returned by the error handler, but instead, the usual Flask-generated HTTP error page is returned.

On the other hand, if the error handler is defined to handle a specific error code (by passing the error code to the app.errorhandler decorator), the exception is trapped and the JSON message returned.

As wekzeug.exceptions.HTTPException is the class raised internally by the abort() function, why isn't it possible to create a "catch-all" handler like this? Am I missing something?

The text was updated successfully, but these errors were encountered:

untitaker · 2014-01-06T21:02:28Z

Long story short: It is a bug, it's a flaw in the design, it's easy to work around as a user, so it's rather low priority to fix. :S

mdeous · 2014-01-07T01:10:19Z

Do you know where the bug exactly lies? If this is an already known bug, with some informations about it maybe I could help by trying to fix it by myself and then submit a pull-request.

untitaker · 2014-01-08T10:53:09Z

I think it would require rewriting most of the error handling system for Flask, the code for this is in flask/app.py

Originally i intended to rewrite the error handling system to register an error handler for each subclass of HTTPException on initialization of the Flask object, removing the distinction between HTTP exceptions and other exception types. However, with the current behavior of preferring the first-registered handler when an error occurs, doing this without compromises would require changing this to preferring the last-registered handler, a backwards-incompatible change. The following is a very minimal fix to only make Flask prioritize the user's error handlers over others. I am not sure which performance implications this has. Maybe a major rewrite of the error handling system is still desirable (with more complex prioritization of errorhandlers -- something based on "distance" in inheritance tree?), but at the moment i don't see a reason to do so, given that the current system is primitive enough to be understandable, while still usable.

untitaker · 2014-01-14T20:42:23Z

#952 is a possible solution to this.

Originally i intended to rewrite the error handling system to register an error handler for each subclass of HTTPException on initialization of the Flask object, removing the distinction between HTTP exceptions and other exception types. However, with the current behavior of preferring the first-registered handler when an error occurs, doing this without compromises would require changing this to preferring the last-registered handler, a backwards-incompatible change. The following is a very minimal fix to only make Flask prioritize the user's error handlers over others. I am not sure which performance implications this has. Maybe a major rewrite of the error handling system is still desirable (with more complex prioritization of errorhandlers -- something based on "distance" in inheritance tree?), but at the moment i don't see a reason to do so, given that the current system is primitive enough to be understandable, while still usable.

untitaker · 2014-10-19T18:14:58Z

Fixed by #839

Bekt · 2015-07-05T18:04:36Z

#839 doesn't fix the issue OP posted -- it only handles subclasses of HTTPException.

untitaker · 2015-07-05T18:08:07Z

Could you provide a testcase that fails?

Bekt · 2015-07-05T18:45:16Z

import unittest

from werkzeug.exceptions import HTTPException, NotFound

import flask


class ErrorHandlerTest(unittest.TestCase):

    def setUp(self):
        app = flask.Flask(__name__)

        @app.errorhandler(HTTPException)
        def http_exception(e):
            return 'generic', 500

        @app.errorhandler(NotFound)
        def notfound_exception(e):
            return 'not found', 404

        @app.route('/error')
        def error():
            flask.abort(500)

        self.app = app

    def test_notfound_handler(self):
        rv = self.app.test_client().get('/')
        self.assertEqual(rv.status_code, 404)
        self.assertEqual(rv.data, 'not found')

    def test_http_handler(self):
        rv = self.app.test_client().get('/error')
        self.assertEqual(rv.status_code, 500)
        self.assertEqual(rv.data, 'generic')


if __name__ == '__main__':
    unittest.main()

$ venv/bin/python app.py
F.
======================================================================
FAIL: test_http_handler (__main__.ErrorHandlerTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "app.py", line 35, in test_http_handler
    self.assertEqual(rv.data, 'generic')
AssertionError: '<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">\n<title>500 ...' != 'generic'

----------------------------------------------------------------------
Ran 2 tests in 0.021s

FAILED (failures=1)

Code 500 is an InternalServerError (subclass of HTTPException). Since there is no specific handler for it (unlike NotFound), one would hope it would be handled by @app.errorhandler(HTTPException)

untitaker · 2015-07-05T19:38:14Z

Ahh, I think this is because HTTPException doesn't have a HTTP code, but the error handling logic tries to find an errorhandler with the HTTP code.

cc @flying-sheep

Bekt · 2015-07-05T19:47:51Z

Has anybody reviewed/tested #1383? I was not very sure by looking at the changes.

untitaker · 2015-07-05T19:52:29Z

Yeah this slipped through the cracks!

I wonder whether we shouldn't remove the code/no-code distinction because of this bug though.

flying-sheep · 2015-07-06T09:09:10Z

right, i didn’t think of this, sorry!

so the legal classes to register handlers for: HTTPException, the finite set of all subclasses of it (synonymous with error codes), and subclasses of subclasses (e.g. class ForbiddenBecauseNotRegistered(Forbidden): ...

and the legal instances of exceptions to raise are instances of subclasses of HTTPException but not direct instances of HTTPException.

this means the following assumptions hold:

if isinstance(e_instance, HTTPException):
    assert e_instance.code is not None
if issubclass(e_class, HTTPException):
    assert e_class is HTTPException or e_class.code is not None

we should add a way to add a handler for HTTPExceptions without allowing users to raise an instance of HTTPException.

flying-sheep · 2015-07-06T09:10:39Z

@Bekt that PR should be closed, it was written against the old broken behavior that I replaced.

Bekt · 2015-07-06T19:41:53Z

For those who simply want to override the default behavior of the default HTTPException subclasses, you can do something like this:

import flask
from werkzeug.exceptions import default_exceptions

def create_app():
    app = flask.Flask(__name__)
    ....
    for code, ex in default_exceptions:
        app.errorhandler(code)(_handle_http_exception)

def _handle_http_exception(error):
    if flask.request.is_json:
        return jsonify({
            'status_code': error.code,
            'message': str(error),
            'description': error.description
        }), error.code
    raise error.get_response()

cerickson · 2017-05-22T18:41:16Z

#2144 seems to solve this issue

Error handlers are now returned in order of blueprint:code, app:code, blueprint:HTTPException, app:HTTPException, None Corresponding tests also added. Ref issue pallets#941, pr pallets#1383, pallets#2082, pallets#2144

jeffwidman · 2017-05-23T19:55:33Z

Resolved by #2314

antgel · 2019-11-09T10:57:45Z

@Bekt I'm a bit late to the party, hope you're around... I'm trying to get flask to return JSON errors when there are exceptions. Your code works great except for raise error.get_response(). According to werkzeug docs:

If you call get_response() on an exception you will get back a regular BaseResponse object, even if you are using a custom subclass.

This results in raise error.get_response() returning TypeError: exceptions must derive from BaseException. Did I miss something? Is there a better way of doing this in 2019 as opposed to 2015.

Bekt · 2019-11-12T03:28:38Z

@antgel i would be lying if i said i remember even remotely what this thread is about... i miss my flask days.

flying-sheep · 2019-11-12T09:16:28Z

I mean, the error message is pretty clear: error.get_response returns a Response, not an Exception, so you probably just have to replace raise with return

mononobi · 2020-10-04T15:18:04Z

Long story short: It is a bug, it's a flaw in the design, it's easy to work around as a user, so it's rather low priority to fix. :S

Hi

So late, but it is NOT a bug, this is a design decision. HTTPException is the base type for all http exceptions in werkzeug.
and werkzeug or flask themselve, never raise an instance of HTTPException, instead they raise different subclasses of it.
so this is implemented this way to enforce that HTTPException is the base type, and should not be raised directly. but you
could register an error handler for HTTPException and handle all subclasses of it without any problem. the only thing
that you have to do is NOT to raise a direct instance of HTTPException, instead raise its subclasses.

untitaker closed this as completed Oct 19, 2014

untitaker reopened this Jul 5, 2015

untitaker added blocker bug labels Jul 5, 2015

hachreak mentioned this issue Jul 5, 2016

templates: fix jinja format inveniosoftware/invenio-deposit#89

Merged

oliwang mentioned this issue Sep 22, 2016

Issue list team-Slytherin-CSE2410/flask#1

Open

untitaker mentioned this issue Feb 28, 2017

The file/path provided (flaskr) does not appear to exist. #2058

Closed

untitaker added the ready label Feb 28, 2017

ngaya-ll mentioned this issue May 3, 2017

Catch-all errorhandler does not catch BadRequestKeyError #2268

Closed

cerickson mentioned this issue May 22, 2017

Added support for generic HTTPException handlers on app and blueprints #2304

Closed

cerickson mentioned this issue May 23, 2017

Added support for generic HTTPException handlers on app and blueprints #2314

Merged

jeffwidman closed this as completed May 23, 2017

thewilkybarkid mentioned this issue Aug 30, 2017

Make errors JSON responses elifesciences/profiles#4

Merged

lafrech mentioned this issue Oct 1, 2018

Drop support for flask 0.x and register default error handler only for HTTPException marshmallow-code/flask-smorest#12

Merged

pallets locked and limited conversation to collaborators Oct 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Registering a handler for HTTPException has no effect #941

Registering a handler for HTTPException has no effect #941

mdeous commented Jan 3, 2014

untitaker commented Jan 6, 2014

mdeous commented Jan 7, 2014

untitaker commented Jan 8, 2014

untitaker commented Jan 14, 2014

untitaker commented Oct 19, 2014

Bekt commented Jul 5, 2015

untitaker commented Jul 5, 2015

Bekt commented Jul 5, 2015

untitaker commented Jul 5, 2015

Bekt commented Jul 5, 2015

untitaker commented Jul 5, 2015

flying-sheep commented Jul 6, 2015

flying-sheep commented Jul 6, 2015

Bekt commented Jul 6, 2015

cerickson commented May 22, 2017

jeffwidman commented May 23, 2017

antgel commented Nov 9, 2019

Bekt commented Nov 12, 2019

flying-sheep commented Nov 12, 2019 •

edited

Loading

mononobi commented Oct 4, 2020 •

edited

Loading

Registering a handler for HTTPException has no effect #941

Registering a handler for HTTPException has no effect #941

Comments

mdeous commented Jan 3, 2014

untitaker commented Jan 6, 2014

mdeous commented Jan 7, 2014

untitaker commented Jan 8, 2014

untitaker commented Jan 14, 2014

untitaker commented Oct 19, 2014

Bekt commented Jul 5, 2015

untitaker commented Jul 5, 2015

Bekt commented Jul 5, 2015

untitaker commented Jul 5, 2015

Bekt commented Jul 5, 2015

untitaker commented Jul 5, 2015

flying-sheep commented Jul 6, 2015

flying-sheep commented Jul 6, 2015

Bekt commented Jul 6, 2015

cerickson commented May 22, 2017

jeffwidman commented May 23, 2017

antgel commented Nov 9, 2019

Bekt commented Nov 12, 2019

flying-sheep commented Nov 12, 2019 • edited Loading

mononobi commented Oct 4, 2020 • edited Loading

flying-sheep commented Nov 12, 2019 •

edited

Loading

mononobi commented Oct 4, 2020 •

edited

Loading