fix: Route.__repr__ and Route.get_undecorated_callback exceptions.

depr: Inspecting closures in Route.get_undecorated_callback() is deprecated. The functionality will depend on proper use of functools.update_wrapper in the future.

fix: Route.get_undecorated_callback() now supports objects with a `__call__` method in addition to normal functions.
change: Route.get_undecorated_callback() now follows `__wrapped__` if present.
fix: Route.__repr__() now allows route-callbacks without a `__name__`.

fix: #1488

Author: defnull

bottle.py

@@ -529,18 +529,22 @@ def get_undecorated_callback(self):
         """ Return the callback. If the callback is a decorated function, try to
             recover the original function. """
         func = self.callback
-        func = getattr(func, '__func__', func)
-        while hasattr(func, '__closure__') and getattr(func, '__closure__'):
-            attributes = getattr(func, '__closure__')
-            func = attributes[0].cell_contents
-
-            # in case of decorators with multiple arguments
-            if not isinstance(func, FunctionType):
-                # pick first FunctionType instance from multiple arguments
-                func = filter(lambda x: isinstance(x, FunctionType),
-                              map(lambda x: x.cell_contents, attributes))
-                func = list(func)[0]  # py3 support
-        return func
+        while True:
+            if getattr(func, '__wrapped__', False):
+                func = func.__wrapped__
+            elif getattr(func, '__func__', False):
+                func = func.__func__
+            elif getattr(func, '__closure__', False):
+                depr(0, 14, "Decorated callback without __wrapped__",
+                     "When applying decorators to route callbacks, make sure" \
+                     " the decorator uses @functools.wraps or update_wrapper." \
+                     " This warning may also trigger if you reference callables" \
+                     " from a nonlocal scope.")
+                cells_values = (cell.cell_contents for cell in func.__closure__)
+                isfunc = lambda x: isinstance(x, FunctionType) or hasattr(x, '__call__')
+                func = next(filter(isfunc, cells_values), func)
+            else:
+                return func
 
     def get_callback_args(self):
         """ Return a list of argument names the callback (most likely) accepts
@@ -561,7 +565,9 @@ def get_config(self, key, default=None):
 
     def __repr__(self):
         cb = self.get_undecorated_callback()
-        return '<%s %s -> %s:%s>' % (self.method, self.rule, cb.__module__, cb.__name__)
+        return '<%s %s -> %s:%s>' % (
+            self.method, self.rule, cb.__module__, getattr(cb, '__name__', '?')
+        )
 
 ###############################################################################
 # Application Object ###########################################################

docs/changelog.rst

@@ -36,6 +36,10 @@ to receive updates on a best-effort basis.
 Release 0.14 (in development)
 =============================
 
+.. rubric:: Deprecated APIs or behavior
+
+* ``Route.get_undecorated_callback()`` was able to look into closure cells to guess the original function wrapped by a decorator, but this is too aggressive in some cases and may return the wrong function. To avoid this, we will depend on proper use of ``@functools.wraps(orig)`` or ``functools.update_wrapper(wrapper, orig)`` in decorators in the future.
+
 .. rubric:: Removed APIs
 
 * Dropped support for Python 2 (EOL: 2020-01-01) and removed workarounds or helpers that only make sense in a Python 2/3 dual codebase.

test/test_route.py

@@ -1,3 +1,4 @@
+import functools
 import unittest
 import bottle
 from .tools import api
@@ -20,6 +21,7 @@ class TestRoute(unittest.TestCase):
     def test_callback_inspection(self):
         def x(a, b): pass
         def d(f):
+            @functools.wraps(f)
             def w():
                 return f()
             return w
@@ -29,6 +31,7 @@ def w():
 
         def d2(foo):
             def d(f):
+                @functools.wraps(f)
                 def w():
                     return f()
                 return w
@@ -42,6 +45,7 @@ def test_callback_inspection_multiple_args(self):
         # decorator with argument, modifying kwargs
         def d2(f="1"):
             def d(fn):
+                @functools.wraps(fn)
                 def w(*args, **kwargs):
                     # modification of kwargs WITH the decorator argument
                     # is necessary requirement for the error
@@ -64,3 +68,57 @@ def test_callback_inspection_newsig(self):
         eval(compile('def foo(a, *, b=5): pass', '<foo>', 'exec'), env, env)
         route = bottle.Route(bottle.Bottle(), None, None, env['foo'])
         self.assertEqual(set(route.get_callback_args()), set(['a', 'b']))
+
+    def test_unwrap_wrapped(self):
+        import functools
+        def func(): pass
+        @functools.wraps(func)
+        def wrapped():
+            return func()
+
+        route = bottle.Route(bottle.Bottle(), None, None, wrapped)
+        self.assertEqual(route.get_undecorated_callback(), func)
+
+    @api("0.12", "0.14")
+    def test_unwrap_closure(self):
+        def func(): pass
+        wrapped = _null_decorator(func, update_wrapper=False)
+        route = bottle.Route(bottle.Bottle(), None, None, wrapped)
+        self.assertEqual(route.get_undecorated_callback(), func)
+
+    # @api("0.15")
+    # def test_not_unwrap_closure(self):
+    #     def other(): pass
+    #     def func():
+    #         return other()
+    #     route = bottle.Route(bottle.Bottle(), None, None, func)
+    #     self.assertEqual(route.get_undecorated_callback(), func)
+
+    @api("0.13", "0.14")
+    def test_unwrap_closure_callable(self):
+        class Foo:
+            def __call__(self): pass
+
+        func = Foo()
+        wrapped = _null_decorator(func, update_wrapper=False)
+        route = bottle.Route(bottle.Bottle(), None, None, wrapped)
+        self.assertEqual(route.get_undecorated_callback(), func)
+        repr(route) # Raised cause cb has no '__name__'
+
+    def test_unwrap_method(self):
+        def func(self): pass
+
+        class Foo:
+            test = _null_decorator(func)
+
+        wrapped = Foo().test
+        route = bottle.Route(bottle.Bottle(), None, None, wrapped)
+        self.assertEqual(route.get_undecorated_callback(), func)
+
+
+def _null_decorator(func, update_wrapper=True):
+    def wrapper():
+        return func()
+    if update_wrapper:
+        functools.update_wrapper(wrapper, func)
+    return wrapper