Repository: MaineKuehn/usim

Issue:
Scopes swallow exceptions during shutdown
**Description of the bug**
If an activity receives an exception *during* scope shutdown (``__aexit__``) it is suppressed. The cause is that ``Scope.__aexit__ -> False`` is assumed to re-raise the *current* exception, but it only re-raises the exception with which the scope was entered.

**How To Reproduce**
In the following example, ``suppressing`` should be killed by a CancelTask. The ``usim.instant`` causes this to be received *during* Scope shutdown.

```
async def suppressing():
    async with usim.Scope() as scope:
        ...

async def setup():
    async with usim.Scope() as scope:
        task = scope.do(suppressing())
        await usim.instant  # advance suppressing Scope to shutdown
        task.cancel()
    await task

usim.run(setup())
```

**Expected behaviour**
The ``Scope`` should raise the most recent exception.

Discussion:

Patch:
diff --git a/docs/source/changes/85.early_task_cancellation.yaml b/docs/source/changes/85.early_task_cancellation.yaml
new file mode 100644
index 0000000..0a4219c
--- /dev/null
+++ b/docs/source/changes/85.early_task_cancellation.yaml
@@ -0,0 +1,7 @@
+category: fixed
+summary: "Cancelling a Task early no longer cancels its parent scope"
+description: |
+  Cancelling a :py:class:`~usim.Task` was incorrectly flagged as a failure of the task.
+  This caused the parent :py:class:`~usim.Scope` to abort as well.
+pull requests:
+- 85
diff --git a/docs/source/changes/85.scope_shutdown_suppression.yaml b/docs/source/changes/85.scope_shutdown_suppression.yaml
new file mode 100644
index 0000000..2bdff80
--- /dev/null
+++ b/docs/source/changes/85.scope_shutdown_suppression.yaml
@@ -0,0 +1,11 @@
+category: fixed
+summary: "Scopes no longer swallow exceptions during graceful shutdown"
+description: |
+  When a :py:class:`~usim.Scope` received a signal during graceful shutdown,
+  its fast shutdown path attempted to propagate the signal incorrectly.
+  Such late signals as well as concurrent exceptions are now explicitly
+  handled and propagated out of the scope as needed.
+issues:
+- 83
+pull requests:
+- 85
diff --git a/usim/_primitives/context.py b/usim/_primitives/context.py
index 4f2b65f..ae9a742 100644
--- a/usim/_primitives/context.py
+++ b/usim/_primitives/context.py
@@ -1,7 +1,7 @@
 from typing import Coroutine, List, TypeVar, Any, Optional, Tuple
 
 from .._core.loop import __LOOP_STATE__, Interrupt as CoreInterrupt
-from .notification import Notification, postpone
+from .notification import Notification
 from .flag import Flag
 from .task import Task, TaskClosed, TaskCancelled, try_close
 from .concurrent_exception import Concurrent
@@ -238,61 +238,32 @@ class Scope:
         return self
 
     async def __aexit__(self, exc_type, exc_val, exc_tb) -> bool:
+        # there was no exception, we regularly exited the loop body
+        # we wait for our children to finish or some interrupt to happen
         if exc_type is None:
             try:
                 # inform everyone that we are shutting down
                 # we may receive any shutdown signal here
                 await self._body_done.set()
+                await self._await_children()
             except BaseException as err:
-                exc_type, exc_val = type(err), err
-            else:
-                return await self._aexit_graceful()
-        self._body_done._value = True
-        self._body_done.__trigger__()
-        # there was an exception, we have to abandon the scope
-        return self._aexit_forceful(exc_type, exc_val)
-
-    def _aexit_forceful(self, exc_type, exc_val) -> bool:
-        """
-        Exit with exception
-
-        This immediately closes all children without waiting for anything.
-        No further interrupts can occur during shutdown (this is a sync function).
+                self._close_scope()
+                if self._propagate_exceptions(type(err), err):
+                    raise
+                return True
+        # there was an exception, we have to abandon the scope fast
+        # we do not want interrupts that conflict with our current exception
+        else:
+            self._body_done._value = True
+            self._body_done.__trigger__()
+        self._close_scope()
+        return not self._propagate_exceptions(exc_type, exc_val)
 
-        If a fatal exception occurred in this scope or a child, the fatal
-        exception replaces all other exceptions in flight.
-        If the current exception is suppressed by the scope, any exceptions from
-        children are reraised as :py:exc:`~.Concurrent` errors.
-        """
-        # we already handle an exception, suppress
+    def _close_scope(self):
+        """Ultimately close the scope, its interrupts and all children"""
         self._disable_interrupts()
