Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove multierror #2891

Merged
merged 25 commits into from
Dec 21, 2023
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
5ff3464
remove multierror
jakkdl Nov 27, 2023
9c23ab0
remove more unused code, fix test_cancel_scope_multierror_filtering i…
jakkdl Nov 27, 2023
4cb329f
fix remaining failing tests, instantly collapse groups in nurseries i…
jakkdl Nov 28, 2023
3822dc1
bump exceptiongroup version to one that has the apport excepthook
jakkdl Nov 28, 2023
f52ea57
remove multierror_scripts, rename _core/_multierror.py to _core/_conc…
jakkdl Nov 28, 2023
4d600bc
edit some comments, remove test_ExceptionGroupNotHashable
jakkdl Nov 28, 2023
091e962
Merge branch 'master' into multierror_removal
jakkdl Nov 28, 2023
8246daf
docs: update some references to MultiError, remove section about main…
jakkdl Nov 28, 2023
a7a5a71
add newsfragment
jakkdl Nov 28, 2023
bfca5a5
Merge remote-tracking branch 'origin/master' into multierror_removal
jakkdl Nov 28, 2023
5dd4b95
update gc comments to my current state of understanding as to what ca…
jakkdl Nov 29, 2023
47ef84a
Merge remote-tracking branch 'origin/master' into multierror_removal
jakkdl Dec 16, 2023
95252ac
Merge remote-tracking branch 'origin/master' into multierror_removal
jakkdl Dec 16, 2023
9c1400c
move 'collapsible' to __notes__, clean up some stray unused-ignores, …
jakkdl Dec 16, 2023
907a2c5
update exceptiongroup message, test for it with a match
jakkdl Dec 16, 2023
3319a01
fix expected message
jakkdl Dec 16, 2023
9941762
fix mypy (this should be checked by pre-commit...)
jakkdl Dec 16, 2023
6977353
add test for loose exceptiongroup collapsing, rename test_multierror
jakkdl Dec 18, 2023
bd72a32
fix test
jakkdl Dec 18, 2023
0439e66
Apply suggestions from code review
jakkdl Dec 19, 2023
e19f341
update comments after review from a5rocks
jakkdl Dec 19, 2023
b0eb6b4
make the note more verbose
jakkdl Dec 20, 2023
d4463eb
Merge branch 'master' into multierror_removal
jakkdl Dec 20, 2023
f433fdb
fix the test
jakkdl Dec 20, 2023
3e485dc
update comment
jakkdl Dec 21, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions docs/source/reference-core.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
++++++++++++++++++++++++++++++++++++++++++++++++

Expand Down
2 changes: 1 addition & 1 deletion docs/source/tutorial.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
1 change: 1 addition & 0 deletions newsfragments/2891.deprecated.rst
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 0 additions & 4 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down Expand Up @@ -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.
Expand Down
25 changes: 1 addition & 24 deletions src/trio/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I guess any release with these should be 0.24.0? Release preparation page says "two months or two releases, whichever is longer") and I assume it means minor releases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense to do major (or whatever 0.X releases are called) release on both this and on #2886 since they can break stuff.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't get why Trio doesn't have a 1.0.0 release yet if compatibility guarantees are this stringent anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be inclined to go 1.0 when we've got this and strict-by-default in; I agree but it seems odd to make that release right before some substantial changes.

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
Expand Down
129 changes: 129 additions & 0 deletions src/trio/_core/_concat_tb.py
Original file line number Diff line number Diff line change
@@ -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:

Check warning on line 87 in src/trio/_core/_concat_tb.py

View check run for this annotation

Codecov / codecov/patch

src/trio/_core/_concat_tb.py#L87

Added line #L87 was not covered by tests
# 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]

Check warning on line 90 in src/trio/_core/_concat_tb.py

View check run for this annotation

Codecov / codecov/patch

src/trio/_core/_concat_tb.py#L90

Added line #L90 was not covered by tests
# 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

Check warning on line 105 in src/trio/_core/_concat_tb.py

View check run for this annotation

Codecov / codecov/patch

src/trio/_core/_concat_tb.py#L105

Added line #L105 was not covered by tests

return cast(

Check warning on line 107 in src/trio/_core/_concat_tb.py

View check run for this annotation

Codecov / codecov/patch

src/trio/_core/_concat_tb.py#L107

Added line #L107 was not covered by tests
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
Loading