disconnect gracefully in an unexpected exit

[das7pad]

The UI currently does not cancel the long-polling request on an unexpected error, for example a SystemExit or KeyboardInterrupt seen in #336.
The cleanup is then done by the garbage collector of python, which does not play nice with asyncio.

Running hangups.client.Client.disconnect would raise the KeyboardInterrupt or SystemExit again, we need to discard a raised Exception manually.

[tdryer]

To avoid the protected access, could we instead call self._listen_future.result() and catch the exception?

[das7pad]

A broad except is needed then. dde895c solves this without a raised Exception.

[das7pad]

Caught KeyboardInterrupt, exiting abnormally
Exception ignored in: <bound method Task.del of <Task pending coro=<<aiohttp.client._RequestContextManager object at 0x7f42a2bd90a8>() running at <...>/hangups/http_utils.py:76> wait_for= cb=[_release_waiter(<Future pendi...sk._wakeup()]>)() at /usr/local/lib/python3.5/asyncio/tasks.py:358]>>
Traceback (most recent call last):
File "/usr/local/lib/python3.5/asyncio/tasks.py", line 92, in del
File "/usr/local/lib/python3.5/asyncio/base_events.py", line 1289, in call_exception_handler
File "/usr/local/lib/python3.5/logging/init.py", line 1309, in error
File "/usr/local/lib/python3.5/logging/init.py", line 1416, in _log
File "/usr/local/lib/python3.5/logging/init.py", line 1426, in handle
File "/usr/local/lib/python3.5/logging/init.py", line 1488, in callHandlers
File "/usr/local/lib/python3.5/logging/init.py", line 856, in handle
File "/usr/local/lib/python3.5/logging/init.py", line 1048, in emit
File "/usr/local/lib/python3.5/logging/init.py", line 1038, in _open
NameError: name 'open' is not defined
Exception ignored in: <bound method Task.del of <Task pending coro=<Conversation.send_message() running at <...>/hangups/conversation.py:446> wait_for= cb=[ConversationWidget._on_message_sent()]>>
Traceback (most recent call last):
File "/usr/local/lib/python3.5/asyncio/tasks.py", line 92, in del
File "/usr/local/lib/python3.5/asyncio/base_events.py", line 1289, in call_exception_handler
File "/usr/local/lib/python3.5/logging/init.py", line 1309, in error
File "/usr/local/lib/python3.5/logging/init.py", line 1416, in _log
File "/usr/local/lib/python3.5/logging/init.py", line 1426, in handle
File "/usr/local/lib/python3.5/logging/init.py", line 1488, in callHandlers
File "/usr/local/lib/python3.5/logging/init.py", line 856, in handle
File "/usr/local/lib/python3.5/logging/init.py", line 1048, in emit
File "/usr/local/lib/python3.5/logging/init.py", line 1038, in _open
NameError: name 'open' is not defined

A KeyboardInterrupt while sending a message.

[das7pad]

The fix in 1348c20 has a hardcoded delay of 3 seconds for resources (especially connections) to be closed internally after sending.
Setting the delay to 0.1 was already sufficient on my hosts to destruct the ssl-connections gracefully, but a busy/very slow host might need more time to do so - see the second part of aio-libs/aiohttp#1925 (comment) and further comments.

[tdryer]

Could we cancel the task instead of waiting for it to complete?

The UI can start some other tasks as well. As a more general solution, could we keep track of them all and cancel each one on exit?

This patch also seems to work for turning KeyboardInterrupt into a graceful exit.

[das7pad]

The loop will likely stop before the cleanup of resources used inside the tasks finished and finally branches completed. A common traceback is shown above - the garbage collector cleaned up in a wrong sequence and resources are missing; another one could be caused by a call on loop.call_exception_handler after the loop got closed.
In addition a task spawned by asyncio.shield(<coro>) might not complete. (which the hangups core does not use)

If the sequence of completion does not matter, asyncio.gather might be useful:

tasks = [<one>, <two>, ...]
# run 
for task in tasks:
    tasks.cancel()
await asyncio.gather(*tasks, return_exceptions=True)

The diff works great for the KeyboardInterrupt case, but still ignores a low-level error. A low-level error could be caused by a change in aiohttp which we support in a wide range of versions, however not all of them are completely backwardscompatible and could raise an Exception. The UI/Terminal should stay clean and any traceback better goes into the log.

[tdryer]

The loop will likely stop before the cleanup of resources used inside the tasks finished and finally branches completed.

Is that a problem if we re-start the loop and wait for the cancelled tasks to finish?

The diff works great for the KeyboardInterrupt case, but still ignores a low-level error. A low-level error could be caused by a change in aiohttp which we support in a wide range of versions, however not all of them are completely backwardscompatible and could raise an Exception. The UI/Terminal should stay clean and any traceback better goes into the log.

I'd actually prefer unexpected exceptions to exit hangups and print a traceback. That makes it obvious when there's a bug.

[das7pad]

Is that a problem if we re-start the loop and wait for the cancelled tasks to finish?

re-start at which point? multiple loop.run_until_complete() are no problem.
After loop.close() there is no way back and pending or scheduled tasks are discard.

I'd actually prefer unexpected exceptions to exit hangups and print a traceback. That makes it obvious when there's a bug.

Ok. The traceback of the raised low-level Exception will reach the terminal and hangups will exit, but the Exceptions or resource warnings this forceful exit would result in, will not be raised as we wait for and close the resources graceful in the recent commits.

[tdryer]

Started working on another approach in #378.

[tdryer]

Superseded by #378.