-        # reap all children now
         self._close_children()
         self._close_volatile()
-        return self._propagate_exceptions(exc_type, exc_val)
-
-    async def _aexit_graceful(self) -> bool:
-        """
-        Exit without exception
-
-        This suspends the scope until all children have finished themselves.
-        If any children encountered an error, they are reraised as
-        :py:exc:`~.Concurrent` errors.
-
-        If an error occurs while waiting for children to stop, a forceful
-        shutdown is performed instead.
-        """
-        try:
-            await self._await_children()
-            # we must always postpone to receive signals, even if there are no children
-            await postpone()
-        except BaseException as err:
-            return self._aexit_forceful(type(err), err)
-        else:
-            # everybody is gone - we just handle the cleanup
-            self._disable_interrupts()
-            self._close_volatile()
-            return self._propagate_exceptions(None, None)
 
     def _collect_exceptions(self)\
             -> Tuple[Optional[BaseException], Optional[Concurrent]]:
@@ -322,25 +293,25 @@ class Scope:
             return None, exc
         return None, None
 
-    def _propagate_exceptions(self, exc_type, exc_val):
+    def _propagate_exceptions(self, exc_type, exc_val) -> bool:
         if exc_type in self.PROMOTE_CONCURRENT:
             # we already have a privileged exception, there is nothing more important
             # propagate it
-            return False
-        elif self._is_suppressed(exc_val):
+            return True
+        elif self._is_suppressed(exc_val) or exc_type is None:
             # we do not have an exception to propagate, take whatever we can get
             privileged, concurrent = self._collect_exceptions()
             if privileged is not None or concurrent is not None:
                 raise privileged or concurrent
             # we handled our own and there was nothing else to propagate
-            return True
+            return False
         else:
             # we already have an exception to propagate, take only important ones
             privileged, _ = self._collect_exceptions()
             if privileged is not None:
                 raise privileged
             # we still have our unhandled exception to propagate
-            return False
+            return True
 
     def _is_suppressed(self, exc_val) -> bool:
         """
diff --git a/usim/_primitives/task.py b/usim/_primitives/task.py
index 0bb4c2a..0028786 100644
--- a/usim/_primitives/task.py
+++ b/usim/_primitives/task.py
@@ -118,7 +118,7 @@ class Task(Awaitable[RT]):
             # check for a pre-run cancellation
             if self._result is not None:
                 try_close(self.payload)
-                self.parent.__child_finished__(self, failed=True)
+                self.parent.__child_finished__(self, failed=False)
                 return
             try:
                 # We suspend the Task internally instead of waiting to start


Test patch:
diff --git a/usim_pytest/test_types/test_scope.py b/usim_pytest/test_types/test_scope.py
index 58cfe58..6565ab3 100644
--- a/usim_pytest/test_types/test_scope.py
+++ b/usim_pytest/test_types/test_scope.py
@@ -2,7 +2,7 @@ import inspect
 
 import pytest
 
-from usim import Scope, time, instant, Concurrent
+from usim import Scope, time, instant, eternity, Concurrent, TaskCancelled
 
 from ..utility import via_usim, assertion_mode
 
@@ -164,6 +164,32 @@ class TestExceptions:
             except Concurrent[Concurrent[IndexError]]:
                 assert False
 
+    @via_usim
+    async def test_fail_interrupt_shutdown(self):
+        """Interrupts during shutdown are not suppressed"""
+        async def bare_scope():
+            await instant  # allow the task to postpone before we cancel it
+            # a bare Scope postpones twice:
+            # 1) signalling body done
+            # 2) waiting for children (even if there are none)
+            async with Scope():
+                ...
+            await eternity  # receive late cancellations
+
+        async with Scope() as scope:
+            # create *more* tasks than we expect postponements (3)
+            # so that we cover every interrupt point in case the
+            # implementation changes
+            tasks = [scope.do(bare_scope()) for _ in range(5)]
+            for task in tasks:
+                assert not task.done
+                task.cancel()
+                await instant
+
+        for task in tasks:
+            with pytest.raises(TaskCancelled):
+                await task
+
 
 class TestScoping:
     @via_usim