More flexible errorhandler #2

[zarnovican]

DO NOT MERGE !

This PR builds on top of #401 adding ability to specify exception by its HTTP code. Although the code is fine and tests are passing, I would like an opinion on it first.

joshfriend:
..It looks like if you were to add an errorhandler to catch BadRequest, your custom resposne would be overridden by these checks.

You can have your own BadRequest handler (see the test).

'data' as dictionary

handle_error code assumes that data is a dictionary (data.update(custom_data)). This may not always be the case. For example, if you would like to have a custom error handler on non-HTTPException with .data attribute as string. This is a problem on existing code already.

Adding this PR would make situation much worse. User may return anything from his error handler. Current code will fall through to a series of if-code==XXX-do-this blocks which assume returned data is a dict. In other words, this PR works, but only in happy-path when User is cooperating..

exceptions in 'handle_error'

Exception in custom error handler will be passed down to Flask, which will fall over with..

  File ".../python2.7/site-packages/flask/app.py", line 1363, in handle_user_exception
    assert exc_value is e
AssertionError

There is NO indication what (or where) was the original exception.

We could (and should) wrap call to custom error handler with try/catch-all. Thou, the problem may not in inside the handler. This code..

@api.errorhandler(404)
def foo1(e):
    return 'foo', 404

..will also generate the same AssertionError. The underlying problem is that default 404 handling assumes that data is a dict.

fall-through-ness

Processing won't stop when error handler is found. This is a source of the problems mentioned above. However, somehow the User may depend on this feature. Eg. someone may want add authorization challenge on 401, even if it was already custom handled. I, personally, don't like current fall-trough. Refactoring the code would be quite a major change..

default handlers >=500, 404, 401, 406

These checks will probably need to be removed and replaced with default errorhandler functions that are set up when an API object is created

I concur the idea.

However, it's not that simple (I tried 😒):

• currently it is possible to have both custom and default handler code executed. Not so after the change
• error handler would need to be able to indicate that it did not handle anything and the next best match should be executed:

def default_404_handler(self, e):
    if ERROR_404_HELP is enabled in config:
        # do dome work
        return data, 404
    else:
        # else what ? Explicit fall-through..
        return None

• existing code is in some cases extending response created earlier..

data["message"] = data["message"].rstrip('.') + '. '
data['message'] += 'You have requested...'

After rewrite, these handlers would have no context other than the exception itself.

• 406 (NotAcceptable) handler is messing around with representation of returned data. This is not possible, if your error handler just returns (data, code, headers) tuple. That code would probably have to go to self.make_response

Summary

Either this PR is merged as it is.. making small incremental steps forward.
or
Much bigger rewrite is needed. It should be clarified first:

• if processing should stop on first matching handler - this will change the existing semantics
• if existing handle_error code should be rewritten as several smaller error handlers
• what to do when exception is thrown inside error handler (may be explicit abort(404))

[joshfriend]

I'm going to close this one in favor of your more recent work in #544