Skip to content

Using Generic Exceptions (ExceptionGroups in particular) with raises in a type-safe manner is not ideal #13115

Closed
@tapetersen

Description

@tapetersen
Contributor

As I understand the arguments for not extending pytest.raises to automatically unwrap ExceptionGroupsas discussed in #11538 I've tried to use the ExceptionInfo.group_contains() method which works well enough for the runtime parts.

However since the return value of raises() ( ExceptionInfo) is generic in the exception type, the stricter type-checkers will complain about the unknown type-argument of the ExceptionGroup.

# pyright: strict

import pytest
import sys
from typing import cast

if sys.version_info < (3, 11):
    from exceptiongroup import ExceptionGroup

class FooError(Exception):
    pass

# Pyright will report an error for the code below as:
# Type of "exc_group_info" is partially unknown
#   Type of "exc_group_info" is "ExceptionInfo[ExceptionGroup]"PylancereportUnknownVariableType
with pytest.raises(ExceptionGroup) as exc_group_info:
    raise ExceptionGroup("Some error occured", [FooError("Error")])

assert exc_group_info.group_contains(FooError)

If trying to rectify this by supplying the type-argument it fails various checks in raises that checks that the argument is an instance of type which GenericAlias (that you get when indexing a generic class) is not.

# If the type is added pyright is ok but the runtime is not:
# Traceback (most recent call last):
#  File ".../test.py", line 21, in <module>
#    with pytest.raises(ExceptionGroup[ExceptionGroup[FooError]]) as exc_group_info:
#         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
#  File ".../.venv/lib/python3.12/site-packages/_pytest/python_api.py", line 959, in raises
#    raise TypeError(msg.format(not_a))
# TypeError: expected exception must be a BaseException type, not GenericAlias
with pytest.raises(ExceptionGroup[ExceptionGroup[FooError]]) as exc_group_info:
    raise ExceptionGroup("Some error occured", [FooError("Error")])

assert exc_group_info.group_contains(FooError)

The correct casting that is required to get this correct would have to be something like below.

# With the cast we can get it to work but that is quite verbose and doesn't
# catch errors of BaseExceptionGroup  vs ExceptionGroup in the cast for example.
with pytest.raises(cast(type[ExceptionGroup[FooError]], ExceptionGroup)) as exc_group_info:
    raise ExceptionGroup("Some error occured", [FooError("Error")])

assert exc_group_info.group_contains(FooError)

This could probably be fixed by handling generic Exceptions (or at least ExceptionGroups) explicitly in raises using https://docs.python.org/3/library/typing.html#typing.get_origin.

Activity

changed the title [-]Using Generic Exceptions (ExceptionGroups in particular) with raises in a type-safe manner is not ideal.[/-] [+]Using Generic Exceptions (ExceptionGroups in particular) with raises in a type-safe manner is not ideal[/+] on Jan 7, 2025
Zac-HD

Zac-HD commented on Jan 9, 2025

@Zac-HD
Member

Yep, I think calling get_origin() on generics to 'unwrap' the bare type as part of this code would be a great fix. Would you be interested in opening a PR?

tapetersen

tapetersen commented on Jan 10, 2025

@tapetersen
ContributorAuthor

I'll take a shot at it.

The only hesitation is that it may look somewhat misleading in that we would still actually catch all ExceptionGroups and don't validate that only the given exception class is actually contained.

i.e. the following satisfying the logic of raises without a single ValueError being thrown.

with pytest.raises(ExceptionGroup[ValueError]) as exc_info:
    raise ExceptionGroup("Some errors occurred", [RuntimeException("Normal error occurred")])

# Of course if you add an assert group_contains which is how I would use it
assert exc_info.group_contains(ValueError)

There are some similarities to the standard isinstance(list_of_ints, list[str]) where the choice was to not support it.

I personally still think practicality beats purity here and that the narrower focus and implied experience-level of pytest users (especially users of pytest.raises with ExceptionGroup and strong typing) but wanted to check.

Of course you can dig deeper and recognize a parametrized ExceptionGroup, extract the parametrized type and if it's an exception do some further magic with ExceptionGroup split but that feels a bit too far with an endless can of worms waiting.

Zac-HD

Zac-HD commented on Jan 10, 2025

@Zac-HD
Member

The only hesitation is that it may look somewhat misleading in that we would still actually catch all ExceptionGroups and don't validate that only the given exception class is actually contained.

Hmm, that's a good point, and actually does make me more nervous about just unwrapping the generic - "tests check less than the developer expected" is a classic way for bugs to sneak through to production.

We've actually been working on #12504 via trio.testing.RaisesGroup() (eg in python-trio/trio#3145), with the goal of merging it into Pytest once the API has been shaken out a bit. Maybe we could push people towards RaisesGroup if their annotation is more precise than (Base)ExceptionGroup[Any / (Base)Exception], so there's a clean syntax that strict type-checkers allow but without leaving the implication that we're checking anything more precise than we actually are? (which would imply waiting, ugh)

tapetersen

tapetersen commented on Jan 10, 2025

@tapetersen
ContributorAuthor

For all my uses just always using the full ExceptionGroup[Exception] or similar even with ExceptionGroup[Any] would be fine but all of those still results in a GenericAlias which fails at runtime.

The real annoyance and problem for me/us here is really only that ExceptionGroup is generic but without a default type-argument which causes pyright/pylance to complain about the resulting type of the return value being incomplete/unspecified on the stricter setting and there being no really good place to add it as you can't simply type-annotate the result of a context-manager.

I do realize that is a quite narrow and specific concern though with annoying/ugly but fully functional workarounds so this is not really something pressing.

Loose thought without thinking too deep would be if it would be worth it if runtime could accept just the explicit common types ExceptionGroup[Exception], BaseExceptionGroup[BaseException] and maybe Any as well for argument and raise for anything else.

Zac-HD

Zac-HD commented on Jan 11, 2025

@Zac-HD
Member

Yep, I'd be happy to accept a PR for that minimal version anytime.

added a commit that references this issue on Jan 15, 2025
4d36df3
tapetersen

tapetersen commented on Jan 16, 2025

@tapetersen
ContributorAuthor

I added a PR with discussed fixes (Only accepts specifically ExceptionGroup[Exception] and BaseExceptionGroup[Exception]. with a hopefully meaningful error message for others).

A bit larger patch than I would've liked due to the need to handle them in tuples as well but seems to work well both on versions with native support and without. Was a bit unsure about where to add the tests so some feedback on the PR would be welcome.

added 4 commits that reference this issue on Jan 16, 2025
0d4ef8c
77c167d
d6872e3
dff8750

3 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    topic: typingtype-annotation issuetype: bugproblem that needs to be addressed

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @tapetersen@Zac-HD

      Issue actions

        Using Generic Exceptions (ExceptionGroups in particular) with raises in a type-safe manner is not ideal · Issue #13115 · pytest-dev/pytest