return_future and friends now pass the result, not the Future, to the…

… callback.

Functions and decorators that take an optional callback and return
a future (return_future, gen.concurrent, and run_executor) no longer pass
the Future object to the callback.  This results in somewhat less flexible
error handling, but is more consistent with prevailing practice without
Futures.

tornado/concurrent.py

@@ -110,7 +110,8 @@ def wrapper(self, *args, **kwargs):
         callback = kwargs.pop("callback", None)
         future = self.executor.submit(fn, self, *args, **kwargs)
         if callback:
-            self.io_loop.add_future(future, callback)
+            self.io_loop.add_future(future,
+                                    lambda future: callback(future.result()))
         return future
     return wrapper
 
@@ -125,10 +126,13 @@ def return_future(f):
 
     From the caller's perspective, the callback argument is optional.
     If one is given, it will be invoked when the function is complete
-    with the `Future` as an argument.  If no callback is given, the caller
-    should use the `Future` to wait for the function to complete
-    (perhaps by yielding it in a `gen.engine` function, or passing it
-    to `IOLoop.add_future`).
+    with `Future.result()` as an argument.  If the function fails,
+    the callback will not be run and an exception will be raised into
+    the surrounding `StackContext`.
+
+    If no callback is given, the caller should use the `Future` to
+    wait for the function to complete (perhaps by yielding it in a
+    `gen.engine` function, or passing it to `IOLoop.add_future`).
 
     Usage::
         @return_future
@@ -142,7 +146,8 @@ def caller(callback):
             callback()
 
     Note that ``@return_future`` and ``@gen.engine`` can be applied to the
-    same function, provided ``@return_future`` appears first.
+    same function, provided ``@return_future`` appears first.  However,
+    consider using ``@gen.coroutine`` instead of this combination.
     """
     replacer = ArgReplacer(f, 'callback')
     @functools.wraps(f)
@@ -178,7 +183,9 @@ def handle_error(typ, value, tb):
         # immediate exception, and again when the future resolves and
         # the callback triggers its exception by calling future.result()).
         if callback is not None:
-            future.add_done_callback(wrap(callback))
+            def run_callback(future):
+                callback(future.result())
+            future.add_done_callback(wrap(run_callback))
         return future
     return wrapper
 

tornado/gen.py

@@ -156,7 +156,9 @@ def coroutine(func):
 
     Functions with this decorator return a `Future`.  Additionally,
     they may be called with a ``callback`` keyword argument, which will
-    be invoked with the future when it resolves.
+    be invoked with the future's result when it resolves.  If the coroutine
+    fails, the callback will not be run and an exception will be raised
+    into the surrounding `StackContext`.
 
     From the caller's perspective, ``@gen.coroutine`` is similar to
     the combination of ``@return_future`` and ``@gen.engine``.
@@ -167,7 +169,9 @@ def wrapper(*args, **kwargs):
         future = Future()
 
         if 'callback' in kwargs:
-            IOLoop.current().add_future(future, kwargs.pop('callback'))
+            callback = kwargs.pop('callback')
+            IOLoop.current().add_future(
+                future, lambda future: callback(future.result()))
 
         def handle_exception(typ, value, tb):
             try:

tornado/netutil.py

@@ -178,7 +178,7 @@ def resolve(self, host, port, family=socket.AF_UNSPEC, callback=None):
         pairs, where address is a tuple suitable to pass to
         `socket.connect` (i.e. a (host, port) pair for IPv4;
         additional fields may be present for IPv6). If a callback is
-        passed, it will be run with the `Future` as an argument when
+        passed, it will be run with the result as an argument when
         it is complete.
         """
         raise NotImplementedError()

tornado/simple_httpclient.py

@@ -152,8 +152,8 @@ def __init__(self, io_loop, client, request, release_callback,
 
             self.resolver.resolve(host, port, af, callback=self._on_resolve)
 
-    def _on_resolve(self, future):
-        af, sockaddr = future.result()[0]
+    def _on_resolve(self, addrinfo):
+        af, sockaddr = addrinfo[0]
 
         if self.parsed.scheme == "https":
             ssl_options = {}

tornado/test/concurrent_test.py

@@ -23,6 +23,7 @@
 from tornado.escape import utf8, to_unicode
 from tornado import gen
 from tornado.iostream import IOStream
+from tornado import stack_context
 from tornado.tcpserver import TCPServer
 from tornado.testing import AsyncTestCase, LogTrapTestCase, bind_unused_port, gen_test
 
@@ -66,16 +67,16 @@ def test_return_value(self):
 
     def test_callback_kw(self):
         future = self.sync_future(callback=self.stop)
-        future2 = self.wait()
-        self.assertIs(future, future2)
+        result = self.wait()
+        self.assertEqual(result, 42)
         self.assertEqual(future.result(), 42)
 
     def test_callback_positional(self):
         # When the callback is passed in positionally, future_wrap shouldn't
         # add another callback in the kwargs.
         future = self.sync_future(self.stop)
-        future2 = self.wait()
-        self.assertIs(future, future2)
+        result = self.wait()
+        self.assertEqual(result, 42)
         self.assertEqual(future.result(), 42)
 
     def test_no_callback(self):
@@ -170,7 +171,8 @@ def capitalize(self, request_data, callback=None):
                             callback=self.handle_connect)
         self.future = Future()
         if callback is not None:
-            self.future.add_done_callback(callback)
+            self.future.add_done_callback(
+                stack_context.wrap(lambda future: callback(future.result())))
         return self.future
 
     def handle_connect(self):
@@ -238,13 +240,12 @@ def tearDown(self):
 
     def test_callback(self):
         self.client.capitalize("hello", callback=self.stop)
-        future = self.wait()
-        self.assertEqual(future.result(), "HELLO")
+        result = self.wait()
+        self.assertEqual(result, "HELLO")
 
     def test_callback_error(self):
         self.client.capitalize("HELLO", callback=self.stop)
-        future = self.wait()
-        self.assertRaisesRegexp(CapError, "already capitalized", future.result)
+        self.assertRaisesRegexp(CapError, "already capitalized", self.wait)
 
     def test_future(self):
         future = self.client.capitalize("hello")

tornado/test/gen_test.py

@@ -512,10 +512,8 @@ def test_pass_callback(self):
         @gen.coroutine
         def f():
             raise gen.Return(42)
-        # The callback version passes a future to the callback without
-        # resolving it so exception information is available to the caller.
-        future = yield gen.Task(f)
-        self.assertEqual(future.result(), 42)
+        result = yield gen.Task(f)
+        self.assertEqual(result, 42)
         self.finished = True
 
     @gen_test

tornado/test/netutil_test.py

@@ -28,9 +28,8 @@
 class _ResolverTestMixin(object):
     def test_localhost(self):
         self.resolver.resolve('localhost', 80, callback=self.stop)
-        future = self.wait()
-        self.assertIn((socket.AF_INET, ('127.0.0.1', 80)),
-                      future.result())
+        result = self.wait()
+        self.assertIn((socket.AF_INET, ('127.0.0.1', 80)), result)
 
     @gen_test
     def test_future_interface(self):