Merge defgen-error-handling-1709

Author: exarkun Reviewer: radix Fixes twisted#1709 This changes deferredGenerator-wrapped generator functions to only produce Deferreds which fire with results which are *not* the result of a yield waitForDeferred-wrapped Deferred. The common annoying manifestation of this bug is that a generator which handles an exception from waitForDeferred.getResult() with no subsequent yields would cause the deferredGenerator's result to be that Failure. git-svn-id: svn://svn.twistedmatrix.com/svn/Twisted/trunk@16779 bbbe8e31-12d6-0310-92fd-ac37d47ddeeb
isange72 · May 12, 2006 · 28bf7a3 · 28bf7a3
1 parent 801cb12
commit 28bf7a3
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 71 deletions.
diff --git a/twisted/internet/defer.py b/twisted/internet/defer.py
@@ -534,9 +534,9 @@ class waitForDeferred:
 
     Maintainer: U{Christopher Armstrong<mailto:radix@twistedmatrix.com>}
 
-    waitForDeferred and deferredGenerator help you write
-    Deferred-using code that looks like it's blocking (but isn't
-    really), with the help of generators.
+    waitForDeferred and deferredGenerator help you write Deferred-using code
+    that looks like it's blocking (but isn't really), with the help of
+    generators.
 
     There are two important functions involved: waitForDeferred, and
     deferredGenerator.  They are used together, like this::
@@ -548,24 +548,26 @@ def thingummy():
             print thing #the result! hoorj!
         thingummy = deferredGenerator(thingummy)
 
-    waitForDeferred returns something that you should immediately yield;
-    when your generator is resumed, calling thing.getResult() will either
-    give you the result of the Deferred if it was a success, or raise an
-    exception if it was a failure.
+    waitForDeferred returns something that you should immediately yield; when
+    your generator is resumed, calling thing.getResult() will either give you
+    the result of the Deferred if it was a success, or raise an exception if it
+    was a failure.  Calling C{getResult} is B{absolutely mandatory}.  If you do
+    not call it, I{your program will not work}.
 
-    deferredGenerator takes one of these waitForDeferred-using
-    generator functions and converts it into a function that returns a
-    Deferred. The result of the Deferred will be the last
-    value that your generator yielded (remember that 'return result' won't
-    work; use 'yield result; return' in place of that).
+    deferredGenerator takes one of these waitForDeferred-using generator
+    functions and converts it into a function that returns a Deferred. The
+    result of the Deferred will be the last value that your generator yielded
+    unless the last value is a waitForDeferred instance, in which case the
+    result will be C{None}.  If the function raises an unhandled exception, the
+    Deferred will errback instead.  Remember that 'return result' won't work;
+    use 'yield result; return' in place of that.
 
-    Note that not yielding anything from your generator will make the
-    Deferred result in None. Yielding a Deferred from your generator
-    is also an error condition; always yield waitForDeferred(d)
-    instead.
+    Note that not yielding anything from your generator will make the Deferred
+    result in None. Yielding a Deferred from your generator is also an error
+    condition; always yield waitForDeferred(d) instead.
 
-    The Deferred returned from your deferred generator may also
-    errback if your generator raised an exception.  For example::
+    The Deferred returned from your deferred generator may also errback if your
+    generator raised an exception.  For example::
 
         def thingummy():
             thing = waitForDeferred(makeSomeRequestResultingInDeferred())
@@ -580,25 +582,30 @@ def thingummy():
                 raise Exception('DESTROY ALL LIFE')
         thingummy = deferredGenerator(thingummy)
 
