Clean story for error handling and basic debugging

src/tygs/app.py

@@ -4,7 +4,8 @@
 from path import Path
 
 from .components import SignalDispatcher
-from .utils import get_project_dir, ensure_awaitable, exception_handler_factory
+from .utils import (get_project_dir, ensure_awaitable, DebugException,
+                    silence_loop_error_log)
 
 
 class App:
@@ -17,6 +18,16 @@ def __init__(self, ns):
         self.main_future = None
         self.loop = asyncio.get_event_loop()
 
+    def fail_fast(self, on=False):
+        if on:
+            task_factory = DebugException.create_task_factory(self)
+            self.loop.set_task_factory(task_factory)
+        else:
+            if DebugException.fail_fast_mode:
+                self.loop.set_task_factory(DebugException.old_factory)
+                DebugException.old_factory = None
+                DebugException.fail_fast_mode = False
+
     def on(self, event):
         return self.components['signals'].on(event)
 
@@ -60,54 +71,78 @@ async def setup(self, cwd=None):
         return self.change_state('running')
 
     async def async_ready(self, cwd=None):
-        self.loop.set_exception_handler(exception_handler_factory(self))
         self.main_future = await asyncio.ensure_future(self.setup(cwd))
         return self.main_future
 
     def ready(self, cwd=None):
+
+        # If we are killed, try to gracefully exit
+        if self.loop.is_running():
+            raise RuntimeError("app.ready() can't be called while an event"
+                               ' loop is running, maybe you want to call '
+                               '"await app.async_ready()" instead?')
+
+        if self.loop.is_closed():
+            raise RuntimeError("app.ready() can't be called while a "
+                               ' closed event loop. Please install a fresh'
+                               ' one with policy.new_event_loop() or make '
+                               "sure you don't close it by mistake")
+
         for signame in ('SIGINT', 'SIGTERM'):
             self.loop.add_signal_handler(getattr(signal, signame),
                                          self.stop)
+
         clean = False  # do not stop cleanly if the user made a mistake
         try:
-            fut = asyncio.ensure_future(self.async_ready(cwd))
+            # Main loop, most of the work happens here
+            self.main_future = asyncio.ensure_future(self.async_ready(cwd))
             clean = True
             self.loop.run_forever()
-        except RuntimeError as e:
-            if self.loop.is_running():
-                try:
-                    fut.cancel()
-                except NameError:  # noqa
-                    pass
-                raise RuntimeError("app.ready() can't be called while an event"
-                                   ' loop is running, maybe you want to call '
-                                   '"await app.async_ready()" instead?') from e
-            else:
-                raise RuntimeError("app.ready() can't be called while a "
-                                   ' closed event loop. Please install a fresh'
-                                   ' one with policy.new_event_loop() or make '
-                                   "sure you don't close it by mistake") from e
+
+        except DebugException as e:
+            clean = False
+            raise e.from_exception
+
+        # On Ctrl+C, try a clean stop. Yes, we need it despite add_signal_handler
+        # for an unknown reason.
+        except KeyboardInterrupt:
+            self.stop()
+
+        # stop() is just setting a state and asking the loop to stop.
+        # In the finally, we call _finish(), which on clean exit will run
+        # callbacks register to the "stop" event force stop the loop and
+        # close it.
         finally:
             if clean:
-                self._stop()
+                self._finish()
 
     async def async_stop(self):
         return await self.change_state('stop')
 
-    def stop(self, error=None):
+    def stop(self):
         """
         Stops the loop, which will trigger a clean app stop later.
         """
-        if self.loop.is_running():
+        if self.state == 'running':
             self.state = 'stopping'
-            self.loop.stop()
-            if error is not None:
-                raise error()
+            if self.loop.is_running():
+                # This stops the loop, and activate ready()'s finally which
+                # will enventually call self._stop().
+                self.loop.stop()
+
+    def break_loop_with_error(self, msg, exception=RuntimeError):
+        # Silence other exception handlers, since we want to break
+        # everything.
+        with silence_loop_error_log(self.loop):
+            raise DebugException(exception(msg))
+
+    def _finish(self):
 
-    def _stop(self, timeout=5):
         if self.state != "stop":
             if self.state != 'stopping':
-                self.loop.stop()
+                # TODO: it would be better to be able to see RuntimeError
+                # directly but we can't figure how to do it now
+                self.break_loop_with_error("Don't call _finish() directly. Call stop()")
             self.state = 'stop'
             self.loop.run_until_complete(self.async_stop())
             self.loop.close()

src/tygs/utils.py

@@ -3,6 +3,8 @@
 import asyncio
 import inspect
 
+import contextlib
+
 from path import Path
 
 
@@ -45,15 +47,42 @@ def aiorun(callable_obj):
     return loop.run_until_complete(awaitable)
 
 
