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.

diff --git a/tornado/concurrent.py b/tornado/concurrent.py
index e24a768ac8e25ba1816559b3207a6928a5a5259e..c92bda0b2e770632e4e35c843bc03ed7e1f919cd 100644 (file)
--- a/tornado/concurrent.py
+++ b/tornado/concurrent.py
@@ -110,7 +110,8 @@ def run_on_executor(fn):
         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 return_future(f):
             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 return_future(f):
         # 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
 

diff --git a/tornado/gen.py b/tornado/gen.py
index 6192fe498344f343da0e0fc9d2c0346a1d73e89f..97a4dd1404a804be920c8affc59b045aa5dd4b40 100644 (file)
--- a/tornado/gen.py
+++ b/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 coroutine(func):
         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:

diff --git a/tornado/netutil.py b/tornado/netutil.py
index 06c88ab61bdbb4b136764745aa5b651ec5c74d45..e60bfedf19c7576ce2293a2de61befedae64dacc 100644 (file)
--- a/tornado/netutil.py
+++ b/tornado/netutil.py
@@ -178,7 +178,7 @@ class Resolver(Configurable):
         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()

diff --git a/tornado/simple_httpclient.py b/tornado/simple_httpclient.py
index 92021f2663c3ec29eac96d1e65397c2aae9981de..760b9b4846fe1dc66052da8a5b223b8638766f01 100644 (file)
--- a/tornado/simple_httpclient.py
+++ b/tornado/simple_httpclient.py
@@ -152,8 +152,8 @@ class _HTTPConnection(object):
 
             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 = {}

diff --git a/tornado/test/concurrent_test.py b/tornado/test/concurrent_test.py
index f58e5c0629dbb1d5b76bbd21648a45f622e0b77b..8e758404c806285a11261c4e8b76851e9f072fc6 100644 (file)
--- a/tornado/test/concurrent_test.py
+++ b/tornado/test/concurrent_test.py
@@ -23,6 +23,7 @@ from tornado.concurrent import Future, return_future, ReturnValueIgnoredError
 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 @@ class ReturnFutureTest(AsyncTestCase):
 
     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 @@ class ManualCapClient(BaseCapClient):
                             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 @@ class ClientTestMixin(object):
 
     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")

diff --git a/tornado/test/gen_test.py b/tornado/test/gen_test.py
index 687fd6758641b97f441c1afeba1bb64e968fe348..f76f9324e9435c20bf41365112cbd509f2e5c36d 100644 (file)
--- a/tornado/test/gen_test.py
+++ b/tornado/test/gen_test.py
@@ -512,10 +512,8 @@ class GenCoroutineTest(AsyncTestCase):
         @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

diff --git a/tornado/test/netutil_test.py b/tornado/test/netutil_test.py
index 53c84c2b95b8ec4ac45a6b0114dbeda318f18705..e83189fc072337c85324fc7050953eabad789ced 100644 (file)
--- a/tornado/test/netutil_test.py
+++ b/tornado/test/netutil_test.py
@@ -28,9 +28,8 @@ else:
 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):