Exceptions in asynchronous property setting not propagated to the user code

[Jajcus]

Hi,

When trying to interact with Bluez using dbussy/ravel I found out that errors in asynchronous property setting cannot be properly handled, instead I get error message:

ERROR:asyncio:Task exception was never retrieved
future: <Task finished coro=<def_proxy_interface.<locals>.def_prop.<locals>.set_prop.<locals>.sendit() done, defined at /usr/local/lib/python3.7/dist-packages/ravel.py:2967> exception=DBusError('org.bluez.Error.Failed -- Not Powered')>
Traceback (most recent call last):
 File "/usr/local/lib/python3.7/dist-packages/ravel.py", line 2977, in sendit
    raise dbus.DBusError(reply.error_name, reply.expect_objects("s")[0])
      dbussy.DBusError: org.bluez.Error.Failed -- Not Powered

In this case I would like to ignore this specific exception, but I cannot, as it is never propagated to my code. That is because the error is raised from the sendit() coroutine, which is started as an untracked task.

I can see two possible solutions to that:

• instead of raising the exception sendit() could set it on the set_prop_pending future (instead of setting its result to None)
• alternatively the sendit() tasks could be stored instead of the set_prop_pending futures – that can be awaited the same way

[ldo]

Hmm, this needs some thinking about. It is fine for the caller to forget about successful async property-setting calls, but what about unsuccessful ones? What happens if there is more than one unsuccessful one?

I have just added a mechanism so these exceptions are not simply thrown away. I keep at most one exception so a set_prop_flush() call can raise it back to the caller; any subsequent ones get raised within sendit() as at present. This is to keep such exceptions from accumulating without bounds.

Maybe a bit of a hacky solution; let me know what you think.

[Jajcus]

When caller wants to handle all exceptions:

while True:
    try:
        await set_prop_flush(interface)
    except Exception as exc:
        handle_the_exception()
    else:
        break

This will handle each of the exceptions raised in the queued property set tasks.

And even when on only uses single set_prop_flush() call raising the first exception here and not waiting for the remaining properties to be set is, in my opinion, better than just ignoring the exceptions – the error will be caught immediately, the program may exit, as on regular exception in a regular non-async program.

[ldo]

That would require all the exceptions to be queued up before getting to the set_prop_flush() calls; if the caller never makes those calls, when are the objects disposed?

[Jajcus]

Oh… the code is not very clear, so I assumed all the pending calls are queued in _set_prop_pending until set_prop_flush is called. No I see it is not the case (sendit() removes them from the queue), so my proposition would be, indeed, a regression in this case (would make calling set_prop_flush() mandatory).

I guess your solution is good enough, though I would prefer explicit waiting for all the calls. Now, to handle each possible exception I will need to call set_prop_flush() after setting each property separately.

Another option would be to allow passing own exception handler to def_proxy_interface() and get_proxy_interface(), that would be invoked by sendit().

[ldo]

The trouble is there is (currently) no way in Python to await the completion of an asynchronous assignment.

I think the only answer is to add a separate async set_prop() method call, and forget trying to make it look like an assignment.

[ldo]

OK, I have added a new set_prop() method. So as an alternative to

server[path].prop = newvalue

and then having to do a separate

await server.set_prop_flush()

You can now write

await server.set_prop(path, prop, newvalue)

and do it all in one. (Or save the returned Future and check it later.)

[Jajcus]

Yes, that is great. But the .prop=value with set_prop_flush() should still stay – it can make code clearer in some cases.

[ldo]

Sure. I won’t remove it.

[ldo]

Anything further on this issue?

[Jajcus]

No, the two changes you made (exception handling and new set_prop()) look ok. Though, I have not tried them yet.