Setting error handler for unknown code fails

[georgthegreat]

The following code:

flask_app.errorhandler(402)(http_exception_handler)

raises KeyError: 402 due to missing 402 in default exception. Code works fine with Flask=0.10.1

[sublee]

I have the same issue.

[mitsuhiko]

I can see that this might be annoying. We need to figure out how to deal with non standard status codes and error handlers. How did you use this with Flask < 0.11?

[georgthegreat]

Here is an example of my approach: https://github.com/hda-technical/dancebooks/blob/master/www/main.py#L488

I just set custom exception handler to each HTTP code from werkzeug.HTTP_STATUS_CODES.
I can easily fix this on my site (I don't have any particular meaning for 402 status), but I decided to create an issue first.

[docapotamus]

I am also having an issue with error handlers.

I am using it slightly differently than above (https://github.com/pjuu/pjuu/blob/master/pjuu/lib/errors.py) to try make it as generic as possible.

Not sure if my case is related but I am getting:

  File "/Users/joedoherty/.virtualenvs/pjuu/lib/python2.7/site-packages/flask/app.py", line 1465, in find_handler
    handler = handler_map.get(cls)
AttributeError: 'function' object has no attribute 'get'

I can get around this I think by implementing the generic error handler as a class with a get method but I am not sure if this is the correct way to go about this.

I can open a new issue if you want or if I'm just using undocumented APIs, please feel free to tell me to get lost 😈

[sublee]

@docapotamus You should register error handlers by the @errorhandler decorator instead.

[docapotamus]

@sublee thanks for showing me the error of my ways. Will get this sorted it's now not a problem.

Sorry to hijack this issue.

[liebald]

Looks like setting error handlers by assigning to app.error_handler_spec no longer works as expected in Flask 0.11, since the semantics of this dict have changed (from my reading of the code, it looks like it used to be a mapping from blueprint + error code -> handler, but it's now a mapping from blueprint + error code + exception class -> handler). As a consequence, setting values in error_handler_spec directly will lead to exceptions when flask tries to look up the correct handler for a given error (AttributeError: 'function' object has no attribute 'get').

Solution: Use app.register_error_handler (or use app.errorhandler in a non-decorator way) instead.

However, the documentation makes no mention that this no longer works (the latest docs still give an example of how to use error_handler_spec that's now broken: http://flask.pocoo.org/docs/0.11/api/#flask.Flask.error_handler_spec). Is this a bug that should be fixed, or did you intend to make this backwards-incompatible change? If the latter, can we update the documentation? I'd be happy to send a pull request, but wanted to make sure I'm not missing something here.

[untitaker]

Yes, sorry, the docs are completely out of date. The new mapping is blueprint => statuscode => class for any kind of exception instead of blueprint => statuscode => ("list of tuples" if statuscode is None else handler).

[untitaker]

Also I'd like to emphasize in the docs that it's discouraged to use this attribute.

[liebald]

Re API improvements: How about making the actual map a private attribute and force access through dedicated APIs, such as register_error_handler and @errorhandler? It would require users to upgrade their code, but anyone who used error_handler_spec in <= 0.10 already had their code broken in 0.11 anyway, and it would be better to make that explicit (we only found out because we happen to have unit tests for our error handlers).

Re documentation: The reason we were using the map in the first place was that using a decorator didn't work with application factories, and somehow we missed register_error_handler (either it didn't exist, or the docs didn't mention it at the time). Given that an application factory with multiple blueprints is a pretty common pattern these days, it would be good to improve the documentation to outline how to set error handlers that work across blueprints, e.g. in this article: http://flask.pocoo.org/docs/0.11/patterns/errorpages/

I'd be happy to help out with either docs or code changes, let me know your thoughts and what would be most useful.

[davidism]

using a decorator didn't work with application factories

Regardless of the existence of the other function, this isn't true. A decorator is just a function that is passed a function, it can be used anywhere.

def factory():
    ...
    app.errorhandler(404)(our_404_handler)

[liebald]

@davidism Sure. Though I suspect this usage isn't obvious to all (most?) users. Hence the existence of register_error_handler. In any case, the main point is that error_handler_spec shouldn't be accessible as a public attribute, since it's subject to backwards-incompatible changes from version to version, and the documentation could make that clearer.

Just sent out a PR for improving some of the documentation: #2077. Happy to clean up the API reference docs, too, if this one's going in the right direction.

@untitaker @mitsuhiko Curious what your thoughts are re my proposal for blocking access to error_handler_spec by making it a private attribute (turn it into _error_handler_spec and put a property on error_handler_spec with a setter that raises AttributeError, or something along those lines)?

[hrbonz]

Not a specialist of Flask code but did hit the same kind of issue, you can see gist of how we set error handling here (code has been cleaned a bit): https://gist.github.com/hrbonz/5cc9d9d1a63593cd87b3ef555470706c

This used to work fine (in 0.10.1), the problem seems to be that now (I don't know how it was before), Flask._find_error_handler() will try to strictly match the error code from app.error_handler_spec when my handlers are registered under the None key.

Should I create a factory to register every error code individually or is it a bug in how the handlers are found?

Bottom line is, it looks like we now have to register all exception codes one by one instead of simply creating a generic error handler.

[lingnand]

Also, trap_http_exception NO LONGER works. I don't know why you have to force people to register error handler by codes. For something simple like this:

app.config['TRAP_HTTP_EXCEPTIONS'] = True

@app.errorhandler(Exception)
def global_handler(e):
    do_something(e)

which used to work, but now no longer works because of this forced look up in the dictionary by code (app.py line 1449).

[tuxpowered]

Not sure this is a correct way of handling this or not but it works...

from werkzeug.exceptions import default_exceptions

def handle_error(e):
    code = 500
    error, message = str(e).split(':', 1)

    if isinstance(e, HTTPException):
        code = e.code

    errors = dict(
        error=error,
        message=message.strip(),
        code=code,
        path=request.path,
    )
    return jsonify(errors=errors), code

for code in default_exceptions:
    app.register_error_handler(code, handle_error)

The key is that default_exceptions is a dictionary of valid HTTP errors that werkzeug processes. I suppose you could add custom HTTP codes to this dictionary as well.

The above returns a response like so:

http -j 127.0.0.1:5000/sms/messages22
HTTP/1.0 404 NOT FOUND
Content-Length: 242
Content-Type: application/json
Date: Wed, 22 Mar 2017 06:23:20 GMT
Server: Werkzeug/0.12.1 Python/3.4.3

{
    "errors": {
        "code": 404,
        "error": "404 Not Found",
        "message": "The requested URL was not found on the server.  If you entered the URL manually please check your spelling and try again.",
        "path": "/sms/messages22"
    }
}

[4poc]

This is still an issue, with this:

@app.errorhandler(402)
def handle_error(e):
    # ...

causes a crash:

Traceback (most recent call last):
  File "[..].py", line 6, in <module>
    exec(compile(open(__file__).read(), __file__, 'exec'))
  File "[..].py", line 22, in <module>
    @app.errorhandler(402)
  File "[..]python3.5/site-packages/flask/app.py", line 1159, in decorator
    self._register_error_handler(None, code_or_exception, f)
  File "[..]python3.5/site-packages/flask/app.py", line 64, in wrapper_func
    return f(self, *args, **kwargs)
  File "[..]python3.5/site-packages/flask/app.py", line 1185, in _register_error_handler
    exc_class, code = self._get_exc_class_and_code(code_or_exception)
  File "[..]python3.5/site-packages/flask/app.py", line 1104, in _get_exc_class_and_code
    exc_class = default_exceptions[exc_class_or_code]
KeyError: 402

In my case I want a global error handler, that catches everything to do the handling of errors myself. Workaround for me (why am I even using flask at this point):

@app.errorhandler(Exception):
def my_global_error_handler(e):
    ...

from werkzeug import HTTP_STATUS_CODES
for code in HTTP_STATUS_CODES:
    try:
        app.register_error_handler(code, my_global_error_handler)
    except KeyError:
        pass

Flask Version: 0.12.1

[untitaker]

I'm looking into fixing this.

@mitsuhiko what do you think about the app object holding a mapping from error codes to exceptions, where httpexception-subclasses are dynamically added if errorhandlers for unknown codes are registered?

So basically werkzeug.exceptions.default_exceptions would live on the app object. We would need to write a custom flask.abort that uses that mapping instead of reexporting from Werkzeug.

[davidism]

Here's what I'm thinking: The Flask app has an instance of werkzeug.exceptions.Aborter. Custom exceptions get added to that instance's map. The Flask version of the abort function tries current_app.aborter() first (or only). This is similar to how the Flask app has a url_map that url_for refers to.

[untitaker]

Right, that's better.

…

On April 15, 2017 2:14:15 PM GMT+02:00, David Lord ***@***.***> wrote: Here's what I'm thinking: The Flask app has an instance of `werkzeug.exceptions.Aborter`. Custom exceptions get added to that instances map. The Flask version of the abort `abort` tries `current_app.aborter()` first (or only). -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #1837 (comment)

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[davidism]

This is a long discussion, to summarize:

• docs are already updated
• catch-all handlers such as Exception or HTTPException do work with status codes, you don't need to register it for each code. Probably due to Added support for generic HTTPException handlers on app and blueprints #2304.
• errorhandler(402) fails because there's no associated exception (same with abort(402), but that wasn't in the original report)

[davidism]

The simplest solution is to subclass HTTPException, add a handler for the class (not the code), and raise it directly (don't use abort).

from werkzeug.exceptions import HTTPException

class FourZeroTwo(HTTPException):
    code = 402

@app.errorhandler(FourZeroTwo)
def handle_402(e):
    return '402', 402

@app.route('/')
def index():
    raise FourZeroTwo()

If you really want to use the code, it needs to be registered with Werkzeug in two places.

from werkzeug.exceptions import default_exceptions, _aborter

default_exceptions[402] = FourZeroTwo
_aborter.mapping[402] = FourZeroTwo

app.errorhandler(402)  # works
abort(402)  # works

[davidism]

@untitaker are you still looking into this?

I'm not sure what a clean solution would look like. If all we have to go on is errorhandler(402), we still don't know what exception to associate with it. We could generate one, but what if the developer subclasses HTTPException like above but registers it using 402? If they try to abort later, they won't get the same exception class.

We could add a register_error_code(exception) function that takes an HTTPException subclass and adds it to the map. If the user tries to add a code that doesn't exist, the error message would point them at calling this first. In that case, we don't necessarily need the custom Aborter, since it would be safe to modify the globals during setup.

[davidism]

@georgthegreat I'm curious how you were aborting with that code. abort(402) would have raised an error because the aborter still doesn't know that code. Additionally, the code you linked doesn't use any non-standard error codes, so not sure how it's related.

If you have a non-standard code that's not known to Werkzeug, create a subclass with the code and use that. If Werkzeug is missing a standard code, create a patch for Werkzeug to support it. Documenting this and closing.