-    Put succinctly, these functions connect deferred-using code with this
-    'fake blocking' style in both directions: waitForDeferred converts from
-    a Deferred to the 'blocking' style, and deferredGenerator converts from
-    the 'blocking' style to a Deferred.
+    Put succinctly, these functions connect deferred-using code with this 'fake
+    blocking' style in both directions: waitForDeferred converts from a
+    Deferred to the 'blocking' style, and deferredGenerator converts from the
+    'blocking' style to a Deferred.
     """
+
     def __init__(self, d):
         if not isinstance(d, Deferred):
             raise TypeError("You must give waitForDeferred a Deferred. You gave it %r." % (d,))
         self.d = d
 
+
     def getResult(self):
-        if hasattr(self, 'failure'):
-            self.failure.raiseException()
+        if isinstance(self.result, failure.Failure):
+            self.result.raiseException()
         return self.result
 
-def _deferGenerator(g, deferred=None, result=None):
+
+
+def _deferGenerator(g, deferred=None):
     """
     See L{waitForDeferred}.
     """
+    result = None
     while 1:
         if deferred is None:
             deferred = Deferred()
@@ -619,30 +626,24 @@ def _deferGenerator(g, deferred=None, result=None):
             return fail(TypeError("Yield waitForDeferred(d), not d!"))
 
         if isinstance(result, waitForDeferred):
-            waiting=[True, None]
+            waiting = [True, None]
             # Pass vars in so they don't get changed going around the loop
             def gotResult(r, waiting=waiting, result=result):
                 result.result = r
                 if waiting[0]:
                     waiting[0] = False
                     waiting[1] = r
                 else:
-                    _deferGenerator(g, deferred, r)
-            def gotError(f, waiting=waiting, result=result):
-                result.failure = f
-                if waiting[0]:
-                    waiting[0] = False
-                    waiting[1] = f
-                else:
-                    _deferGenerator(g, deferred, f)
-            result.d.addCallbacks(gotResult, gotError)
+                    _deferGenerator(g, deferred)
+            result.d.addBoth(gotResult)
             if waiting[0]:
                 # Haven't called back yet, set flag so that we get reinvoked
                 # and return from the loop
                 waiting[0] = False
                 return deferred
-            else:
-                result = waiting[1]
+            result = None # waiting[1]
+
+
 
 def deferredGenerator(f):
     """

diff --git a/twisted/test/test_defgen.py b/twisted/test/test_defgen.py
@@ -66,6 +66,51 @@ def _genDeferred():
 
         return self.assertFailure(_genDeferred(), TypeError)
 
+
+    def testHandledTerminalFailure(self):
+        """
+        Create a Deferred Generator which yields a Deferred which fails and
+        handles the exception which results.  Assert that the Deferred
+        Generator does not errback its Deferred.
+        """
+        class TerminalException(Exception):
+            pass
+
+        def _genFailure():
+            x = waitForDeferred(defer.fail(TerminalException("Handled Terminal Failure")))
+            yield x
+            try:
+                x.getResult()
+            except TerminalException:
+                pass
+        _genFailure = deferredGenerator(_genFailure)
+        return _genFailure().addCallback(self.assertEqual, None)
+
+
+    def testHandledTerminalAsyncFailure(self):
+        """
+        Just like testHandledTerminalFailure, only with a Deferred which fires
+        asynchronously with an error.
+        """
+        class TerminalException(Exception):
+            pass
+
+
+        d = defer.Deferred()
+        def _genFailure():
+            x = waitForDeferred(d)
+            yield x
+            try:
+                x.getResult()
+            except TerminalException:
+                pass
+        _genFailure = deferredGenerator(_genFailure)
+        deferredGeneratorResultDeferred = _genFailure()
+        d.errback(TerminalException("Handled Terminal Failure"))
+        return deferredGeneratorResultDeferred.addCallback(
+            self.assertEqual, None)
+
+
     def testStackUsage(self):
         # Make sure we don't blow the stack when yielding immediately
         # available values

diff --git a/twisted/test/test_newcred.py b/twisted/test/test_newcred.py
@@ -332,8 +332,6 @@ def testNegative(self):
                 r = wFD(chk.requestAvatarId(cred))
                 yield r
                 self.assertRaises(error.UnauthorizedLogin, r.getResult)
-        # Work around deferredGenerator bug
-        yield None
     testNegative = dG(testNegative)
 
 class HashlessFilePasswordDBMixin:

diff --git a/twisted/web2/fileupload.py b/twisted/web2/fileupload.py
@@ -11,23 +11,6 @@
 from twisted.web2 import http_headers
 from cStringIO import StringIO
 
