Merge pull request #706 from rob-smallshire/error-handler-suppression

Fix error handler exception suppression

Author: j5awry

CHANGELOG.rst

@@ -6,7 +6,7 @@ Changelog
 Current
 -------
 
-- Nothing yet
+- Ensure that exceptions raised in error handler, including programming errors, are logged (:issue:`705`, :pr:`706`)
 
 0.13.0 (2019-08-12)
 -------------------

flask_restplus/api.py

@@ -579,8 +579,8 @@ def error_router(self, original_handler, e):
         if self._has_fr_route():
             try:
                 return self.handle_error(e)
-            except Exception:
-                pass  # Fall through to original handler
+            except Exception as f:
+                return original_handler(f)
         return original_handler(e)
 
     def handle_error(self, e):

tests/test_errors.py

@@ -2,6 +2,8 @@
 from __future__ import unicode_literals
 
 import json
+import logging
+
 import pytest
 
 from flask import Blueprint, abort
@@ -162,6 +164,31 @@ def handle_custom_exception(error):
             'test': 'value',
         }
 
+    def test_blunder_in_errorhandler_is_not_suppressed_in_logs(self, app, client, caplog):
+
+        api = restplus.Api(app)
+
+        class CustomException(RuntimeError):
+            pass
+
+        class ProgrammingBlunder(Exception):
+            pass
+
+        @api.route('/test/', endpoint="test")
+        class TestResource(restplus.Resource):
+            def get(self):
+                raise CustomException('error')
+
+        @api.errorhandler(CustomException)
+        def handle_custom_exception(error):
+            raise ProgrammingBlunder("This exception needs to be logged, not suppressed, then cause 500")
+
+        with caplog.at_level(logging.ERROR):
+            response = client.get('/test/')
+        exc_type, value, traceback = caplog.records[0].exc_info
+        assert exc_type is ProgrammingBlunder
+        assert response.status_code == 500
+
     def test_errorhandler_for_custom_exception_with_headers(self, app, client):
         api = restplus.Api(app)
 
@@ -480,15 +507,23 @@ def test_handle_not_include_error_message(self, app):
             assert 'message' not in json.loads(response.data.decode())
 
     def test_error_router_falls_back_to_original(self, app, mocker):
+        class ProgrammingBlunder(Exception):
+            pass
+
+        blunder = ProgrammingBlunder("This exception needs to be detectable")
+
+        def raise_blunder(arg):
+            raise blunder
+
         api = restplus.Api(app)
         app.handle_exception = mocker.Mock()
-        api.handle_error = mocker.Mock(side_effect=Exception())
+        api.handle_error = mocker.Mock(side_effect=raise_blunder)
         api._has_fr_route = mocker.Mock(return_value=True)
         exception = mocker.Mock(spec=HTTPException)
 
         api.error_router(app.handle_exception, exception)
 
-        app.handle_exception.assert_called_with(exception)
+        app.handle_exception.assert_called_with(blunder)
 
     def test_fr_405(self, app, client):
         api = restplus.Api(app)