[Twisted-Python] A kinder and more consistent defer.inlineCallbacks
Here's a suggestion for making inlineCallbacks more consistent and less confusing. Let's suppose you're writing something like this: @inlineCallbacks def func(): .... result = func() There are 2 things that could be better, IMO: 1. func may not yield. In that case, you either get an AttributeError when inlineCallbacks tries to send(). Or worse, the call to send might actually work, and do who knows what. I.e., func() could return an object with a send method but which is not a generator. For some fun, run some code that calls the following decorated function (see if you can figure out what will happen before you do): @defer.inlineCallbacks def f(): class yes(): def send(x, y): print 'yes' # accidentally_destroy_the_universe_too() return yes() 2. func might raise before it get to its first yield. In that case you'll get an exception thrown when the inlineCallbacks decorator tries to create the wrapper function: File "/usr/lib/python2.5/site-packages/twisted/internet/defer.py", line 813, in unwindGenerator return _inlineCallbacks(None, f(*args, **kwargs), Deferred()) There's a simple and consistent way to handle both of these. Just have inlineCallbacks do some initial work based on what it has been passed: def altInlineCallbacks(f): def unwindGenerator(*args, **kwargs): deferred = defer.Deferred() try: result = f(*args, **kwargs) except Exception, e: deferred.errback(e) return deferred if isinstance(result, types.GeneratorType): return defer._inlineCallbacks(None, result, deferred) deferred.callback(result) return deferred return mergeFunctionMetadata(f, unwindGenerator) This has the advantage that (barring e.g., a KeyboardInterrupt in the middle of things) you'll *always* get a deferred back when you call an inlineCallbacks decorated function. That deferred might have already called or erred back (corresponding to cases 1 and 2 above). I'm going to use this version of inlineCallbacks in my code. There's a case for it making it into Twisted itself: inlinecallbacks is already cryptic enough in its operation that anything we can do to make its operation more uniform and less surprising, the better. You might think that case 1 rarely comes up. But I've hit it a few times, usually when commenting out sections of code for testing. If you accidentally comment out the last yield in func, it no longer returns a generator and that causes a different error. And case 2 happens to me too. Having inlinecallbacks try/except the call to func is nicer because it means I don't have to be quite so defensive in coding. So instead of me having to write try: d = func() except Exception: # ??? and try to figure out what happened if an exception fired, I can just write d = func() and add errbacks as I please (they then have to figure out what happened). The (slight?) disadvantage to my suggestion is that with the above try/except fragment you can tell if the call to func() raised before ever yielding. You can detect that, if you need to, with my approach if you're not offended by looking at d.called immediately after calling func. The alternate approach also helps if you're a novice, or simply being lazy/careless/forgetful, and writing: d = func() d.addCallback(ok) d.addErrback(not_ok) thinking you have your ass covered, but you actually don't (due to case 2). There's some test code attached that illustrates all this. Terry import types from twisted.internet import defer, reactor from twisted.python.util import mergeFunctionMetadata def altInlineCallbacks(f): def unwindGenerator(*args, **kwargs): deferred = defer.Deferred() try: result = f(*args, **kwargs) except Exception, e: deferred.errback(e) return deferred if isinstance(result, types.GeneratorType): return defer._inlineCallbacks(None, result, deferred) deferred.callback(result) return deferred return mergeFunctionMetadata(f, unwindGenerator) @altInlineCallbacks def f0(): # This causes mayhem with the normal inlineCallbacks class yes(): def send(x, y): print 'yes' return yes() @altInlineCallbacks def f1(): return 'hey' @altInlineCallbacks def f2(): raise Exception('whoops!') @altInlineCallbacks def f3(): if True: raise Exception('ouch!') d = defer.Deferred() reactor.callLater(0, d.callback, None) yield d @altInlineCallbacks def f4(): d = defer.Deferred() reactor.callLater(0, d.callback, None) yield d def ok(result, name): print '%s ok: %s' % (name, result) def fail(failure, name): print '%s fail: %s' % (name, failure) def stop(_): reactor.stop() def main(): L = [] for func in f0, f1, f2, f3, f4: name = func.__name__ L.append(func().addCallbacks( ok, fail, callbackArgs=[name], errbackArgs=[name])) d = defer.DeferredList(L) d.addBoth(stop) if __name__ == '__main__': reactor.callLater(0, main) reactor.run()
On Fri, Nov 21, 2008 at 1:04 PM, Terry Jones <terry@jon.es> wrote:
Here's a suggestion for making inlineCallbacks more consistent and less confusing. Let's suppose you're writing something like this:
@inlineCallbacks def func(): ....
result = func()
There are 2 things that could be better, IMO:
1. func may not yield. In that case, you either get an AttributeError when inlineCallbacks tries to send(). Or worse, the call to send might actually work, and do who knows what. I.e., func() could return an object with a send method but which is not a generator. For some fun, run some code that calls the following decorated function (see if you can figure out what will happen before you do):
@defer.inlineCallbacks def f(): class yes(): def send(x, y): print 'yes' # accidentally_destroy_the_universe_too() return yes()
Why not just return a Deferred from the function and not decorate it. Or document the function as returning a value if it doesn't block.
2. func might raise before it get to its first yield. In that case you'll get an exception thrown when the inlineCallbacks decorator tries to create the wrapper function:
File "/usr/lib/python2.5/site-packages/twisted/internet/defer.py", line 813, in unwindGenerator return _inlineCallbacks(None, f(*args, **kwargs), Deferred())
There's a simple and consistent way to handle both of these. Just have inlineCallbacks do some initial work based on what it has been passed:
def altInlineCallbacks(f): def unwindGenerator(*args, **kwargs): deferred = defer.Deferred() try: result = f(*args, **kwargs) except Exception, e: deferred.errback(e) return deferred if isinstance(result, types.GeneratorType): return defer._inlineCallbacks(None, result, deferred) deferred.callback(result) return deferred
return mergeFunctionMetadata(f, unwindGenerator)
Essentially this is equivalent to _not_ decorating your function and returning a Deferred via defer.succeed(value) or defer.fail(error). I'm still not sure why it's necessary to stitch this kind of behavior into inlineCallbacks(). From my perspective inlineCallbacks() simply equates: "I want to do more than one asynchronous operation inline and here's some syntactic sugar using yield as an expression."
This has the advantage that (barring e.g., a KeyboardInterrupt in the middle of things) you'll *always* get a deferred back when you call an inlineCallbacks decorated function. That deferred might have already called or erred back (corresponding to cases 1 and 2 above).
You will always get a Deferred back if the function is a generator.
I'm going to use this version of inlineCallbacks in my code. There's a case for it making it into Twisted itself: inlinecallbacks is already cryptic enough in its operation that anything we can do to make its operation more uniform and less surprising, the better.
You might think that case 1 rarely comes up. But I've hit it a few times, usually when commenting out sections of code for testing. If you accidentally comment out the last yield in func, it no longer returns a generator and that causes a different error.
That's what unit tests are for :) But I'm not sure how you could easily wind up in this scenario considering generators don't allow return with an argument. Drew
On 21 Nov, 06:04 pm, terry@jon.es wrote:
Here's a suggestion for making inlineCallbacks more consistent and less confusing. Let's suppose you're writing something like this:
Let's put this a bit less vaguely: inlineCallbacks appears to have a bug: 'raise' before 'yield' in a generator results in a synchronous exception rather than an errback, although its documentation does not explain this. inlineCallbacks is also unhelpful in debugging a particular type of error - it doesn't tell you what happened if you unintentionally returned something other than a generator, it just blows up without your code on the stack, and no mention of your code in the error message. These are definitely interesting problems. I think there should probably be a ticket for each one. I don't like your solution, though.
1. func may not yield. In that case, you either get an AttributeError when inlineCallbacks tries to send().
Following you so far.
Or worse, the call to send might actually work, and do who knows what. I.e., func() could return an object with a send method but which is not a generator.
Now I'm not sure what you're talking about. Do you have a lot of very dangerous objects lying around with methods called 'send'? ;-) This is an important behavior which should continue to be supported. If we don't, then users will get surprising behavior if they try to mix inlineCallbacks with other decorators that modify generators. A "generator-like" object (iterable with 'send') should be good enough.
For some fun, run some code that calls the following decorated function
In this particular case, there may be a third ticket to file, i.e. to refuse to accept objects that implement __call__ and send but not __iter__ and next; however, this infinite loop seems really, really obscure. Have you actually hit it in real life?
2. func might raise before it get to its first yield. In that case you'll get an exception thrown when the inlineCallbacks decorator tries to create the wrapper function:
Definitely problematic. Code expecting to handle errors with an errback should not need to have an except: block as well.
There's a simple and consistent way to handle both of these. Just have inlineCallbacks do some initial work based on what it has been passed:
isinstance() is bad for the reasons I mentioned above.
This has the advantage that (barring e.g., a KeyboardInterrupt in the middle of things) you'll *always* get a deferred back when you call an inlineCallbacks decorated function. That deferred might have already called or erred back (corresponding to cases 1 and 2 above).
This property, however, I definitely think is a desirable one.
And case 2 happens to me too. Having inlinecallbacks try/except the call to func is nicer because it means I don't have to be quite so defensive in coding. So instead of me having to write
try: d = func() except Exception: # ???
IMHO the most important thing discussed here. Not quite sure how to do this change in a compatible way; some people might be depending on this weird behavior. @inlineCallbacks2? @inlineCallbacks(beGood=YES)? Suggestions are appreciated.
On 03:18 am, glyph@divmod.com wrote:
On 21 Nov, 06:04 pm, terry@jon.es wrote:
Here's a suggestion for making inlineCallbacks more consistent and less confusing. Let's suppose you're writing something like this:
inlineCallbacks appears to have a bug: 'raise' before 'yield' in a generator results in a synchronous exception rather than an errback, although its documentation does not explain this.
Ugh, scratch that. No, it doesn't have this bug. When I read this:
2. func might raise before it get to its first yield. In that case you'll get an exception thrown when the inlineCallbacks decorator tries to create the wrapper function:
I did some quick testing and saw some tracebacks, but apparently wasn't paying very close attention to them; they were unhandled-traceback-in- deferred tracebacks, and they *did* have the application code in question on the stack. Run this and observe: from twisted.internet.defer import succeed, inlineCallbacks, returnValue @inlineCallbacks def f(): result = 1 / 0 yield returnValue(succeed(result)) print f().addErrback(lambda err: err.trap(ZeroDivisionError) and 2) If func "raises before it gets to its first yield", we get the right behavior. If it just raises and doesn't yield *anywhere*, then it's not a generator and that's the same as your other case of accidentally-not- returning-a-generator. Still worth debugging, but not as serious.
participants (3)
-
Drew Smathers -
glyph＠divmod.com -
Terry Jones