Repository: MaineKuehn/usim

Issue:
Concurrent Exception Cascade is ill-defined
**Description of the bug**
A ``Concurrent`` Exception may not contain another ``Concurrent`` Exception. If one ``Scope`` failure cascades into another, this breaks error handling.

**How To Reproduce**
```python3
from usim import Scope, run

async def outer():
    async with Scope() as scope:
        scope.do(inner())

async def inner():
    async with Scope() as scope:
        scope.do(async_raise(KeyError))

async def async_raise(e):
    raise e

run(outer())
```

This fails with ``AssertionError: 'Concurrent' may only be specialised by Exception subclasses``.

**Expected behavior**
Cascading ``Concurrent`` Exceptions should be well-defined. Either allow ``Concurrent`` to contain itself, or flatten contained exceptions.

Discussion:

Patch:
diff --git a/docs/source/topics/exceptions.rst b/docs/source/topics/exceptions.rst
index e55e3af..551c495 100644
--- a/docs/source/topics/exceptions.rst
+++ b/docs/source/topics/exceptions.rst
@@ -201,6 +201,28 @@ Handling Multiple Exceptions
     Finally, we recommend using ``Concurrent[...]`` only if you want to suppress
     concurrent exceptions unconditionally.
 
+Propagating Exceptions Best-Practices
+-------------------------------------
+
+.. content-tabs:: left-col
+
+    Whether an :term:`activity` uses concurrency or not is usually not obvious.
+    Unless explicitly documented, a :py:exc:`~usim.Concurrent` exception propagating
+    out of an :term:`activity` is likely unexpected.
+    As such, avoid propagating :py:exc:`~usim.Concurrent` exceptions whenever possible!
+
+    All abstractions in μSim convert internal :py:exc:`~usim.Concurrent` exceptions
+    to regular exceptions.
+    Only :py:class:`~usim.Scope` and derived types may raise
+    :py:exc:`~usim.Concurrent` exceptions when exiting an ``async with`` context.
+    The intention is to hide implementation details,
+    while allowing full control on expected concurrency.
+
+    We strongly recommend to similarly avoid propagating
+    :py:exc:`~usim.Concurrent` exceptions if possible.
+    When unavoidable, do not expose nested concurrency but
+    propagate a :py:meth:`~usim.Concurrent.flattened` exception.
+
 
     .. [#debug] For the use of :py:exc:`AssertionError` by μSim,
                 see also :doc:`./debug`.
diff --git a/usim/_primitives/concurrent_exception.py b/usim/_primitives/concurrent_exception.py
index 02ab97c..b64bb04 100644
--- a/usim/_primitives/concurrent_exception.py
+++ b/usim/_primitives/concurrent_exception.py
@@ -14,7 +14,7 @@ class MetaConcurrent(type):
     # metaclass instance fields - i.e. class fields
     # used to define specialisations of a base type
     inclusive: bool
-    specialisations: Optional[Tuple[Type[Exception]]]
+    specialisations: 'Optional[Tuple[Type[Union[Concurrent, Exception]], ...]]'
     template: 'MetaConcurrent'
     __specialisations__: WeakValueDictionary
 
@@ -34,7 +34,7 @@ class MetaConcurrent(type):
         bases: Tuple[Type, ...],
         namespace: Dict[str, Any],
         specialisations:
-            Optional[Tuple[Type[Exception], ...]] = None,
+            'Optional[Tuple[Type[Union[Concurrent, Exception]], ...]]' = None,
         inclusive: bool = True,
         **kwargs,
     ):
@@ -142,8 +142,9 @@ class MetaConcurrent(type):
         item:  # [Exception] or [...] or [Exception, ...]
             Union[
                 Type[Exception],
+                'Type[Concurrent]',
                 'ellipsis',
-                Tuple[Union[Type[Exception], 'ellipsis'], ...]
+                'Tuple[Union[Type[Concurrent], Type[Exception], "ellipsis"], ...]',
             ]
     ):
         """``cls[item]`` - used to specialise ``cls`` with ``item``"""