-# This functionality of allowing non-deferreds to be yielded
-# perhaps ought to be part of defgen
-def wait(d):
-    if isinstance(d, defer.Deferred):
-        return defer.waitForDeferred(d)
-    return _fakewait(d)
-
-class _fakewait(defer.waitForDeferred):
-    def __init__(self, val):
-        #print "_fakewait", val
-        self.d = self
-        self.result = val
-
-    def addCallbacks(self, success, fail):
-        success(self.result)
-
-
 ###################################
 #####  Multipart MIME Reader  #####
 ###################################
@@ -64,7 +47,10 @@ def _readHeaders(stream):
     # Now read headers
     while 1:
         line = stream.readline(maxLength=1024)
-        line = wait(line); yield line; line = line.getResult()
+        if isinstance(line, defer.Deferred):
+            line = defer.waitForDeferred(line)
+            yield line
+            line = line.getResult()
         #print "GOT", line
         if line == "":
             break # End of headers
@@ -180,7 +166,10 @@ def read(self):
     def _readFirstBoundary(self):
         #print "_readFirstBoundary"
         line = self.stream.readline(maxLength=1024)
-        line = wait(line); yield line; line = line.getResult()
+        if isinstance(line, defer.Deferred):
+            line = defer.waitForDeferred(line)
+            yield line
+            line = line.getResult()
         if line != self.boundary:
             raise MimeFormatError("Extra data before first boundary: %r"% line)
 
@@ -192,7 +181,10 @@ def _readFirstBoundary(self):
     def _readBoundaryLine(self):
         #print "_readBoundaryLine"
         line = self.stream.readline(maxLength=1024)
-        line = wait(line); yield line; line = line.getResult()
+        if isinstance(line, defer.Deferred):
+            line = defer.waitForDeferred(line)
+            yield line
+            line = line.getResult()
 
         if line == "--":
             # THE END!
@@ -252,7 +244,10 @@ def parseMultipartFormData(stream, boundary,
 
     while 1:
         datas = mms.read()
-        datas = wait(datas); yield datas; datas = datas.getResult()
+        if isinstance(datas, defer.Deferred):
+            datas = defer.waitForDeferred(datas)
+            yield datas
+            datas = datas.getResult()
         if datas is None:
             break
 
@@ -269,7 +264,11 @@ def parseMultipartFormData(stream, boundary,
         else:
             outfile = tempfile.NamedTemporaryFile()
             maxBuf = maxSize
-        x = wait(readIntoFile(stream, outfile, maxBuf)); yield x; x=x.getResult()
+        x = readIntoFile(stream, outfile, maxBuf)
+        if isinstance(x, defer.Deferred):
+            x = defer.waitForDeferred()
+            yield x
+            x = x.getResult()
         if filename is None:
             # Is a normal form field
             outfile.seek(0)
@@ -335,7 +334,10 @@ def parse_urlencoded(stream, maxMem=100*1024, maxFields=1024,
 
     while 1:
         datas = s.read()
-        datas = wait(datas); yield datas; datas = datas.getResult()
+        if isinstance(datas, defer.Deferred):
+            datas = defer.waitForDeferred(datas)
+            yield datas
+            datas = datas.getResult()
         if datas is None:
             break
         name, value = datas

diff --git a/twisted/words/test/test_service.py b/twisted/words/test/test_service.py
@@ -59,10 +59,6 @@ def _entityCreationTest(self, kind):
         d = wFD(get(u"another" + kind.lower()))
         yield d
         self.assertRaises(noSuchExc, d.getResult)
-
-        # XXX wfd hack to keep the test from failing even though the
-        # last "result" was a failure
-        yield None
     _entityCreationTest = dG(_entityCreationTest)
 
 
@@ -98,10 +94,6 @@ def testUserRetrieval(self):
         d = wFD(realm.lookupUser(u"nosuchuser"))
         yield d
         self.assertRaises(ewords.NoSuchUser, d.getResult)
-
-        # XXX wfd hack to keep the test from failing even though the
-        # last "result" was a failure
-        yield None
     testUserRetrieval = dG(testUserRetrieval)
 
 
@@ -145,7 +137,6 @@ def testGroupRetrieval(self):
         d = wFD(realm.getGroup(u"nosuchgroup"))
         yield d
         self.assertRaises(ewords.NoSuchGroup, d.getResult)
-        yield None # XXX wfd hack
     testGroupRetrieval = dG(testGroupRetrieval)