Exceptions in a @return_future function go to both the caller and the Future.

Add a test for the UnboundLocal error seen here and remove the
no-longer-needed initialization.  I'm not 100% sure we want to do both
(due to the risk of callback-doubling), but the old way where the caller
saw one error and the future resolved to a different error was clearly
incorrect, and leaving the future unresolved doesn't seem right either.

diff --git a/tornado/concurrent.py b/tornado/concurrent.py
index 2ca05901df2f8c12ed53dffd526b9bb0423136f1..66cea30bef01e9efafee5fdd690fbad99ca01414 100644 (file)
--- a/tornado/concurrent.py
+++ b/tornado/concurrent.py
@@ -156,11 +156,11 @@ def return_future(f):
             return True
         exc_info = None
         with ExceptionStackContext(handle_error):
-            result = None
             try:
                 result = f(*args, **kwargs)
             except:
                 exc_info = sys.exc_info()
+                raise
             assert result is None, ("@return_future should not be used with "
                                     "functions that return values")
         if exc_info is not None:

diff --git a/tornado/test/concurrent_test.py b/tornado/test/concurrent_test.py
index 3419bbfd3b0d6e0724eea920d67ae7b105a4a771..f0add77942c8a986ca329136fb3e5bdeb1b666ff 100644 (file)
--- a/tornado/test/concurrent_test.py
+++ b/tornado/test/concurrent_test.py
@@ -46,7 +46,12 @@ class ReturnFutureTest(AsyncTestCase):
 
     def test_immediate_failure(self):
         with self.assertRaises(ZeroDivisionError):
+            # The caller sees the error just like a normal function.
             self.immediate_failure(callback=self.stop)
+        # The callback is also run, with a future that contains the error.
+        future = self.wait()
+        with self.assertRaises(ZeroDivisionError):
+            future.result()
 
     def test_callback_kw(self):
         future = self.sync_future(callback=self.stop)