Error-handling interoperability

[abathur]

I ran into an interoperability issue caused by error handling in restplus that has surfaced in the flask-jwt-extended repo (vimalloc/flask-jwt-extended#86, vimalloc/flask-jwt-extended#83) and did a little digging to figure out where things are going awry and found a few different problems. To show my work,

Here's a gist with server code and three logs for specific requests (both client and server side): https://gist.github.com/abathur/5d9de7c2adbbd3f2c85e50e5f81fb267

I'm not really sure what the "solution" is (and FWIW, I suspect multiple open flask-restplus issues regarding exceptions/debugging are related to these interop issues), but hopefully this will help.

Problem 1: Flask/app-registered user error handlers aren't getting called on restplus-managed endpoints.

• When we have used @app.errorhandler(ExceptionClass) to specify our own error handler, app.handle_user_exception is responsible for calling the decorated function in order for it to construct a response, but flask_restplus is injecting api.error_router in the way and intercepting all exceptions.
• api.error_router will pass exceptions for non-restplus endpoints back to the original app.handle_user_exception, but all exceptions for restplus endpoints get passed to api.handle_error and only get passed back in the event of an exception in api.handle_error. I think this would be fine if api.handle_error intentionally re-raised non-restplus exceptions.
• If a given exception hasn't been handled before the "else" in the first if-else block of api.handle_error, it will get assigned an INTERNAL_SERVER_ERROR code and message.

Problem 2: Restplus is eating exceptions that occur within the try-except in api.error_router (instead returning cryptic/misleading assertion errors from Flask), but at least some such exceptions can result from problems in user code. I'm guessing many developers could sort out issues mentioning these assertion errors before reporting if the relevant error wasn't getting swallowed.

These errors may look like:

...
  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1542, in handle_exception
    raise e
AssertionError

or:

  File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1504, in handle_user_exception
    assert exc_value is e
AssertionError

Not sure how widespread these may be, but one simple example I found is registering an incorrect error-handler with restplus which fails to return a flask response.

Problem 3: The restplus @errorhandler API appears to just do a class equality check (in handle_error()), whereas the equivalent API in Flask (see flask/flask/app.py::_find_error_handler()) checks the full MRO for the exception to see if the user has registered a handler for any of the parent classes. I haven't gone looking for examples, but I would guess some users of this feature expect equivalent behavior.

[vimalloc]

Author of Flask-JWT-Extended here. From what I can can tell, under normal circumstances this line in flask-restplus will never be hit: https://github.com/noirbizarre/flask-restplus/blob/master/flask_restplus/api.py#L567

That said, if you have a non-standard circumstance and the return original_handler(e) does get hit, it leads to an assertion failure in base flask under python2 (but not under python3). Examples of how to trigger it and the resulting stacktrace can be seen here:

https://github.com/vimalloc/flask-jwt-extended/issues/86#issuecomment-337684948 https://github.com/vimalloc/flask-jwt-extended/issues/86#issuecomment-337697862

Ideally, I would love to see the flask error handler be called if the given exception doesn't exist in the flask-restplus error handlers, although I'm not sure the ramifications this would have on your project (namely for something like a default error handler). Also, having the handoff back to the native error handlers in python2 fixed to avoid that assertion error would be awesome.

Hope this helps!

[YJinHai]

As for the internal error mechanism of this plus, I hope you can spare some time to think about it. Thank you very much @ziirish @SteadBytes @j5awry @a-luna