Merge pull request #2362 from davidism/remove-error-handler-cache

Remove error handler cache

CHANGES

@@ -76,6 +76,9 @@ Major release, unreleased
 - Extract JSON handling to a mixin applied to both the request and response
   classes used by Flask. This adds the ``is_json`` and ``get_json`` methods to
   the response to make testing JSON response much easier. (`#2358`_)
+- Removed error handler caching because it caused unexpected results for some
+  exception inheritance hierarchies. Register handlers explicitly for each
+  exception if you don't want to traverse the MRO. (`#2362`_)
 
 .. _#1489: https://github.com/pallets/flask/pull/1489
 .. _#1621: https://github.com/pallets/flask/pull/1621
@@ -98,6 +101,7 @@ Major release, unreleased
 .. _#2352: https://github.com/pallets/flask/pull/2352
 .. _#2354: https://github.com/pallets/flask/pull/2354
 .. _#2358: https://github.com/pallets/flask/pull/2358
+.. _#2362: https://github.com/pallets/flask/pull/2362
 
 Version 0.12.2
 --------------

flask/app.py

@@ -1484,32 +1484,28 @@ class Flask(_PackageBoundObject):
         return f
 
     def _find_error_handler(self, e):
-        """Find a registered error handler for a request in this order:
+        """Return a registered error handler for an exception in this order:
         blueprint handler for a specific code, app handler for a specific code,
-        blueprint generic HTTPException handler, app generic HTTPException handler,
+        blueprint handler for an exception class, app handler for an exception
-        and returns None if a suitable handler is not found.
+        class, or ``None`` if a suitable handler is not found.
         """
         exc_class, code = self._get_exc_class_and_code(type(e))
 
-        def find_handler(handler_map):
+        for name, c in (
+            (request.blueprint, code), (None, code),
+            (request.blueprint, None), (None, None)
+        ):
+            handler_map = self.error_handler_spec.setdefault(name, {}).get(c)
+
             if not handler_map:
-                return
+                continue
 
             for cls in exc_class.__mro__:
                 handler = handler_map.get(cls)
+
                 if handler is not None:
-                    # cache for next time exc_class is raised
-                    handler_map[exc_class] = handler
                     return handler
 
-        # check for any in blueprint or app
-        for name, c in ((request.blueprint, code), (None, code),
-                        (request.blueprint, None), (None, None)):
-            handler = find_handler(self.error_handler_spec.get(name, {}).get(c))
-
-            if handler:
-                return handler
-
     def handle_http_exception(self, e):
         """Handles an HTTP exception.  By default this will invoke the
         registered error handlers and fall back to returning the

tests/test_basic.py

@@ -980,6 +980,39 @@ def test_http_error_subclass_handling(app, client):
     assert client.get('/3').data == b'apple'
 
 
+def test_errorhandler_precedence(app, client):
+    class E1(Exception):
+        pass
+
+    class E2(Exception):
+        pass
+
+    class E3(E1, E2):
+        pass
+
+    @app.errorhandler(E2)
+    def handle_e2(e):
+        return 'E2'
+
+    @app.errorhandler(Exception)
+    def handle_exception(e):
+        return 'Exception'
+
+    @app.route('/E1')
+    def raise_e1():
+        raise E1
+
+    @app.route('/E3')
+    def raise_e3():
+        raise E3
+
+    rv = client.get('/E1')
+    assert rv.data == b'Exception'
+
+    rv = client.get('/E3')
+    assert rv.data == b'E2'
+
+
 def test_trapping_of_bad_request_key_errors(app, client):
     @app.route('/fail')
     def fail():