diff --git a/docs/source/reference-core.rst b/docs/source/reference-core.rst index a6dc5b1f5a..fe187b36ce 100644 --- a/docs/source/reference-core.rst +++ b/docs/source/reference-core.rst @@ -768,12 +768,6 @@ inside the handler function(s) with the ``nonlocal`` keyword:: async with trio.open_nursery() as nursery: nursery.start_soon(broken1) -For reasons of backwards compatibility, nurseries raise ``trio.MultiError`` and -``trio.NonBaseMultiError`` which inherit from :exc:`BaseExceptionGroup` and -:exc:`ExceptionGroup`, respectively. Users should refrain from attempting to raise or -catch the Trio specific exceptions themselves, and treat them as if they were standard -:exc:`BaseExceptionGroup` or :exc:`ExceptionGroup` instances instead. - "Strict" versus "loose" ExceptionGroup semantics ++++++++++++++++++++++++++++++++++++++++++++++++ diff --git a/docs/source/tutorial.rst b/docs/source/tutorial.rst index 40eafd2833..a280bc839a 100644 --- a/docs/source/tutorial.rst +++ b/docs/source/tutorial.rst @@ -1168,7 +1168,7 @@ TODO: maybe a brief discussion of :exc:`KeyboardInterrupt` handling? you can stick anything inside a timeout block, even child tasks [show something like the first example but with a timeout – they - both get cancelled, the cancelleds get packed into a multierror, and + both get cancelled, the cancelleds get packed into an ExceptionGroup, and then the timeout block catches the cancelled] brief discussion of KI? diff --git a/newsfragments/2891.deprecated.rst b/newsfragments/2891.deprecated.rst new file mode 100644 index 0000000000..59dba79360 --- /dev/null +++ b/newsfragments/2891.deprecated.rst @@ -0,0 +1 @@ +``MultiError`` has been fully removed, and all relevant trio functions now raise ExceptionGroups instead. This should not affect end users that have transitioned to using ``except*`` or catching ExceptionGroup/BaseExceptionGroup. diff --git a/pyproject.toml b/pyproject.toml index 48c97eccd2..a4b10b8380 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -123,7 +123,6 @@ extend-exclude = [ [tool.ruff.per-file-ignores] 'src/trio/__init__.py' = ['F401'] 'src/trio/_core/__init__.py' = ['F401'] -'src/trio/_core/_tests/test_multierror_scripts/*' = ['F401'] 'src/trio/abc.py' = ['F401'] 'src/trio/lowlevel.py' = ['F401'] 'src/trio/socket.py' = ['F401'] @@ -236,9 +235,6 @@ showcontent = true branch = true source_pkgs = ["trio"] omit = [ - # These are run in subprocesses, but still don't work. We follow - # coverage's documentation to no avail. - "*/trio/_core/_tests/test_multierror_scripts/*", # Omit the generated files in trio/_core starting with _generated_ "*/trio/_core/_generated_*", # Type tests aren't intended to be run, just passed to type checkers. diff --git a/src/trio/__init__.py b/src/trio/__init__.py index 115211934c..3d3c7d3e7d 100644 --- a/src/trio/__init__.py +++ b/src/trio/__init__.py @@ -45,10 +45,6 @@ open_nursery as open_nursery, run as run, ) -from ._core._multierror import ( - MultiError as _MultiError, - NonBaseMultiError as _NonBaseMultiError, -) from ._deprecate import TrioDeprecationWarning as TrioDeprecationWarning from ._dtls import ( DTLSChannel as DTLSChannel, @@ -120,26 +116,7 @@ _deprecate.enable_attribute_deprecations(__name__) -__deprecated_attributes__: dict[str, _deprecate.DeprecatedAttribute] = { - "MultiError": _deprecate.DeprecatedAttribute( - value=_MultiError, - version="0.22.0", - issue=2211, - instead=( - "BaseExceptionGroup (on Python 3.11 and later) or " - "exceptiongroup.BaseExceptionGroup (earlier versions)" - ), - ), - "NonBaseMultiError": _deprecate.DeprecatedAttribute( - value=_NonBaseMultiError, - version="0.22.0", - issue=2211, - instead=( - "ExceptionGroup (on Python 3.11 and later) or " - "exceptiongroup.ExceptionGroup (earlier versions)" - ), - ), -} +__deprecated_attributes__: dict[str, _deprecate.DeprecatedAttribute] = {} # Having the public path in .__module__ attributes is important for: # - exception names in printed tracebacks diff --git a/src/trio/_core/_concat_tb.py b/src/trio/_core/_concat_tb.py new file mode 100644 index 0000000000..14f31f6184 --- /dev/null +++ b/src/trio/_core/_concat_tb.py @@ -0,0 +1,129 @@ +from __future__ import annotations + +from types import TracebackType +from typing import Any, ClassVar, cast + +################################################################ +# concat_tb +################################################################ + +# We need to compute a new traceback that is the concatenation of two existing +# tracebacks. This requires copying the entries in 'head' and then pointing +# the final tb_next to 'tail'. +# +# NB: 'tail' might be None, which requires some special handling in the ctypes +# version. +# +# The complication here is that Python doesn't actually support copying or +# modifying traceback objects, so we have to get creative... +# +# On CPython, we use ctypes. On PyPy, we use "transparent proxies". +# +# Jinja2 is a useful source of inspiration: +# https://github.com/pallets/jinja/blob/main/src/jinja2/debug.py + +try: + import tputil +except ImportError: + # ctypes it is + import ctypes + + # How to handle refcounting? I don't want to use ctypes.py_object because + # I don't understand or trust it, and I don't want to use + # ctypes.pythonapi.Py_{Inc,Dec}Ref because we might clash with user code + # that also tries to use them but with different types. So private _ctypes + # APIs it is! + import _ctypes + + class CTraceback(ctypes.Structure): + _fields_: ClassVar = [ + ("PyObject_HEAD", ctypes.c_byte * object().__sizeof__()), + ("tb_next", ctypes.c_void_p), + ("tb_frame", ctypes.c_void_p), + ("tb_lasti", ctypes.c_int), + ("tb_lineno", ctypes.c_int), + ] + + def copy_tb(base_tb: TracebackType, tb_next: TracebackType | None) -> TracebackType: + # TracebackType has no public constructor, so allocate one the hard way + try: + raise ValueError + except ValueError as exc: + new_tb = exc.__traceback__ + assert new_tb is not None + c_new_tb = CTraceback.from_address(id(new_tb)) + + # At the C level, tb_next either points to the next traceback or is + # NULL. c_void_p and the .tb_next accessor both convert NULL to None, + # but we shouldn't DECREF None just because we assigned to a NULL + # pointer! Here we know that our new traceback has only 1 frame in it, + # so we can assume the tb_next field is NULL. + assert c_new_tb.tb_next is None + # If tb_next is None, then we want to set c_new_tb.tb_next to NULL, + # which it already is, so we're done. Otherwise, we have to actually + # do some work: + if tb_next is not None: + _ctypes.Py_INCREF(tb_next) # type: ignore[attr-defined] + c_new_tb.tb_next = id(tb_next) + + assert c_new_tb.tb_frame is not None + _ctypes.Py_INCREF(base_tb.tb_frame) # type: ignore[attr-defined] + old_tb_frame = new_tb.tb_frame + c_new_tb.tb_frame = id(base_tb.tb_frame) + _ctypes.Py_DECREF(old_tb_frame) # type: ignore[attr-defined] + + c_new_tb.tb_lasti = base_tb.tb_lasti + c_new_tb.tb_lineno = base_tb.tb_lineno + + try: + return new_tb + finally: + # delete references from locals to avoid creating cycles + # see test_cancel_scope_exit_doesnt_create_cyclic_garbage + del new_tb, old_tb_frame + +else: + # http://doc.pypy.org/en/latest/objspace-proxies.html + def copy_tb(base_tb: TracebackType, tb_next: TracebackType | None) -> TracebackType: + # tputil.ProxyOperation is PyPy-only, and there's no way to specify + # cpython/pypy in current type checkers. + def controller(operation: tputil.ProxyOperation) -> Any | None: # type: ignore[no-any-unimported] + # Rationale for pragma: I looked fairly carefully and tried a few + # things, and AFAICT it's not actually possible to get any + # 'opname' that isn't __getattr__ or __getattribute__. So there's + # no missing test we could add, and no value in coverage nagging + # us about adding one. + if ( + operation.opname + in { + "__getattribute__", + "__getattr__", + } + and operation.args[0] == "tb_next" + ): # pragma: no cover + return tb_next + return operation.delegate() # Delegate is reverting to original behaviour + + return cast( + TracebackType, tputil.make_proxy(controller, type(base_tb), base_tb) + ) # Returns proxy to traceback + + +# this is used for collapsing single-exception ExceptionGroups when using +# `strict_exception_groups=False`. Once that is retired this function and its helper can +# be removed as well. +def concat_tb( + head: TracebackType | None, tail: TracebackType | None +) -> TracebackType | None: + # We have to use an iterative algorithm here, because in the worst case + # this might be a RecursionError stack that is by definition too deep to + # process by recursion! + head_tbs = [] + pointer = head + while pointer is not None: + head_tbs.append(pointer) + pointer = pointer.tb_next + current_head = tail + for head_tb in reversed(head_tbs): + current_head = copy_tb(head_tb, tb_next=current_head) + return current_head diff --git a/src/trio/_core/_multierror.py b/src/trio/_core/_multierror.py deleted file mode 100644 index f439f59c97..0000000000 --- a/src/trio/_core/_multierror.py +++ /dev/null @@ -1,501 +0,0 @@ -from __future__ import annotations - -import sys -from types import TracebackType -from typing import TYPE_CHECKING, Any, ClassVar, cast, overload - -import attr - -from trio._deprecate import warn_deprecated - -if sys.version_info < (3, 11): - from exceptiongroup import BaseExceptionGroup, ExceptionGroup - -if TYPE_CHECKING: - from collections.abc import Callable, Sequence - - from typing_extensions import Self -################################################################ -# MultiError -################################################################ - - -def _filter_impl( - handler: Callable[[BaseException], BaseException | None], root_exc: BaseException -) -> BaseException | None: - # We have a tree of MultiError's, like: - # - # MultiError([ - # ValueError, - # MultiError([ - # KeyError, - # ValueError, - # ]), - # ]) - # - # or similar. - # - # We want to - # 1) apply the filter to each of the leaf exceptions -- each leaf - # might stay the same, be replaced (with the original exception - # potentially sticking around as __context__ or __cause__), or - # disappear altogether. - # 2) simplify the resulting tree -- remove empty nodes, and replace - # singleton MultiError's with their contents, e.g.: - # MultiError([KeyError]) -> KeyError - # (This can happen recursively, e.g. if the two ValueErrors above - # get caught then we'll just be left with a bare KeyError.) - # 3) preserve sensible tracebacks - # - # It's the tracebacks that are most confusing. As a MultiError - # propagates through the stack, it accumulates traceback frames, but - # the exceptions inside it don't. Semantically, the traceback for a - # leaf exception is the concatenation the tracebacks of all the - # exceptions you see when traversing the exception tree from the root - # to that leaf. Our correctness invariant is that this concatenated - # traceback should be the same before and after. - # - # The easy way to do that would be to, at the beginning of this - # function, "push" all tracebacks down to the leafs, so all the - # MultiErrors have __traceback__=None, and all the leafs have complete - # tracebacks. But whenever possible, we'd actually prefer to keep - # tracebacks as high up in the tree as possible, because this lets us - # keep only a single copy of the common parts of these exception's - # tracebacks. This is cheaper (in memory + time -- tracebacks are - # unpleasantly quadratic-ish to work with, and this might matter if - # you have thousands of exceptions, which can happen e.g. after - # cancelling a large task pool, and no-one will ever look at their - # tracebacks!), and more importantly, factoring out redundant parts of - # the tracebacks makes them more readable if/when users do see them. - # - # So instead our strategy is: - # - first go through and construct the new tree, preserving any - # unchanged subtrees - # - then go through the original tree (!) and push tracebacks down - # until either we hit a leaf, or we hit a subtree which was - # preserved in the new tree. - - # This used to also support async handler functions. But that runs into: - # https://bugs.python.org/issue29600 - # which is difficult to fix on our end. - - # Filters a subtree, ignoring tracebacks, while keeping a record of - # which MultiErrors were preserved unchanged - def filter_tree( - exc: MultiError | BaseException, preserved: set[int] - ) -> MultiError | BaseException | None: - if isinstance(exc, MultiError): - new_exceptions = [] - changed = False - for child_exc in exc.exceptions: - new_child_exc = filter_tree( # noqa: F821 # Deleted in local scope below, causes ruff to think it's not defined (astral-sh/ruff#7733) - child_exc, preserved - ) - if new_child_exc is not child_exc: - changed = True - if new_child_exc is not None: - new_exceptions.append(new_child_exc) - if not new_exceptions: - return None - elif changed: - return MultiError(new_exceptions) - else: - preserved.add(id(exc)) - return exc - else: - new_exc = handler(exc) - # Our version of implicit exception chaining - if new_exc is not None and new_exc is not exc: - new_exc.__context__ = exc - return new_exc - - def push_tb_down( - tb: TracebackType | None, exc: BaseException, preserved: set[int] - ) -> None: - if id(exc) in preserved: - return - new_tb = concat_tb(tb, exc.__traceback__) - if isinstance(exc, MultiError): - for child_exc in exc.exceptions: - push_tb_down( # noqa: F821 # Deleted in local scope below, causes ruff to think it's not defined (astral-sh/ruff#7733) - new_tb, child_exc, preserved - ) - exc.__traceback__ = None - else: - exc.__traceback__ = new_tb - - preserved: set[int] = set() - new_root_exc = filter_tree(root_exc, preserved) - push_tb_down(None, root_exc, preserved) - # Delete the local functions to avoid a reference cycle (see - # test_simple_cancel_scope_usage_doesnt_create_cyclic_garbage) - del filter_tree, push_tb_down - return new_root_exc - - -# Normally I'm a big fan of (a)contextmanager, but in this case I found it -# easier to use the raw context manager protocol, because it makes it a lot -# easier to reason about how we're mutating the traceback as we go. (End -# result: if the exception gets modified, then the 'raise' here makes this -# frame show up in the traceback; otherwise, we leave no trace.) -@attr.s(frozen=True) -class MultiErrorCatcher: - _handler: Callable[[BaseException], BaseException | None] = attr.ib() - - def __enter__(self) -> None: - pass - - def __exit__( - self, - exc_type: type[BaseException] | None, - exc_value: BaseException | None, - traceback: TracebackType | None, - ) -> bool | None: - if exc_value is not None: - filtered_exc = _filter_impl(self._handler, exc_value) - - if filtered_exc is exc_value: - # Let the interpreter re-raise it - return False - if filtered_exc is None: - # Swallow the exception - return True - # When we raise filtered_exc, Python will unconditionally blow - # away its __context__ attribute and replace it with the original - # exc we caught. So after we raise it, we have to pause it while - # it's in flight to put the correct __context__ back. - old_context = filtered_exc.__context__ - try: - raise filtered_exc - finally: - _, value, _ = sys.exc_info() - assert value is filtered_exc - value.__context__ = old_context - # delete references from locals to avoid creating cycles - # see test_MultiError_catch_doesnt_create_cyclic_garbage - del _, filtered_exc, value - return False - - -if TYPE_CHECKING: - _BaseExceptionGroup = BaseExceptionGroup[BaseException] -else: - _BaseExceptionGroup = BaseExceptionGroup - - -class MultiError(_BaseExceptionGroup): - """An exception that contains other exceptions; also known as an - "inception". - - It's main use is to represent the situation when multiple child tasks all - raise errors "in parallel". - - Args: - exceptions (list): The exceptions - - Returns: - If ``len(exceptions) == 1``, returns that exception. This means that a - call to ``MultiError(...)`` is not guaranteed to return a - :exc:`MultiError` object! - - Otherwise, returns a new :exc:`MultiError` object. - - Raises: - TypeError: if any of the passed in objects are not instances of - :exc:`BaseException`. - - """ - - def __init__( - self, exceptions: Sequence[BaseException], *, _collapse: bool = True - ) -> None: - self.collapse = _collapse - - # Avoid double initialization when _collapse is True and exceptions[0] returned - # by __new__() happens to be a MultiError and subsequently __init__() is called. - if _collapse and getattr(self, "exceptions", None) is not None: - # This exception was already initialized. - return - - super().__init__("multiple tasks failed", exceptions) - - def __new__( # type: ignore[misc] # mypy says __new__ must return a class instance - cls, exceptions: Sequence[BaseException], *, _collapse: bool = True - ) -> NonBaseMultiError | Self | BaseException: - exceptions = list(exceptions) - for exc in exceptions: - if not isinstance(exc, BaseException): - raise TypeError(f"Expected an exception object, not {exc!r}") - if _collapse and len(exceptions) == 1: - # If this lone object happens to itself be a MultiError, then - # Python will implicitly call our __init__ on it again. See - # special handling in __init__. - return exceptions[0] - else: - # The base class __new__() implicitly invokes our __init__, which - # is what we want. - # - # In an earlier version of the code, we didn't define __init__ and - # simply set the `exceptions` attribute directly on the new object. - # However, linters expect attributes to be initialized in __init__. - from_class: type[Self | NonBaseMultiError] = cls - if all(isinstance(exc, Exception) for exc in exceptions): - from_class = NonBaseMultiError - - # Ignoring arg-type: 'Argument 3 to "__new__" of "BaseExceptionGroup" has incompatible type "list[BaseException]"; expected "Sequence[_BaseExceptionT_co]"' - # We have checked that exceptions is indeed a list of BaseException objects, this is fine. - new_obj = super().__new__(from_class, "multiple tasks failed", exceptions) # type: ignore[arg-type] - assert isinstance(new_obj, (cls, NonBaseMultiError)) - return new_obj - - def __reduce__( - self, - ) -> tuple[object, tuple[type[Self], list[BaseException]], dict[str, bool]]: - return ( - self.__new__, - (self.__class__, list(self.exceptions)), - {"collapse": self.collapse}, - ) - - def __str__(self) -> str: - return ", ".join(repr(exc) for exc in self.exceptions) - - def __repr__(self) -> str: - return f"" - - @overload # type: ignore[override] # 'Exception' != '_ExceptionT' - def derive(self, excs: Sequence[Exception], /) -> NonBaseMultiError: - ... - - @overload - def derive(self, excs: Sequence[BaseException], /) -> MultiError: - ... - - def derive( - self, excs: Sequence[Exception | BaseException], / - ) -> NonBaseMultiError | MultiError: - # We use _collapse=False here to get ExceptionGroup semantics, since derive() - # is part of the PEP 654 API - exc = MultiError(excs, _collapse=False) - exc.collapse = self.collapse - return exc - - @classmethod - def filter( - cls, - handler: Callable[[BaseException], BaseException | None], - root_exc: BaseException, - ) -> BaseException | None: - """Apply the given ``handler`` to all the exceptions in ``root_exc``. - - Args: - handler: A callable that takes an atomic (non-MultiError) exception - as input, and returns either a new exception object or None. - root_exc: An exception, often (though not necessarily) a - :exc:`MultiError`. - - Returns: - A new exception object in which each component exception ``exc`` has - been replaced by the result of running ``handler(exc)`` – or, if - ``handler`` returned None for all the inputs, returns None. - - """ - warn_deprecated( - "MultiError.filter()", - "0.22.0", - instead="BaseExceptionGroup.split()", - issue=2211, - ) - return _filter_impl(handler, root_exc) - - @classmethod - def catch( - cls, handler: Callable[[BaseException], BaseException | None] - ) -> MultiErrorCatcher: - """Return a context manager that catches and re-throws exceptions - after running :meth:`filter` on them. - - Args: - handler: as for :meth:`filter` - - """ - warn_deprecated( - "MultiError.catch", - "0.22.0", - instead="except* or exceptiongroup.catch()", - issue=2211, - ) - - return MultiErrorCatcher(handler) - - -if TYPE_CHECKING: - _ExceptionGroup = ExceptionGroup[Exception] -else: - _ExceptionGroup = ExceptionGroup - - -class NonBaseMultiError(MultiError, _ExceptionGroup): - __slots__ = () - - -# Clean up exception printing: -MultiError.__module__ = "trio" -NonBaseMultiError.__module__ = "trio" - -################################################################ -# concat_tb -################################################################ - -# We need to compute a new traceback that is the concatenation of two existing -# tracebacks. This requires copying the entries in 'head' and then pointing -# the final tb_next to 'tail'. -# -# NB: 'tail' might be None, which requires some special handling in the ctypes -# version. -# -# The complication here is that Python doesn't actually support copying or -# modifying traceback objects, so we have to get creative... -# -# On CPython, we use ctypes. On PyPy, we use "transparent proxies". -# -# Jinja2 is a useful source of inspiration: -# https://github.com/pallets/jinja/blob/master/jinja2/debug.py - -try: - import tputil -except ImportError: - # ctypes it is - import ctypes - - # How to handle refcounting? I don't want to use ctypes.py_object because - # I don't understand or trust it, and I don't want to use - # ctypes.pythonapi.Py_{Inc,Dec}Ref because we might clash with user code - # that also tries to use them but with different types. So private _ctypes - # APIs it is! - import _ctypes - - class CTraceback(ctypes.Structure): - _fields_: ClassVar = [ - ("PyObject_HEAD", ctypes.c_byte * object().__sizeof__()), - ("tb_next", ctypes.c_void_p), - ("tb_frame", ctypes.c_void_p), - ("tb_lasti", ctypes.c_int), - ("tb_lineno", ctypes.c_int), - ] - - def copy_tb(base_tb: TracebackType, tb_next: TracebackType | None) -> TracebackType: - # TracebackType has no public constructor, so allocate one the hard way - try: - raise ValueError - except ValueError as exc: - new_tb = exc.__traceback__ - assert new_tb is not None - c_new_tb = CTraceback.from_address(id(new_tb)) - - # At the C level, tb_next either pointer to the next traceback or is - # NULL. c_void_p and the .tb_next accessor both convert NULL to None, - # but we shouldn't DECREF None just because we assigned to a NULL - # pointer! Here we know that our new traceback has only 1 frame in it, - # so we can assume the tb_next field is NULL. - assert c_new_tb.tb_next is None - # If tb_next is None, then we want to set c_new_tb.tb_next to NULL, - # which it already is, so we're done. Otherwise, we have to actually - # do some work: - if tb_next is not None: - _ctypes.Py_INCREF(tb_next) # type: ignore[attr-defined] - c_new_tb.tb_next = id(tb_next) - - assert c_new_tb.tb_frame is not None - _ctypes.Py_INCREF(base_tb.tb_frame) # type: ignore[attr-defined] - old_tb_frame = new_tb.tb_frame - c_new_tb.tb_frame = id(base_tb.tb_frame) - _ctypes.Py_DECREF(old_tb_frame) # type: ignore[attr-defined] - - c_new_tb.tb_lasti = base_tb.tb_lasti - c_new_tb.tb_lineno = base_tb.tb_lineno - - try: - return new_tb - finally: - # delete references from locals to avoid creating cycles - # see test_MultiError_catch_doesnt_create_cyclic_garbage - del new_tb, old_tb_frame - -else: - # http://doc.pypy.org/en/latest/objspace-proxies.html - def copy_tb(base_tb: TracebackType, tb_next: TracebackType | None) -> TracebackType: - # tputil.ProxyOperation is PyPy-only, but we run mypy on CPython - def controller(operation: tputil.ProxyOperation) -> Any | None: # type: ignore[no-any-unimported] - # Rationale for pragma: I looked fairly carefully and tried a few - # things, and AFAICT it's not actually possible to get any - # 'opname' that isn't __getattr__ or __getattribute__. So there's - # no missing test we could add, and no value in coverage nagging - # us about adding one. - if ( - operation.opname - in { - "__getattribute__", - "__getattr__", - } - and operation.args[0] == "tb_next" - ): # pragma: no cover - return tb_next - return operation.delegate() # Deligate is reverting to original behaviour - - return cast( - TracebackType, tputil.make_proxy(controller, type(base_tb), base_tb) - ) # Returns proxy to traceback - - -def concat_tb( - head: TracebackType | None, tail: TracebackType | None -) -> TracebackType | None: - # We have to use an iterative algorithm here, because in the worst case - # this might be a RecursionError stack that is by definition too deep to - # process by recursion! - head_tbs = [] - pointer = head - while pointer is not None: - head_tbs.append(pointer) - pointer = pointer.tb_next - current_head = tail - for head_tb in reversed(head_tbs): - current_head = copy_tb(head_tb, tb_next=current_head) - return current_head - - -# Ubuntu's system Python has a sitecustomize.py file that import -# apport_python_hook and replaces sys.excepthook. -# -# The custom hook captures the error for crash reporting, and then calls -# sys.__excepthook__ to actually print the error. -# -# We don't mind it capturing the error for crash reporting, but we want to -# take over printing the error. So we monkeypatch the apport_python_hook -# module so that instead of calling sys.__excepthook__, it calls our custom -# hook. -# -# More details: https://github.com/python-trio/trio/issues/1065 -if sys.version_info < (3, 11) and getattr(sys.excepthook, "__name__", None) in ( - "apport_excepthook", - "partial_apport_excepthook", -): - from types import ModuleType - - import apport_python_hook - from exceptiongroup import format_exception - - assert sys.excepthook is apport_python_hook.apport_excepthook - - def replacement_excepthook( - etype: type[BaseException], value: BaseException, tb: TracebackType | None - ) -> None: - # This does work, it's an overloaded function - sys.stderr.write("".join(format_exception(etype, value, tb))) # type: ignore[arg-type] - - fake_sys = ModuleType("trio_fake_sys") - fake_sys.__dict__.update(sys.__dict__) - # Fake does have __excepthook__ after __dict__ update, but type checkers don't recognize this - fake_sys.__excepthook__ = replacement_excepthook # type: ignore[attr-defined] - apport_python_hook.sys = fake_sys diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index 015065863e..1e37a7dbb2 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -35,11 +35,11 @@ from .._abc import Clock, Instrument from .._util import NoPublicConstructor, coroutine_or_error, final from ._asyncgens import AsyncGenerators +from ._concat_tb import concat_tb from ._entry_queue import EntryQueue, TrioToken from ._exceptions import Cancelled, RunFinishedError, TrioInternalError from ._instrumentation import Instruments from ._ki import LOCALS_KEY_KI_PROTECTION_ENABLED, KIManager, enable_ki_protection -from ._multierror import MultiError, concat_tb from ._thread_cache import start_thread_soon from ._traps import ( Abort, @@ -95,6 +95,9 @@ def __call__( # Passed as a sentinel _NO_SEND: Final[Outcome[Any]] = cast("Outcome[Any]", object()) +# Used to track if an exceptiongroup can be collapsed +NONSTRICT_EXCEPTIONGROUP_NOTE = 'This is a "loose" ExceptionGroup, and may be collapsed by Trio if it only contains one exception - typically after `Cancelled` has been stripped from it. Note this has consequences for exception handling, and strict_exception_groups=True is recommended.' + @final class _NoStatus(metaclass=NoPublicConstructor): @@ -206,7 +209,11 @@ def collapse_exception_group( modified = True exceptions[i] = new_exc - if len(exceptions) == 1 and isinstance(excgroup, MultiError) and excgroup.collapse: + if ( + len(exceptions) == 1 + and isinstance(excgroup, BaseExceptionGroup) + and NONSTRICT_EXCEPTIONGROUP_NOTE in getattr(excgroup, "__notes__", ()) + ): exceptions[0].__traceback__ = concat_tb( excgroup.__traceback__, exceptions[0].__traceback__ ) @@ -642,7 +649,7 @@ def __exit__( elif remaining_error_after_cancel_scope is exc: return False else: - # Copied verbatim from MultiErrorCatcher. Python doesn't + # Copied verbatim from the old MultiErrorCatcher. Python doesn't # allow us to encapsulate this __context__ fixup. old_context = remaining_error_after_cancel_scope.__context__ try: @@ -653,6 +660,7 @@ def __exit__( value.__context__ = old_context # delete references from locals to avoid creating cycles # see test_cancel_scope_exit_doesnt_create_cyclic_garbage + # Note: still relevant del remaining_error_after_cancel_scope, value, _, exc # deep magic to remove refs via f_locals locals() @@ -956,7 +964,7 @@ async def __aexit__( elif combined_error_from_nursery is exc: return False else: - # Copied verbatim from MultiErrorCatcher. Python doesn't + # Copied verbatim from the old MultiErrorCatcher. Python doesn't # allow us to encapsulate this __context__ fixup. old_context = combined_error_from_nursery.__context__ try: @@ -966,7 +974,7 @@ async def __aexit__( assert value is combined_error_from_nursery value.__context__ = old_context # delete references from locals to avoid creating cycles - # see test_simple_cancel_scope_usage_doesnt_create_cyclic_garbage + # see test_cancel_scope_exit_doesnt_create_cyclic_garbage del _, combined_error_from_nursery, value, new_exc # make sure these raise errors in static analysis if called @@ -1087,7 +1095,7 @@ def _child_finished(self, task: Task, outcome: Outcome[Any]) -> None: async def _nested_child_finished( self, nested_child_exc: BaseException | None ) -> BaseException | None: - # Returns MultiError instance (or any exception if the nursery is in loose mode + # Returns ExceptionGroup instance (or any exception if the nursery is in loose mode # and there is just one contained exception) if there are pending exceptions if nested_child_exc is not None: self._add_exc(nested_child_exc) @@ -1102,6 +1110,7 @@ def aborted(raise_cancel: _core.RaiseCancelT) -> Abort: exn = capture(raise_cancel).error if not isinstance(exn, Cancelled): self._add_exc(exn) + # see test_cancel_scope_exit_doesnt_create_cyclic_garbage del exn # prevent cyclic garbage creation return Abort.FAILED @@ -1120,12 +1129,17 @@ def aborted(raise_cancel: _core.RaiseCancelT) -> Abort: assert popped is self if self._pending_excs: try: - return MultiError( - self._pending_excs, _collapse=not self._strict_exception_groups + if not self._strict_exception_groups and len(self._pending_excs) == 1: + return self._pending_excs[0] + exception = BaseExceptionGroup( + "Exceptions from Trio nursery", self._pending_excs ) + if not self._strict_exception_groups: + exception.add_note(NONSTRICT_EXCEPTIONGROUP_NOTE) + return exception finally: # avoid a garbage cycle - # (see test_nursery_cancel_doesnt_create_cyclic_garbage) + # (see test_locals_destroyed_promptly_on_cancel) del self._pending_excs return None @@ -1768,8 +1782,7 @@ async def python_wrapper(orig_coro: Awaitable[RetT]) -> RetT: self.instruments.call("task_spawned", task) # Special case: normally next_send should be an Outcome, but for the # very first send we have to send a literal unboxed None. - # TODO: remove [unused-ignore] when Outcome is typed - self.reschedule(task, None) # type: ignore[arg-type, unused-ignore] + self.reschedule(task, None) # type: ignore[arg-type] return task def task_exited(self, task: Task, outcome: Outcome[Any]) -> None: @@ -2615,8 +2628,7 @@ def unrolled_run( # protocol of unwrapping whatever outcome gets sent in. # Instead, we'll arrange to throw `exc` in directly, # which works for at least asyncio and curio. - # TODO: remove [unused-ignore] when Outcome is typed - runner.reschedule(task, exc) # type: ignore[arg-type, unused-ignore] + runner.reschedule(task, exc) # type: ignore[arg-type] task._next_send_fn = task.coro.throw # prevent long-lived reference # TODO: develop test for this deletion @@ -2626,7 +2638,7 @@ def unrolled_run( runner.instruments.call("after_task_step", task) del GLOBAL_RUN_CONTEXT.task # prevent long-lived references - # TODO: develop test for these deletions + # TODO: develop test for this deletion del task, next_send, next_send_fn except GeneratorExit: diff --git a/src/trio/_core/_tests/test_exceptiongroup_gc.py b/src/trio/_core/_tests/test_exceptiongroup_gc.py new file mode 100644 index 0000000000..8957a581a5 --- /dev/null +++ b/src/trio/_core/_tests/test_exceptiongroup_gc.py @@ -0,0 +1,101 @@ +from __future__ import annotations + +import gc +import sys +from traceback import extract_tb +from typing import TYPE_CHECKING, Callable, NoReturn + +import pytest + +from .._concat_tb import concat_tb + +if TYPE_CHECKING: + from types import TracebackType + +if sys.version_info < (3, 11): + from exceptiongroup import ExceptionGroup + + +def raiser1() -> NoReturn: + raiser1_2() + + +def raiser1_2() -> NoReturn: + raiser1_3() + + +def raiser1_3() -> NoReturn: + raise ValueError("raiser1_string") + + +def raiser2() -> NoReturn: + raiser2_2() + + +def raiser2_2() -> NoReturn: + raise KeyError("raiser2_string") + + +def get_exc(raiser: Callable[[], NoReturn]) -> Exception: + try: + raiser() + except Exception as exc: + return exc + raise AssertionError("raiser should always raise") # pragma: no cover + + +def get_tb(raiser: Callable[[], NoReturn]) -> TracebackType | None: + return get_exc(raiser).__traceback__ + + +def test_concat_tb() -> None: + tb1 = get_tb(raiser1) + tb2 = get_tb(raiser2) + + # These return a list of (filename, lineno, fn name, text) tuples + # https://docs.python.org/3/library/traceback.html#traceback.extract_tb + entries1 = extract_tb(tb1) + entries2 = extract_tb(tb2) + + tb12 = concat_tb(tb1, tb2) + assert extract_tb(tb12) == entries1 + entries2 + + tb21 = concat_tb(tb2, tb1) + assert extract_tb(tb21) == entries2 + entries1 + + # Check degenerate cases + assert extract_tb(concat_tb(None, tb1)) == entries1 + assert extract_tb(concat_tb(tb1, None)) == entries1 + assert concat_tb(None, None) is None + + # Make sure the original tracebacks didn't get mutated by mistake + assert extract_tb(get_tb(raiser1)) == entries1 + assert extract_tb(get_tb(raiser2)) == entries2 + + +# Unclear if this can still fail, removing the `del` from _concat_tb.copy_tb does not seem +# to trigger it (on a platform where the `del` is executed) +@pytest.mark.skipif( + sys.implementation.name != "cpython", reason="Only makes sense with refcounting GC" +) +def test_ExceptionGroup_catch_doesnt_create_cyclic_garbage() -> None: + # https://github.com/python-trio/trio/pull/2063 + gc.collect() + old_flags = gc.get_debug() + + def make_multi() -> NoReturn: + raise ExceptionGroup("", [get_exc(raiser1), get_exc(raiser2)]) + + try: + gc.set_debug(gc.DEBUG_SAVEALL) + with pytest.raises(ExceptionGroup) as excinfo: + # covers ~~MultiErrorCatcher.__exit__ and~~ _concat_tb.copy_tb + # TODO: is the above comment true anymore? as this no longer uses MultiError.catch + raise make_multi() + for exc in excinfo.value.exceptions: + assert isinstance(exc, (ValueError, KeyError)) + gc.collect() + assert not gc.garbage + finally: + gc.set_debug(old_flags) + gc.garbage.clear() diff --git a/src/trio/_core/_tests/test_multierror.py b/src/trio/_core/_tests/test_multierror.py deleted file mode 100644 index f7f5e3d076..0000000000 --- a/src/trio/_core/_tests/test_multierror.py +++ /dev/null @@ -1,518 +0,0 @@ -from __future__ import annotations - -import gc -import os -import pickle -import re -import subprocess -import sys -import warnings -from pathlib import Path -from traceback import extract_tb, print_exception -from typing import TYPE_CHECKING, Callable, NoReturn - -import pytest - -from ... import TrioDeprecationWarning -from ..._core import open_nursery -from .._multierror import MultiError, NonBaseMultiError, concat_tb -from .tutil import slow - -if TYPE_CHECKING: - from types import TracebackType - -if sys.version_info < (3, 11): - from exceptiongroup import ExceptionGroup - - -class NotHashableException(Exception): - code: int | None = None - - def __init__(self, code: int) -> None: - super().__init__() - self.code = code - - def __eq__(self, other: object) -> bool: - if not isinstance(other, NotHashableException): - return False - return self.code == other.code - - -async def raise_nothashable(code: int) -> NoReturn: - raise NotHashableException(code) - - -def raiser1() -> NoReturn: - raiser1_2() - - -def raiser1_2() -> NoReturn: - raiser1_3() - - -def raiser1_3() -> NoReturn: - raise ValueError("raiser1_string") - - -def raiser2() -> NoReturn: - raiser2_2() - - -def raiser2_2() -> NoReturn: - raise KeyError("raiser2_string") - - -def raiser3() -> NoReturn: - raise NameError - - -def get_exc(raiser: Callable[[], NoReturn]) -> BaseException: - try: - raiser() - except Exception as exc: - return exc - raise AssertionError("raiser should always raise") # pragma: no cover - - -def get_tb(raiser: Callable[[], NoReturn]) -> TracebackType | None: - return get_exc(raiser).__traceback__ - - -def test_concat_tb() -> None: - tb1 = get_tb(raiser1) - tb2 = get_tb(raiser2) - - # These return a list of (filename, lineno, fn name, text) tuples - # https://docs.python.org/3/library/traceback.html#traceback.extract_tb - entries1 = extract_tb(tb1) - entries2 = extract_tb(tb2) - - tb12 = concat_tb(tb1, tb2) - assert extract_tb(tb12) == entries1 + entries2 - - tb21 = concat_tb(tb2, tb1) - assert extract_tb(tb21) == entries2 + entries1 - - # Check degenerate cases - assert extract_tb(concat_tb(None, tb1)) == entries1 - assert extract_tb(concat_tb(tb1, None)) == entries1 - assert concat_tb(None, None) is None - - # Make sure the original tracebacks didn't get mutated by mistake - assert extract_tb(get_tb(raiser1)) == entries1 - assert extract_tb(get_tb(raiser2)) == entries2 - - -def test_MultiError() -> None: - exc1 = get_exc(raiser1) - exc2 = get_exc(raiser2) - - assert MultiError([exc1]) is exc1 - m = MultiError([exc1, exc2]) - assert m.exceptions == (exc1, exc2) - assert "ValueError" in str(m) - assert "ValueError" in repr(m) - - with pytest.raises(TypeError): - MultiError(object()) # type: ignore[arg-type] - with pytest.raises(TypeError): - MultiError([KeyError(), ValueError]) # type: ignore[list-item] - - -def test_MultiErrorOfSingleMultiError() -> None: - # For MultiError([MultiError]), ensure there is no bad recursion by the - # constructor where __init__ is called if __new__ returns a bare MultiError. - exceptions = (KeyError(), ValueError()) - a = MultiError(exceptions) - b = MultiError([a]) - assert b == a - assert b.exceptions == exceptions - - -async def test_MultiErrorNotHashable() -> None: - exc1 = NotHashableException(42) - exc2 = NotHashableException(4242) - exc3 = ValueError() - assert exc1 != exc2 - assert exc1 != exc3 - - with pytest.raises(MultiError): # noqa: PT012 - async with open_nursery() as nursery: - nursery.start_soon(raise_nothashable, 42) - nursery.start_soon(raise_nothashable, 4242) - - -def test_MultiError_filter_NotHashable() -> None: - excs = MultiError([NotHashableException(42), ValueError()]) - - def handle_ValueError(exc: BaseException) -> BaseException | None: - if isinstance(exc, ValueError): - return None - else: - return exc - - with pytest.warns(TrioDeprecationWarning): - filtered_excs = MultiError.filter(handle_ValueError, excs) - - assert isinstance(filtered_excs, NotHashableException) - - -def make_tree() -> MultiError: - # Returns an object like: - # MultiError([ - # MultiError([ - # ValueError, - # KeyError, - # ]), - # NameError, - # ]) - # where all exceptions except the root have a non-trivial traceback. - exc1 = get_exc(raiser1) - exc2 = get_exc(raiser2) - exc3 = get_exc(raiser3) - - # Give m12 a non-trivial traceback - try: - raise MultiError([exc1, exc2]) - except BaseException as m12: - return MultiError([m12, exc3]) - - -def assert_tree_eq( - m1: BaseException | MultiError | None, m2: BaseException | MultiError | None -) -> None: - if m1 is None or m2 is None: - assert m1 is m2 - return - assert type(m1) is type(m2) - assert extract_tb(m1.__traceback__) == extract_tb(m2.__traceback__) - assert_tree_eq(m1.__cause__, m2.__cause__) - assert_tree_eq(m1.__context__, m2.__context__) - if isinstance(m1, MultiError): - assert isinstance(m2, MultiError) - assert len(m1.exceptions) == len(m2.exceptions) - for e1, e2 in zip(m1.exceptions, m2.exceptions): - assert_tree_eq(e1, e2) - - -def test_MultiError_filter() -> None: - def null_handler(exc: BaseException) -> BaseException: - return exc - - m = make_tree() - assert_tree_eq(m, m) - with pytest.warns(TrioDeprecationWarning): - assert MultiError.filter(null_handler, m) is m - - assert_tree_eq(m, make_tree()) - - # Make sure we don't pick up any detritus if run in a context where - # implicit exception chaining would like to kick in - m = make_tree() - try: - raise ValueError - except ValueError: - with pytest.warns(TrioDeprecationWarning): - assert MultiError.filter(null_handler, m) is m - assert_tree_eq(m, make_tree()) - - def simple_filter(exc: BaseException) -> BaseException | None: - if isinstance(exc, ValueError): - return None - if isinstance(exc, KeyError): - return RuntimeError() - return exc - - with pytest.warns(TrioDeprecationWarning): - new_m = MultiError.filter(simple_filter, make_tree()) - - assert isinstance(new_m, MultiError) - assert len(new_m.exceptions) == 2 - # was: [[ValueError, KeyError], NameError] - # ValueError disappeared & KeyError became RuntimeError, so now: - assert isinstance(new_m.exceptions[0], RuntimeError) - assert isinstance(new_m.exceptions[1], NameError) - - # implicit chaining: - assert isinstance(new_m.exceptions[0].__context__, KeyError) - - # also, the traceback on the KeyError incorporates what used to be the - # traceback on its parent MultiError - orig = make_tree() - # make sure we have the right path - assert isinstance(orig.exceptions[0], MultiError) - assert isinstance(orig.exceptions[0].exceptions[1], KeyError) - # get original traceback summary - orig_extracted = ( - extract_tb(orig.__traceback__) - + extract_tb(orig.exceptions[0].__traceback__) - + extract_tb(orig.exceptions[0].exceptions[1].__traceback__) - ) - - def p(exc: BaseException) -> None: - print_exception(type(exc), exc, exc.__traceback__) - - p(orig) - p(orig.exceptions[0]) - p(orig.exceptions[0].exceptions[1]) - p(new_m.exceptions[0].__context__) - # compare to the new path - assert new_m.__traceback__ is None - new_extracted = extract_tb(new_m.exceptions[0].__context__.__traceback__) - assert orig_extracted == new_extracted - - # check preserving partial tree - def filter_NameError(exc: BaseException) -> BaseException | None: - if isinstance(exc, NameError): - return None - return exc - - m = make_tree() - with pytest.warns(TrioDeprecationWarning): - new_m = MultiError.filter(filter_NameError, m) - # with the NameError gone, the other branch gets promoted - assert new_m is m.exceptions[0] - - # check fully handling everything - def filter_all(exc: BaseException) -> None: - return None - - with pytest.warns(TrioDeprecationWarning): - assert MultiError.filter(filter_all, make_tree()) is None - - -def test_MultiError_catch() -> None: - # No exception to catch - - def noop(_: object) -> None: - pass # pragma: no cover - - with pytest.warns(TrioDeprecationWarning), MultiError.catch(noop): - pass - - # Simple pass-through of all exceptions - m = make_tree() - with pytest.raises(MultiError) as excinfo: - with pytest.warns(TrioDeprecationWarning), MultiError.catch(lambda exc: exc): - raise m - assert excinfo.value is m - # Should be unchanged, except that we added a traceback frame by raising - # it here - assert m.__traceback__ is not None - assert m.__traceback__.tb_frame.f_code.co_name == "test_MultiError_catch" - assert m.__traceback__.tb_next is None - m.__traceback__ = None - assert_tree_eq(m, make_tree()) - - # Swallows everything - with pytest.warns(TrioDeprecationWarning), MultiError.catch(lambda _: None): - raise make_tree() - - def simple_filter(exc): - if isinstance(exc, ValueError): - return None - if isinstance(exc, KeyError): - return RuntimeError() - return exc - - with pytest.raises(MultiError) as excinfo: - with pytest.warns(TrioDeprecationWarning), MultiError.catch(simple_filter): - raise make_tree() - new_m = excinfo.value - assert isinstance(new_m, MultiError) - assert len(new_m.exceptions) == 2 - # was: [[ValueError, KeyError], NameError] - # ValueError disappeared & KeyError became RuntimeError, so now: - assert isinstance(new_m.exceptions[0], RuntimeError) - assert isinstance(new_m.exceptions[1], NameError) - # Make sure that Python did not successfully attach the old MultiError to - # our new MultiError's __context__ - assert not new_m.__suppress_context__ - assert new_m.__context__ is None - - # check preservation of __cause__ and __context__ - v = ValueError("waffles are great") - v.__cause__ = KeyError() - with pytest.raises(ValueError, match="^waffles are great$") as excinfo: - with pytest.warns(TrioDeprecationWarning), MultiError.catch(lambda exc: exc): - raise v - assert isinstance(excinfo.value.__cause__, KeyError) - - v = ValueError("mushroom soup") - context = KeyError() - v.__context__ = context - with pytest.raises(ValueError, match="^mushroom soup$") as excinfo: - with pytest.warns(TrioDeprecationWarning), MultiError.catch(lambda exc: exc): - raise v - assert excinfo.value.__context__ is context - assert not excinfo.value.__suppress_context__ - - for suppress_context in [True, False]: - v = ValueError("unique text") - context = KeyError() - v.__context__ = context - v.__suppress_context__ = suppress_context - distractor = RuntimeError() - - def catch_RuntimeError(exc: Exception) -> Exception | None: - if isinstance(exc, RuntimeError): - return None - return exc - - with pytest.raises(ValueError, match="^unique text$") as excinfo: # noqa: PT012 - with pytest.warns(TrioDeprecationWarning): - with MultiError.catch(catch_RuntimeError): - raise MultiError([v, distractor]) - assert excinfo.value.__context__ is context - assert excinfo.value.__suppress_context__ == suppress_context - - -@pytest.mark.skipif( - sys.implementation.name != "cpython", reason="Only makes sense with refcounting GC" -) -def test_MultiError_catch_doesnt_create_cyclic_garbage() -> None: - # https://github.com/python-trio/trio/pull/2063 - gc.collect() - old_flags = gc.get_debug() - - def make_multi() -> NoReturn: - # make_tree creates cycles itself, so a simple - raise MultiError([get_exc(raiser1), get_exc(raiser2)]) - - def simple_filter(exc: BaseException) -> Exception | RuntimeError: - if isinstance(exc, ValueError): - return Exception() - if isinstance(exc, KeyError): - return RuntimeError() - raise AssertionError( - "only ValueError and KeyError should exist" - ) # pragma: no cover - - try: - gc.set_debug(gc.DEBUG_SAVEALL) - with pytest.raises(MultiError): - # covers MultiErrorCatcher.__exit__ and _multierror.copy_tb - with pytest.warns(TrioDeprecationWarning), MultiError.catch(simple_filter): - raise make_multi() - gc.collect() - assert not gc.garbage - finally: - gc.set_debug(old_flags) - gc.garbage.clear() - - -def assert_match_in_seq(pattern_list: list[str], string: str) -> None: - offset = 0 - print("looking for pattern matches...") - for pattern in pattern_list: - print("checking pattern:", pattern) - reobj = re.compile(pattern) - match = reobj.search(string, offset) - assert match is not None - offset = match.end() - - -def test_assert_match_in_seq() -> None: - assert_match_in_seq(["a", "b"], "xx a xx b xx") - assert_match_in_seq(["b", "a"], "xx b xx a xx") - with pytest.raises(AssertionError): - assert_match_in_seq(["a", "b"], "xx b xx a xx") - - -def test_base_multierror() -> None: - """ - Test that MultiError() with at least one base exception will return a MultiError - object. - """ - - exc = MultiError([ZeroDivisionError(), KeyboardInterrupt()]) - assert type(exc) is MultiError - - -def test_non_base_multierror() -> None: - """ - Test that MultiError() without base exceptions will return a NonBaseMultiError - object. - """ - - exc = MultiError([ZeroDivisionError(), ValueError()]) - assert type(exc) is NonBaseMultiError - assert isinstance(exc, ExceptionGroup) - - -def run_script(name: str) -> subprocess.CompletedProcess[bytes]: - import trio - - trio_path = Path(trio.__file__).parent.parent - script_path = Path(__file__).parent / "test_multierror_scripts" / name - - env = dict(os.environ) - print("parent PYTHONPATH:", env.get("PYTHONPATH")) - pp = [] - if "PYTHONPATH" in env: # pragma: no cover - pp = env["PYTHONPATH"].split(os.pathsep) - pp.insert(0, str(trio_path)) - pp.insert(0, str(script_path.parent)) - env["PYTHONPATH"] = os.pathsep.join(pp) - print("subprocess PYTHONPATH:", env.get("PYTHONPATH")) - - cmd = [sys.executable, "-u", str(script_path)] - print("running:", cmd) - completed = subprocess.run( - cmd, env=env, stdout=subprocess.PIPE, stderr=subprocess.STDOUT - ) - print("process output:") - print(completed.stdout.decode("utf-8")) - return completed - - -@slow -@pytest.mark.skipif( - not Path("/usr/lib/python3/dist-packages/apport_python_hook.py").exists(), - reason="need Ubuntu with python3-apport installed", -) -def test_apport_excepthook_monkeypatch_interaction() -> None: - completed = run_script("apport_excepthook.py") - stdout = completed.stdout.decode("utf-8") - - # No warning - assert "custom sys.excepthook" not in stdout - - # Proper traceback - assert_match_in_seq( - ["--- 1 ---", "KeyError", "--- 2 ---", "ValueError"], - stdout, - ) - - -@pytest.mark.parametrize("protocol", range(0, pickle.HIGHEST_PROTOCOL + 1)) -def test_pickle_multierror(protocol: int) -> None: - # use trio.MultiError to make sure that pickle works through the deprecation layer - import trio - - my_except = ZeroDivisionError() - - try: - 1 / 0 # noqa: B018 # "useless statement" - except ZeroDivisionError as exc: - my_except = exc - - # MultiError will collapse into different classes depending on the errors - for cls, errors in ( - (ZeroDivisionError, [my_except]), - (NonBaseMultiError, [my_except, ValueError()]), - (MultiError, [BaseException(), my_except]), - ): - with warnings.catch_warnings(): - warnings.simplefilter("ignore", TrioDeprecationWarning) - me = trio.MultiError(errors) # type: ignore[attr-defined] - dump = pickle.dumps(me, protocol=protocol) - load = pickle.loads(dump) - assert repr(me) == repr(load) - assert me.__class__ == load.__class__ == cls - - assert me.__dict__.keys() == load.__dict__.keys() - for me_val, load_val in zip(me.__dict__.values(), load.__dict__.values()): - # tracebacks etc are not preserved through pickling for the default - # exceptions, so we only check that the repr stays the same - assert repr(me_val) == repr(load_val) diff --git a/src/trio/_core/_tests/test_multierror_scripts/__init__.py b/src/trio/_core/_tests/test_multierror_scripts/__init__.py deleted file mode 100644 index 26b5192ec7..0000000000 --- a/src/trio/_core/_tests/test_multierror_scripts/__init__.py +++ /dev/null @@ -1,3 +0,0 @@ -# This isn't really a package, everything in here is a standalone script. This -# __init__.py is just to fool setuptools' find = {namespaces = false} into -# actually installing the things. diff --git a/src/trio/_core/_tests/test_multierror_scripts/_common.py b/src/trio/_core/_tests/test_multierror_scripts/_common.py deleted file mode 100644 index 0c70df1840..0000000000 --- a/src/trio/_core/_tests/test_multierror_scripts/_common.py +++ /dev/null @@ -1,7 +0,0 @@ -# https://coverage.readthedocs.io/en/latest/subprocess.html -try: - import coverage -except ImportError: # pragma: no cover - pass -else: - coverage.process_startup() diff --git a/src/trio/_core/_tests/test_multierror_scripts/apport_excepthook.py b/src/trio/_core/_tests/test_multierror_scripts/apport_excepthook.py deleted file mode 100644 index 0e46f37e17..0000000000 --- a/src/trio/_core/_tests/test_multierror_scripts/apport_excepthook.py +++ /dev/null @@ -1,15 +0,0 @@ -# The apport_python_hook package is only installed as part of Ubuntu's system -# python, and not available in venvs. So before we can import it we have to -# make sure it's on sys.path. -import sys - -import _common # isort: split - -sys.path.append("/usr/lib/python3/dist-packages") -import apport_python_hook - -apport_python_hook.install() - -from trio._core._multierror import MultiError # Bypass deprecation warnings - -raise MultiError([KeyError("key_error"), ValueError("value_error")]) diff --git a/src/trio/_core/_tests/test_multierror_scripts/simple_excepthook.py b/src/trio/_core/_tests/test_multierror_scripts/simple_excepthook.py deleted file mode 100644 index 236d34e9ba..0000000000 --- a/src/trio/_core/_tests/test_multierror_scripts/simple_excepthook.py +++ /dev/null @@ -1,21 +0,0 @@ -import _common # isort: split - -from trio._core._multierror import MultiError # Bypass deprecation warnings - - -def exc1_fn() -> Exception: - try: - raise ValueError - except Exception as exc: - return exc - - -def exc2_fn() -> Exception: - try: - raise KeyError - except Exception as exc: - return exc - - -# This should be printed nicely, because Trio overrode sys.excepthook -raise MultiError([exc1_fn(), exc2_fn()]) diff --git a/src/trio/_core/_tests/test_run.py b/src/trio/_core/_tests/test_run.py index 9c39bebe03..71b95c8dcd 100644 --- a/src/trio/_core/_tests/test_run.py +++ b/src/trio/_core/_tests/test_run.py @@ -17,7 +17,6 @@ import sniffio from ... import _core -from ..._core._multierror import MultiError, NonBaseMultiError from ..._threads import to_thread_run_sync from ..._timeouts import fail_after, sleep from ...testing import Sequencer, assert_checkpoints, wait_all_tasks_blocked @@ -184,7 +183,7 @@ async def main() -> None: def test_main_and_task_both_crash() -> None: # If main crashes and there's also a task crash, then we get both in a - # MultiError + # ExceptionGroup async def crasher() -> NoReturn: raise ValueError @@ -193,7 +192,7 @@ async def main() -> NoReturn: nursery.start_soon(crasher) raise KeyError - with pytest.raises(MultiError) as excinfo: + with pytest.raises(ExceptionGroup) as excinfo: _core.run(main) print(excinfo.value) assert {type(exc) for exc in excinfo.value.exceptions} == { @@ -211,7 +210,7 @@ async def main() -> None: nursery.start_soon(crasher, KeyError) nursery.start_soon(crasher, ValueError) - with pytest.raises(MultiError) as excinfo: + with pytest.raises(ExceptionGroup) as excinfo: _core.run(main) assert {type(exc) for exc in excinfo.value.exceptions} == { ValueError, @@ -426,10 +425,14 @@ async def test_cancel_edge_cases() -> None: await sleep_forever() -async def test_cancel_scope_multierror_filtering() -> None: +async def test_cancel_scope_exceptiongroup_filtering() -> None: async def crasher() -> NoReturn: raise KeyError + # check that the inner except is properly executed. + # alternative would be to have a `except BaseException` and an `else` + exception_group_caught_inner = False + # This is outside the outer scope, so all the Cancelled # exceptions should have been absorbed, leaving just a regular # KeyError from crasher() @@ -449,7 +452,8 @@ async def crasher() -> NoReturn: # And one that raises a different error nursery.start_soon(crasher) # t4 # and then our __aexit__ also receives an outer Cancelled - except MultiError as multi_exc: + except BaseExceptionGroup as multi_exc: + exception_group_caught_inner = True # Since the outer scope became cancelled before the # nursery block exited, all cancellations inside the # nursery block continue propagating to reach the @@ -462,8 +466,8 @@ async def crasher() -> NoReturn: summary[type(exc)] += 1 assert summary == {_core.Cancelled: 3, KeyError: 1} raise - else: - raise AssertionError("No ExceptionGroup") + + assert exception_group_caught_inner async def test_precancelled_task() -> None: @@ -784,7 +788,7 @@ async def task2() -> None: RuntimeError, match="which had already been exited" ) as exc_info: await nursery_mgr.__aexit__(*sys.exc_info()) - assert type(exc_info.value.__context__) is NonBaseMultiError + assert type(exc_info.value.__context__) is ExceptionGroup assert len(exc_info.value.__context__.exceptions) == 3 cancelled_in_context = False for exc in exc_info.value.__context__.exceptions: @@ -926,7 +930,7 @@ async def main() -> None: _core.run(main) -def test_system_task_crash_MultiError() -> None: +def test_system_task_crash_ExceptionGroup() -> None: async def crasher1() -> NoReturn: raise KeyError @@ -946,7 +950,7 @@ async def main() -> None: _core.run(main) me = excinfo.value.__cause__ - assert isinstance(me, MultiError) + assert isinstance(me, ExceptionGroup) assert len(me.exceptions) == 2 for exc in me.exceptions: assert isinstance(exc, (KeyError, ValueError)) @@ -954,7 +958,7 @@ async def main() -> None: def test_system_task_crash_plus_Cancelled() -> None: # Set up a situation where a system task crashes with a - # MultiError([Cancelled, ValueError]) + # ExceptionGroup([Cancelled, ValueError]) async def crasher() -> None: try: await sleep_forever() @@ -1206,11 +1210,11 @@ async def test_nursery_exception_chaining_doesnt_make_context_loops() -> None: async def crasher() -> NoReturn: raise KeyError - with pytest.raises(MultiError) as excinfo: # noqa: PT012 + with pytest.raises(ExceptionGroup) as excinfo: # noqa: PT012 async with _core.open_nursery() as nursery: nursery.start_soon(crasher) raise ValueError - # the MultiError should not have the KeyError or ValueError as context + # the ExceptionGroup should not have the KeyError or ValueError as context assert excinfo.value.__context__ is None @@ -1865,7 +1869,8 @@ async def raise_keyerror_after_started( t0 = _core.current_time() with pytest.raises(RuntimeError): await closed_nursery.start(sleep_then_start, 7) - assert _core.current_time() == t0 + # sub-second delays can be caused by unrelated multitasking by an OS + assert int(_core.current_time()) == int(t0) async def test_task_nursery_stack() -> None: @@ -2030,9 +2035,9 @@ async def my_child_task() -> NoReturn: with pytest.raises(ExceptionGroup) as excinfo: # noqa: PT012 # Trick: For now cancel/nursery scopes still leave a bunch of tb gunk - # behind. But if there's a MultiError, they leave it on the MultiError, + # behind. But if there's an ExceptionGroup, they leave it on the group, # which lets us get a clean look at the KeyError itself. Someday I - # guess this will always be a MultiError (#611), but for now we can + # guess this will always be an ExceptionGroup (#611), but for now we can # force it by raising two exceptions. async with _core.open_nursery() as nursery: nursery.start_soon(my_child_task) @@ -2379,6 +2384,9 @@ async def test_cancel_scope_deadline_duplicates() -> None: await sleep(0.01) +# I don't know if this one can fail anymore, the `del` next to the comment that used to +# refer to this only seems to break test_cancel_scope_exit_doesnt_create_cyclic_garbage +# We're keeping it for now to cover Outcome and potential future refactoring @pytest.mark.skipif( sys.implementation.name != "cpython", reason="Only makes sense with refcounting GC" ) @@ -2441,7 +2449,7 @@ async def crasher() -> NoReturn: outer.cancel() # And one that raises a different error nursery.start_soon(crasher) - # so that outer filters a Cancelled from the MultiError and + # so that outer filters a Cancelled from the ExceptionGroup and # covers CancelScope.__exit__ (and NurseryManager.__aexit__) # (See https://github.com/python-trio/trio/pull/2063) @@ -2521,7 +2529,9 @@ async def main() -> NoReturn: async with _core.open_nursery(): raise Exception("foo") - with pytest.raises(MultiError) as exc: + with pytest.raises( + ExceptionGroup, match="^Exceptions from Trio nursery \\(1 sub-exception\\)$" + ) as exc: _core.run(main, strict_exception_groups=True) assert len(exc.value.exceptions) == 1 @@ -2545,7 +2555,7 @@ async def main() -> NoReturn: async def test_nursery_strict_exception_groups() -> None: """Test that strict exception groups can be enabled on a per-nursery basis.""" - with pytest.raises(MultiError) as exc: + with pytest.raises(ExceptionGroup) as exc: async with _core.open_nursery(strict_exception_groups=True): raise Exception("foo") @@ -2554,6 +2564,30 @@ async def test_nursery_strict_exception_groups() -> None: assert exc.value.exceptions[0].args == ("foo",) +async def test_nursery_loose_exception_groups() -> None: + """Test that loose exception groups can be enabled on a per-nursery basis.""" + + async def raise_error() -> NoReturn: + raise RuntimeError("test error") + + with pytest.raises(RuntimeError, match="^test error$"): + async with _core.open_nursery(strict_exception_groups=False) as nursery: + nursery.start_soon(raise_error) + + with pytest.raises( # noqa: PT012 # multiple statements + ExceptionGroup, match="^Exceptions from Trio nursery \\(2 sub-exceptions\\)$" + ) as exc: + async with _core.open_nursery(strict_exception_groups=False) as nursery: + nursery.start_soon(raise_error) + nursery.start_soon(raise_error) + + assert exc.value.__notes__ == [_core._run.NONSTRICT_EXCEPTIONGROUP_NOTE] + assert len(exc.value.exceptions) == 2 + for subexc in exc.value.exceptions: + assert type(subexc) is RuntimeError + assert subexc.args == ("test error",) + + async def test_nursery_collapse_strict() -> None: """ Test that a single exception from a nested nursery with strict semantics doesn't get @@ -2563,7 +2597,7 @@ async def test_nursery_collapse_strict() -> None: async def raise_error() -> NoReturn: raise RuntimeError("test error") - with pytest.raises(MultiError) as exc: # noqa: PT012 + with pytest.raises(ExceptionGroup) as exc: # noqa: PT012 async with _core.open_nursery() as nursery: nursery.start_soon(sleep_forever) nursery.start_soon(raise_error) @@ -2575,7 +2609,7 @@ async def raise_error() -> NoReturn: exceptions = exc.value.exceptions assert len(exceptions) == 2 assert isinstance(exceptions[0], RuntimeError) - assert isinstance(exceptions[1], MultiError) + assert isinstance(exceptions[1], ExceptionGroup) assert len(exceptions[1].exceptions) == 1 assert isinstance(exceptions[1].exceptions[0], RuntimeError) @@ -2589,7 +2623,7 @@ async def test_nursery_collapse_loose() -> None: async def raise_error() -> NoReturn: raise RuntimeError("test error") - with pytest.raises(MultiError) as exc: # noqa: PT012 + with pytest.raises(ExceptionGroup) as exc: # noqa: PT012 async with _core.open_nursery() as nursery: nursery.start_soon(sleep_forever) nursery.start_soon(raise_error) diff --git a/src/trio/_highlevel_open_tcp_stream.py b/src/trio/_highlevel_open_tcp_stream.py index f6f8aba425..d5c83da7c0 100644 --- a/src/trio/_highlevel_open_tcp_stream.py +++ b/src/trio/_highlevel_open_tcp_stream.py @@ -5,7 +5,6 @@ from typing import TYPE_CHECKING, Any import trio -from trio._core._multierror import MultiError from trio.socket import SOCK_STREAM, SocketType, getaddrinfo, socket if TYPE_CHECKING: @@ -13,7 +12,7 @@ from socket import AddressFamily, SocketKind if sys.version_info < (3, 11): - from exceptiongroup import ExceptionGroup + from exceptiongroup import BaseExceptionGroup, ExceptionGroup # Implementation of RFC 6555 "Happy eyeballs" @@ -130,7 +129,7 @@ def close_all() -> Generator[set[SocketType], None, None]: if len(errs) == 1: raise errs[0] elif errs: - raise MultiError(errs) + raise BaseExceptionGroup("", errs) def reorder_for_rfc_6555_section_5_4( diff --git a/src/trio/_tests/test_exports.py b/src/trio/_tests/test_exports.py index 60db11330f..4138df0e5e 100644 --- a/src/trio/_tests/test_exports.py +++ b/src/trio/_tests/test_exports.py @@ -316,10 +316,6 @@ def lookup_symbol(symbol: str) -> dict[str, str]: continue if module_name == "trio.socket" and class_name in dir(stdlib_socket): continue - # Deprecated classes are exported with a leading underscore - # We don't care about errors in _MultiError as that's on its way out anyway - if class_name.startswith("_"): # pragma: no cover - continue # dir() and inspect.getmembers doesn't display properties from the metaclass # also ignore some dunder methods that tend to differ but are of no consequence diff --git a/test-requirements.in b/test-requirements.in index 683eeb21db..0cc6f73285 100644 --- a/test-requirements.in +++ b/test-requirements.in @@ -30,4 +30,5 @@ sortedcontainers idna outcome sniffio -exceptiongroup >= 1.0.0rc9; python_version < "3.11" +# 1.2.0 ships monkeypatching for apport excepthook +exceptiongroup >= 1.2.0; python_version < "3.11"