@@ -157,15 +158,19 @@ class MetaConcurrent(type):
             return cls
         # Cls[item]
         elif type(item) is not tuple:
-            assert issubclass(item, Exception),\
-                f'{cls.__name__!r} may only be specialised by Exception subclasses'
+            assert issubclass(item, (Exception, cls)), (
+                f'{cls.__name__!r} may only be specialised by Exception subclasses, '
+                f'not {item}'
+            )
             item = (item,)
         # Cls[item1, item2]
         else:
             assert all(
-                (child is ...) or issubclass(child, Exception) for child in item
-            ),\
-                f'{cls.__name__!r} may only be specialised by Exception subclasses'
+                (child is ...) or issubclass(child, (Exception, cls)) for child in item
+            ), (
+                f'{cls.__name__!r} may only be specialised by Exception subclasses, '
+                f'not {item}'
+            )
         return cls._get_specialisation(item)
 
     def _get_specialisation(cls, item):
@@ -182,9 +187,12 @@ class MetaConcurrent(type):
         except KeyError:
             inclusive = ... in unique_spec
             specialisations = tuple(child for child in unique_spec if child is not ...)
-            spec = ", ".join(  # the specialisation string "KeyError, IndexError, ..."
+            # the specialisation string "KeyError, IndexError, ..."
+            spec = ", ".join(
                 child.__name__ for child in specialisations
             ) + (', ...' if inclusive else '')
+            # the specialised subclass, as in
+            #
             # class 'cls.__name__[spec]'(cls, specialisations, inclusive):
             #   pass
             #
@@ -284,11 +292,11 @@ class Concurrent(BaseException, metaclass=MetaConcurrent):
     #: Basic template of specialisation
     template: ClassVar[MetaConcurrent]
     #: Exceptions that occurred concurrently
-    children: Tuple[Exception, ...]
+    children: 'Tuple[Union[Concurrent, Exception], ...]'
 
     # __new__ automatically specialises Concurrent to match its children.
     # Concurrent(A(), B()) => Concurrent[A, B](A(), B())
-    def __new__(cls: 'Type[Concurrent]', *children: Exception):
+    def __new__(cls: 'Type[Concurrent]', *children: 'Union[Concurrent, Exception]'):
         if not children:
             assert cls.specialisations is None,\
                 f"specialisation {cls.specialisations} does not match"\
@@ -297,7 +305,7 @@ class Concurrent(BaseException, metaclass=MetaConcurrent):
         special_cls = cls[tuple(type(child) for child in children)]
         return super().__new__(special_cls)
 
-    def __init__(self, *children: Exception):
+    def __init__(self, *children: 'Union[Concurrent, Exception]'):
         super().__init__(children)
         self.children = children
 
@@ -309,3 +317,25 @@ class Concurrent(BaseException, metaclass=MetaConcurrent):
         return \
             f'<object usim.{self.__class__.__name__} '\
             f'of {", ".join(map(repr, self.children))}>'
+
+    def flattened(self) -> 'Concurrent':
+        """
+        Collapse nested Concurrent exceptions
+
+        Recursively collapses nested ``Concurrent`` exceptions to provide a single
+        ``Concurrent`` exception containing all :py:attr:`~.children` of the hierarchy.
+        For example, flattening a ``Concurrent(Concurrent(KeyError()), IndexError())``
+        provides a ``Concurrent(KeyError(), IndexError())``.
+        """
+        if not any(isinstance(exc, Concurrent) for exc in self.children):
+            return self
+        leafs = []
+        for child in self.children:
+            if isinstance(child, Concurrent):
+                leafs.extend(child.flattened().children)
+            else:
+                leafs.append(child)
+        flat = Concurrent(*leafs)
+        flat.__cause__ = self.__cause__
+        flat.__context__ = self.__context__
+        return flat


Test patch:
diff --git a/usim_pytest/test_types/test_concurrent_exception.py b/usim_pytest/test_types/test_concurrent_exception.py
index bbb209b..b85be77 100644
--- a/usim_pytest/test_types/test_concurrent_exception.py
+++ b/usim_pytest/test_types/test_concurrent_exception.py
@@ -10,6 +10,11 @@ class FakeConcurrent:
 
 class TestConcurrent:
     """Test the semantics of ``usim.Concurrent``"""
+    def test_misuse(self):
+        base_case = Concurrent[KeyError]
+        with pytest.raises(TypeError):
+            base_case[IndexError]
+
     def test_generic(self):
         """Generic ``Concurrent`` is a catch-all"""
         with pytest.raises(Concurrent):
@@ -126,3 +131,52 @@ class TestConcurrent:
         assert not issubclass(
             Concurrent[KeyError, LookupError], Concurrent[KeyError, RuntimeError]
         )
+
+    def test_flattened_type(self):
+        """Flattening ``Concurrent[[Concurrent[a], b, ...]`` type-checks to flat case"""
+        assert issubclass(
+            type(Concurrent(Concurrent(KeyError())).flattened()), Concurrent[KeyError]
+        )
+        assert issubclass(
+            type(Concurrent(Concurrent(KeyError()), KeyError()).flattened()),
+            Concurrent[KeyError]
+        )
+        assert issubclass(
+            type(Concurrent(Concurrent(KeyError()), IndexError()).flattened()),
+            Concurrent[KeyError, IndexError]
+        )
+        assert issubclass(
+            type(Concurrent(Concurrent(LookupError()), IndexError()).flattened()),
+            Concurrent[LookupError, IndexError]
+        )
+        assert issubclass(
+            type(Concurrent(KeyError()).flattened()), Concurrent[KeyError]
+        )
+        assert issubclass(
+            type(Concurrent(Concurrent(KeyError())).flattened().flattened()),
+            Concurrent[KeyError],
+        )
+
+    def test_unflattend_type(self):
+        """Unflattened ``Concurrent[[Concurrent[a], b, ...]`` type-checks precisely"""
+        assert issubclass(
+            type(Concurrent(Concurrent(KeyError()))), Concurrent[Concurrent[KeyError]]
+        )
+        assert issubclass(
+            type(Concurrent(Concurrent(KeyError()), KeyError())),
+            Concurrent[Concurrent[KeyError], KeyError]
+        )
+        assert issubclass(
+            type(Concurrent(Concurrent(KeyError()), IndexError())),
+            Concurrent[Concurrent[KeyError], IndexError]
+        )
+        assert issubclass(
+            type(Concurrent(Concurrent(LookupError()), IndexError())),
+            Concurrent[Concurrent[LookupError], IndexError]
+        )
+
+    def test_flattened_value(self):
+        children = KeyError(), KeyError(), IndexError()
+        assert Concurrent(
+            Concurrent(children[0]), *children[1:]
+        ).flattened().children == children
diff --git a/usim_pytest/test_types/test_scope.py b/usim_pytest/test_types/test_scope.py
index a024ac1..58cfe58 100644
--- a/usim_pytest/test_types/test_scope.py
+++ b/usim_pytest/test_types/test_scope.py
@@ -138,6 +138,32 @@ class TestExceptions:
                     scope.do(async_raise(KeyError(), 0))
                     raise exc_type
 
+    @via_usim
+    async def test_fail_nested(self):
+        """Nested failures may be handled"""
+        async def inner_raise(exc: BaseException):
+            async with Scope() as scope:
+                scope.do(async_raise(exc))
+
+        # raising is valid
+        with pytest.raises(Concurrent):
+            async with Scope() as scope:
+                scope.do(inner_raise(KeyError()))
+        # handling is valid
+        with pytest.raises(Concurrent[Concurrent[KeyError]]):
+            async with Scope() as scope:
+                scope.do(inner_raise(KeyError()))
+        with pytest.raises(Concurrent[Concurrent]):
+            async with Scope() as scope:
+                scope.do(inner_raise(KeyError()))
+        with pytest.raises(Concurrent[Concurrent]):
+            # this layer is expected to let the exception pass through
+            try:
+                async with Scope() as scope:
+                    scope.do(inner_raise(KeyError()))
+            except Concurrent[Concurrent[IndexError]]:
+                assert False
+
 
 class TestScoping:
     @via_usim