-def clean_exceptions_cb(fut):
-    exc = fut.exception()
-    if exc is not None:
-        fut._loop.stop()
-        raise exc
+class DebugException(BaseException):
+
+    old_factory = None
+    fail_fast_mode = False
+
+    def __init__(self, from_exception):
+        self.from_exception = from_exception
+
+    @classmethod
+    def create_task_factory(cls, app):
+        """
+        Surcharge the default asyncio Task factory, by adding automatically an
+        exception handler callback to every Task.
+        Returning a factory instead of a Task prevents loop.get_task_factory() to
+        be called for each Task.
+
+        If you want to set up your own Task factory, make sure to call this one
+        too, or you'll lose Tygs Task exceptions handling.
+        """
+
+        cls.fail_fast_mode = True
+        loop = app.loop
+        old_factory = loop.get_task_factory() or asyncio.Task
+
+        def factory(loop, coro):
+            task = old_factory(loop=loop, coro=coro)
+            task.set_exception = app.break_loop_with_error
+            return task
+
+        return factory
+
 
+@contextlib.contextmanager
+def silence_loop_error_log(loop):
+    old_handler = loop._exception_handler
+    loop.set_exception_handler(lambda loop, context: None)
+    yield
+    loop.set_exception_handler(old_handler)
 
-def exception_handler_factory(app):
-    def exception_handler(loop, context):
-        loop.default_exception_handler(loop, context)
-        app.stop(context.get('exception'))
-    return exception_handler

tests/test_app.py

@@ -8,6 +8,7 @@
 
 from tygs.components import SignalDispatcher
 from tygs.test_utils import AsyncMock
+from tygs.utils import aioloop as get_loop, DebugException
 
 
 def test_basic_api(app):
@@ -88,26 +89,33 @@ async def test_ready_in_loop(app):
 # the previous one
 def test_ready_keyboard_interrupt(aioloop, app):
     beacon = Mock()
-    app._stop = Mock()
+    real_stop = app._finish
+
+    def beacon_stop():
+        beacon()
+        real_stop()
+
+    app._finish = beacon_stop
 
     @app.on('running')
     def stahp():
         beacon()
         raise KeyboardInterrupt()
 
-    try:
-        app.ready()
-    except KeyboardInterrupt:
-        pass
-    beacon.assert_called_once_with()
-    app._stop.assert_called_once_with()
+    @app.on('stop')
+    def stop():
+        beacon()
+
+    app.ready()
+
+    assert beacon.call_count == 3
 
 
 def test_ready_sigterm():
     shell = subprocess.Popen([sys.executable, 'tests/tygs_process'],
                              stdout=subprocess.PIPE,
                              stderr=subprocess.PIPE)
-    time.sleep(.5)
+    time.sleep(1)
     shell.terminate()
     stderr = shell.stderr.read()
     return_code = shell.wait()
@@ -150,15 +158,38 @@ def test_ready_closed_loop(aioloop, app):
         app.ready()
 
 
-def test_dirty_stop(aioloop, app):
+def test_ready_running_loop(aioloop, app):
+
+    async def start():
+        app.ready()
+
+    with pytest.raises(RuntimeError):
+        aioloop.run_until_complete(start())
+
+
+def test_forbid_finish(aioloop, app):
 
     @app.on('running')
     def stahp():
-        with pytest.raises(RuntimeError):
-            app._stop()
+        app._finish()
+
+    with pytest.raises(RuntimeError):
+        app.ready()
+
+
+def test_stop(aioloop, app):
+
+    @app.on('running')
+    def stop():
+        app.stop()
 
     app.ready()
 
+    # calling _finish() after stop() is a no op
+    app._finish()
+    # calling stop() again after stop() is a no op
+    app.stop()
+
 
 @pytest.mark.asyncio
 async def test_stop_twice(app):
@@ -167,10 +198,54 @@ async def test_stop_twice(app):
     app.stop()
 
 
-def test_runtime_error(aioloop, app):
+def test_fail_fast_mode(aioloop, app):
+
+    with pytest.raises(Exception):
+        app.fail_fast(True)
+
+        @app.on('running')
+        def stahp():
+            raise Exception('Breaking the loop')
+
+        app.ready()
+
+
+def test_fail_fast_mode_disable(aioloop, app):
+
+    app.fail_fast(True)
+    assert DebugException.fail_fast_mode
 
     @app.on('running')
     def stahp():
         raise Exception('Breaking the loop')
 
+    with pytest.raises(Exception):
+        app.ready()
+
+    app.fail_fast(False)
+    assert DebugException.old_factory is None
+    assert not DebugException.fail_fast_mode
+
+    app.fail_fast(False)  # this is a noop
+    assert DebugException.old_factory is None
+    assert not DebugException.fail_fast_mode
+
+    # refresh the loop
+    app.loop = get_loop()
+
+    # TODO: write an app.restart() method ?
+
+    @app.on('running')
+    def non_stop():
+        raise Exception('Breaking the loop')
+
+    @app.on('running')
+    def stop():
+        app.stop()
+
     app.ready()
+
+
+
+
+

tests/test_utils.py

@@ -67,3 +67,13 @@ async def test():
 
     with pytest.raises(TypeError):
         a = utils.ensure_awaitable("test()")
+
+
+def test_silence_loop_error_log(aioloop):
+    assert aioloop._exception_handler is None
+    with utils.silence_loop_error_log(aioloop):
+        assert aioloop._exception_handler.__name__ == '<lambda>'
+        assert aioloop._exception_handler(None, None) is None
+        assert aioloop._exception_handler(1, {}) is None
+
+    assert aioloop._exception_handler is None