GeezerVison is showing github.com — Subclasses of `ExceptionGroup` can wrap `BaseException`s · Issue #99553 · python/cpython
Subclasses of ExceptionGroup can wrap BaseExceptions #99553Labels 3.11only security fixesonly security fixes3.12only security fixesonly security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error Descriptionclass MyEG(ExceptionGroup):
"""Holds BaseExceptions without itself being a BaseException."""
oops = MyEG("oops", [KeyboardInterrupt()])
assert isinstance(oops, Exception)
assert not isinstance(oops.exceptions[0], Exception)I believe that this is a bug tracing to the period when PEP-654 did not intend (Base)ExceptionGroup to be usable as a parent class; and I think a sufficient fix would be to replace the type-equality check with an isinstance check in:
|
if (cls == PyExc_ExceptionGroup) { |
|
if (nested_base_exceptions) { |
|
PyErr_SetString(PyExc_TypeError, |
|
"Cannot nest BaseExceptions in an ExceptionGroup"); |
|
goto error; |
cc @iritkatriel; raised via agronholm/exceptiongroup#40 (comment)
Linked PRs
Activityiritkatriel commented on Nov 17, 2022 I believe we discussed this and decided that we leave it to the subclass author to deal with it. Maybe it needs to be documented (or reconsidered)?
CC @gvanrossum @1st1 Zac-HD commented on Nov 17, 2022 ContributorAuthor More actions I think at least documented, though I'd prefer it was changed.
If people intend MyEG to contain BaseExceptions then the obvious way is to inherit from BaseExceptionGroup (and likely have a BaseMyEg; Trio has already shipped this pattern), so any cases where MyEG ends up containing e.g. a KeyboardInterrupt and then discarding that to an except Exception: would be unfortunate! I think the PEP argues against this well 🙂
The == comparison also makes static typing very confusing: if we have BaseExceptionGroup -> ExceptionGroup -> MyEG and the last can also contain BaseExceptions, then either ExceptionGroup.exceptions must be annotated as able to contain BaseExceptions when it cannot, the annotations for MyEG must say it cannot when in fact it can, or we violate the substitution principle. This last is currently the case at runtime, but typecheckers are not keen on such things. Zac-HD commented on Nov 17, 2022 ContributorAuthor More actions Hah! It looks like I just find this persistently startling: #28569 (comment) (but I promise, no PEP this time 😉) gvanrossum commented on Nov 17, 2022 Yeah, I definitely recall that we discussed this before. But looking at the code it's possible that we were too conservative. In particular in the fallback clause we could have chosen to check if cls is a subclass of PyExc_ExceptionGroup if nested_base_exceptions is set and raise the same error as on line 742.
The thing we definitely can't do is adjusting the class if it is exactly PyExc_BaseExceptionGroup and nested_base_exceptions is false.
The question is, can we treat this as a bug and fix it (in 3.11.1 if it isn't out yet) or would such a fix be considered backwards compatible? iritkatriel commented on Nov 17, 2022 I think we can call it a bug. It’s better to change it in 3.11.1 than in 3.12. Reacted by Zac Hatfield-Dodds gvanrossum commented on Nov 17, 2022 added a commit that references this issue on Nov 18, 2022gh-99553: fix bug where an ExceptionGroup subclass can wrap a BaseExc… added 2 commits that reference this issue on Nov 18, 2022pythongh-99553: fix bug where an ExceptionGroup subclass can wrap a B… gh-99553: fix bug where an ExceptionGroup subclass can wrap a BaseExc…
MetadataMetadata3.11only security fixesonly security fixes3.12only security fixesonly security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error DevelopmentNo branches or pull requestsIssue actions
|