When callback throws an exception, callback's (wrongly) called again about brukva

[drdaeman]

Possible solution: drdaeman/brukva@de1c52fae9b3045e4b75c465c77a26bddd4bb259 (the code somehow feels dirty, though)

[kmike]

It seems more clear to catch only brukva's exceptions (not all exceptions) at forward_error context manager. Inherit brukva exceptions from 1 base class and wrap other exceptions (IOError?) into brukva's exception if it is necessary.

Though I must admin I don't fully understand how error handling works now: why is exceptions passed back to callbacks and what are these callbacks. Can somebody provide high-level description on what is error handling supposed to be? This description can be put into source code comments then.

[evilkost]

2drdaeman
Yep, my bad. Callbacks was wrongly called with exception, because in function like Client.execute self.call_callbacks... was placed inside 'with' statement. Simple fix: call_callbacks shift outside from with exception.
One special case is Client.listen, so i added disable&enable method to forward_error context (FYI: evilkost/brukva@3d15814e8 )

2kmike
About error handling. I wanted to eliminate code like this:

...
(error, value) = yield client.async.get('foo')
if error:
    handle_error(error)
else:
    (error, other_value) = yield client.async.get('bar')
    if error:
        handle_error(error)
...

I patched brukva.adisp:110-113 6f5514c1 to throw exception from generator if value is instance of Exception.
So now, it look much better:

...
try:
    value = yield client.async.get('foo')
    other_value = yield client.async.get('bar')
except Exception, e:
    handle_error(error)
...

And with context manager forward_error is just syntax sugar:

...
with forward_error(handle_error):
    value = yield client.async.get('foo')
    other_value = yield client.async.get('bar')   
...

[drdaeman]

Simple fix: call_callbacks shift outside from with exception.

No, just shifting the indentation won't do it. While this fixes the original bug, it'll introduce another one — a callback will be called twice, first with the exception, then with some "failure" result.

This is debatable, but I believe callback're expected to be called only once per request — unless a command is a special case like listen. Therefore call_callbacks should be normally called only when no exception occured. This is why I used the try...except...else. An example illustrating the problem:

#!/usr/bin/env python

import contextlib

@contextlib.contextmanager
def chew_error(callback):
    try:
        yield callback
    except Exception, e:
        callback(e)

def callback(result):
    print("Got {0!r}".format(result))

def example():
    result = "failure"
    with chew_error(callback):
        raise Exception, "exception"
        result = "ok"
    callback(result)

if __name__ == "__main__":
    example()

[evilkost]

Oops.
Although, if shift back callback calling inside with and restrict error handling, example will be working.
But i think, it's not clean.

def callback(result):
    print("Got {0!r}".format(result))

def example():
    result = "failure"
    with forward_error(callback) as forward:
        raise Exception, "exception"
        result = "ok"
        forward.disable() # restrict error handling
        callback(result)

Or add special method to forward_context

class ForwardErrorManager(object):
    def ret(self, result):
        self.disable()
        self.callbacks(result)
        self.enable()


def example():
    result = "failure"
    with forward_error(callback) as forward:
        raise Exception, "exception"
        result = "ok"
        forward.ret(result)

I should think for some time.