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

Exception chaining accepts exception classes #87168

Closed
cool-RR mannequin opened this issue Jan 22, 2021 · 5 comments
Closed

Exception chaining accepts exception classes #87168

cool-RR mannequin opened this issue Jan 22, 2021 · 5 comments
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@cool-RR
Copy link
Mannequin

cool-RR mannequin commented Jan 22, 2021

BPO 43002
Nosy @mdickinson, @stevendaprano, @cool-RR, @miss-islington
PRs
  • bpo-43002: Fix description of behaviour of an exception class in 'from' clause #24303
  • [3.9] bpo-43002: Fix description of behaviour of an exception class in 'from' clause (GH-24303) #25341
  • [3.8] bpo-43002: Fix description of behaviour of an exception class in 'from' clause (GH-24303) #25342
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2021-01-22.22:29:37.398>
    created_at = <Date 2021-01-22.13:52:41.081>
    labels = ['interpreter-core', 'type-bug', '3.8', '3.9', '3.10', 'invalid']
    title = 'Exception chaining accepts exception classes'
    updated_at = <Date 2021-04-11.08:49:04.101>
    user = 'https://github.com/cool-RR'

    bugs.python.org fields:

    activity = <Date 2021-04-11.08:49:04.101>
    actor = 'miss-islington'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-01-22.22:29:37.398>
    closer = 'steven.daprano'
    components = ['Interpreter Core']
    creation = <Date 2021-01-22.13:52:41.081>
    creator = 'cool-RR'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 43002
    keywords = []
    message_count = 5.0
    messages = ['385499', '385514', '385525', '385530', '385531']
    nosy_count = 4.0
    nosy_names = ['mark.dickinson', 'steven.daprano', 'cool-RR', 'miss-islington']
    pr_nums = ['24303', '25341', '25342']
    priority = 'normal'
    resolution = 'not a bug'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue43002'
    versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

    @cool-RR
    Copy link
    Mannequin Author

    cool-RR mannequin commented Jan 22, 2021

    I saw this line of code today: hyperledger-archives/sawtooth-sdk-python@c27b962#diff-eb008203eae2160c5e14c42e5fd2eee164709a93bf5136fa79cc256d4e46eaffR92

    I was about to tell this guy that his code is bad since the part after the from should be a specific exception, not just an exception class. I thought I should double-check myself first. Lo and behold:

    $ cat x.py
    def f():
        raise TypeError
    
    try:
        f()
    except TypeError:
        raise ValueError from TypeError
    
    $ python x.py
    TypeError
    
    The above exception was the direct cause of the following exception:
    
    Traceback (most recent call last):
      File "x.py", line 7, in <module>
        raise ValueError from TypeError
    ValueError
    

    The line doesn't fail, but it silently trims away the stacktrace of the previous exception. (Normally the stacktrace would include what's inside the f function.)

    I'm not sure whether this is intended behavior or not. The documentation does say "the second expression must be another exception class or instance" but (a) it's not clear what the use case is when it's a class and (b) I doubt that the person who wrote the code above, and any other person who writes such code, would be aware that their traceback got snipped until it's too late and they're stuck with a bug report with incomplete information.

    @cool-RR cool-RR mannequin added 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Jan 22, 2021
    @stevendaprano
    Copy link
    Member

    It is intended behaviour. raise ... from is a general mechanism that you can call anywhere, it is not just limited to raising from the previous exception. It is designed for explicitly setting the chained exception to some arbitrary exception. See the PEP:

    https://www.python.org/dev/peps/pep-3134/#explicit-exception-chaining

    >>> raise ValueError('bad value') from IndexError('bad index')
    IndexError: bad index
    
    The above exception was the direct cause of the following exception:
    
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    ValueError: bad value

    If you wish to chain from the current exception, you have to chain from the current exception, and not create a new one.

    This is not an interpreter bug, it's working fine. Your first instinct was correct: your colleague had written "bad code" and his intention was probably to raise from the exception he had just caught.

    Or just don't do anything. A raise exception inside an except block will be automatically chained with the current exception unless you explicitly disable it with raise exception from None.

    @cool-RR
    Copy link
    Mannequin Author

    cool-RR mannequin commented Jan 23, 2021

    People barely know how to use the right form of this feature. Providing them with a wrong form that's confusingly similar to the right form and fails silently is an unfortunate choice.

    "Or just don't do anything. A raise exception inside an except block will be automatically chained with the current exception [...]"

    With the wrong message, yes.

    @stevendaprano
    Copy link
    Member

    How do you "the wrong message" to implicitly chain exceptions rather
    than explicitly?

    The difference between:

     try:
         len(1)
     except TypeError as e:
         raise ValueError(msg) from e
    

    and

     try:
         len(1)
     except TypeError as e:
       	 raise ValueError(msg)
    

    is that the first traceback says:

    "The above exception was the direct cause of the following exception"

    and the second says:

    "During handling of the above exception, another exception occurred"

    Both messages are correct, but if the difference beween the two matters
    to you, feel free to use whichever form you prefer.

    @mdickinson
    Copy link
    Member

    Ram: I think you're conflating two separate things, here:

    (1) The ability to use an exception *class* instead of an exception *instance* in the "from" clause - that is, the ability to do "raise ValueError from TypeError" in place of "raise ValueError from TypeError()"

    (2) The lack of a traceback from the local exception-handling context when doing raise from.

    The two are independent: you'll see the same lack of traceback that you described if you do "raise ValueError from TypeError()" instead of "raise ValueError from TypeError".

    Both behaviours are by design (as Steven already pointed out for (2)). However, on point (1), there may be a documentation bug here. The reference manual, under https://docs.python.org/3/reference/simple_stmts.html#the-raise-statement, says:

    The from clause is used for exception chaining: if given, the second expression must be another exception class or instance, which will then be attached to the raised exception as the __cause__ attribute (which is writable).

    However, this description appears not to match the implementation. In the case that the second expression is an exception class, it's *not* attached to the raised exception as the __cause__ attribute. Instead, the exception class is first instantiated, and then the resulting exception *instance* is attached to the raised exception as the __cause__ attribute.

    The corresponding part of the implementation is here:

    cpython/Python/ceval.c

    Lines 4758 to 4763 in b745a61

    if (PyExceptionClass_Check(cause)) {
    fixed_cause = _PyObject_CallNoArg(cause);
    if (fixed_cause == NULL)
    goto raise_error;
    Py_DECREF(cause);
    }

    Demonstration:

        >>> try:
        ...     raise ZeroDivisionError() from RuntimeError
        ... except Exception as e:
        ...     exc = e
        ... 
        >>> exc.__cause__
        RuntimeError()
        >>> exc.__cause__ is RuntimeError  # reference manual would suggest this is True
        False
        >>> isinstance(exc.__cause__, RuntimeError)  # actual behaviour
        True

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes 3.9 only security fixes 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants