classification
Title: unrelated `from None` exceptions hide prior exception information
Type: behavior Stage: patch review
Components: Versions: Python 3.9
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, ethan.furman, ncoghlan, rhettinger, serhiy.storchaka, terry.reedy
Priority: normal Keywords: patch

Created on 2020-02-22 21:05 by ethan.furman, last changed 2020-03-30 23:45 by Ido Michael.

Pull Requests
URL Status Linked Edit
PR 18629 open serhiy.storchaka, 2020-02-23 22:37
Messages (10)
msg362478 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2020-02-22 21:05
Using the example from https://bugs.python.org/msg293185:

-----------------------------------------------------------------------
--> import os
--> try:
...     os.environ["NEW_VARIABLE"] = bug  # bug is not a str
... finally:
...     del os.environ["NEW_VARIABLE"]  # KeyError
... 
Traceback (most recent call last):
  ...
KeyError: 'NEW_VARIABLE'
-----------------------------------------------------------------------

We lost the original exception, `TypeError: str expected, not object`, because in os.py we have:

    def __delitem__(self, key):
        encodedkey = self.encodekey(key)
        unsetenv(encodedkey)
        try:
            del self._data[encodedkey]
        except KeyError:
            # raise KeyError with the original key value
            raise KeyError(key) from None


If we remove the `from None` the result of the above code is:

-----------------------------------------------------------------------
Traceback (most recent call last):
TypeError: str expected, not type

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
KeyError: b'NEW_VARIABLE'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
KeyError: 'NEW_VARIABLE'
-----------------------------------------------------------------------


There are various tricks we can do to fix this isolated issue (and others like it), but the real problem is that one exception handler's work was destroyed by an unrelated exception handler.

The intent of `from None` is to get rid of any exception details in the try/except block it is contained within, not to lose details from exceptions that were already in play when its try/except block was entered.

Any ideas on how to correct this?
msg362481 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-02-22 22:00
Interesting problem.

Example which does not depend on os.environ:

import traceback
try:
    try:
        raise TypeError
    except:
        try:
            raise ValueError
        except:
            raise KeyError from None
except BaseException as exc:
    e = exc

print([e, e.__cause__, e.__context__, e.__suppress_context__])
traceback.print_exception(type(e), e, e.__traceback__)


"from None" sets __suppress_context__ to True. Currently, if __suppress_context__ is true, the output of __context__ will be suppressed in the traceback. But what if we suppress only a part __context__ and output the "original" exception. The problem is what is the "original" exception. Is it __context__.__context__? Or __context__.__cause__ if it is not None? Should we take __context__.__suppress_context__ into account? Maybe go down recursively?
msg362492 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-02-23 07:31
For reference: PEP 3134 (and superseded PEP 344), PEP 409, PEP 415.
msg362531 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2020-02-23 18:24
I think the way forward is going to be a recursive walk back to the first exception, and if __suppress_context__ is true for any exception then only the previous exception is omitted.

For example, the above example had the following chain:

Exception    __context__    __cause__    __suppress_context__
---------    -----------    ---------    --------------------
TypeError:     None           None          False
KeyError:      TypeError      None          False
KeyError:      KeyError       None          True

When walking the stack to decide which items get printed, the final KeyError is printed (obviously, as it's the last one), but because its __suppress_context__ is True then the immediately preceding exception, the first KeyError, is skipped; however, the first KeyError's __suppress_context__ is False, so we do print its __context__, which is the TypeError.  Giving us a traceback similar to:

---------------------------------------------------------------------
Traceback (most recent call last):
TypeError: str expected, not type

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
KeyError: 'NEW_VARIABLE'
---------------------------------------------------------------------

Which is entirely correct.

In cases where NewException has it's __cause__ set, we should print that as normal, then keep following the NewException.__context__ chain (with some kind of verbage or visual indicator between the __context__ and the __cause__).
msg362532 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2020-02-23 18:26
Can this be fixed in the traceback module or is there C code behind it?
msg362543 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-02-23 22:13
It is more complicated. In the following example

try:
    try:
        raise TypeError
    except:
        try:
            try:
                raise OverflowError
            except:
                raise ValueError
        except:
            raise KeyError from None
except BaseException as exc:
    e = exc

the __context__ chain is KeyError -> ValueError -> OverflowError -> TypeError. We want to suppress ValueError and OverflowError, and keep KeyError -> TypeError. I afraid that the chain of exceptions just do not have enough information to do this. Maybe analyzing tracebacks can help, but it looks too complex.

Maybe the simpler solution is to "cut" __context__ when we leave an exception handler. If __suppress_context__ is true and the current handled exception is not __context__, set it as __context__.

Here is a PR which implements this solution. I am not sure that it is correct, and it may miss some corner cases, it is just an example. Pay attention to the ipaddress module: in some cases we want to suppress more than one exception, and currently it is inconvenient.


Maybe we incorrectly handle exception chains. Maybe they should be build in a way similar to tracebacks: initially set __context__ to None and add a handled exception to the end of the chain when leave an exception handler.
msg362943 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2020-02-29 00:00
Would it generaly work better if 'from None' only suppressed the previous exception?  That is the only one that the code writer knows about.
msg362965 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-02-29 06:46
Because there may be more exceptions than the code writer know. The code raised OverflowError and ValueError may be in different function, so the code writer know only about ValueError, but OverflowError is an implementation detail (and may be absent in other versions).

Currently the code writer has to write the cumbersome code

        was_raised = False
        try:
            foo()  # may raise ValueError -> OverflowError
        except:
            was_raised = True
        if was_raised:
            raise KeyError from None

if they want to suppress only exceptions raised in foo(). And it is worse in case of finally.
msg364241 - (view) Author: Ido Michael (Ido Michael) * Date: 2020-03-15 15:45
Can I take this change into a PR?
Or is it still in the process?
msg364272 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2020-03-15 23:35
Tweaked title to be "hide" rather "lose" (all the info is theoretically still there in various places, it's just hidden by the default traceback printing machinery, and hard to extract from the exception tree).

Issue #18861 is the original report of this problem.

My concern with Serhiy's suggestion is that it would take us from merely hiding information to actually losing it.

I'm wondering if we're actually missing a piece of lower level infrastructure: a query API that reports *all* live exceptions in a thread.

For example, there might be a "sys.active_exceptions()" API that returned a list of all active exceptions.

* []: No exception active
* [exc]: Running an except/finally/__exit__ for "exc"
* [exc_inner, exc_outer]: Running an except/finally/__exit__ for "exc_inner" inside one for "exc_outer"
* etc

So in Serhiy's example, at the point where the context is suppressed, that new API would report [ValueError(), TypeError()] (with the relevant caught exception instances).

Given that extra low level API, we would gain more options for populating attributes on exceptions.

Alternatively, Serhiy's suggested approach might be sufficient, even without that new API, if we added a new __suppressed_context__ attribute to capture the original value of __context__ before it was replaced.
History
Date User Action Args
2020-03-30 23:45:47Ido Michaelsetnosy: - Ido Michael
2020-03-15 23:35:40ncoghlansetmessages: + msg364272
title: unrelated `from None` exceptions lose prior exception information -> unrelated `from None` exceptions hide prior exception information
2020-03-15 15:45:32Ido Michaelsetnosy: + Ido Michael
messages: + msg364241
2020-02-29 06:46:36serhiy.storchakasetmessages: + msg362965
2020-02-29 00:00:14terry.reedysetnosy: + terry.reedy
messages: + msg362943
2020-02-23 22:37:02serhiy.storchakasetkeywords: + patch
stage: patch review
pull_requests: + pull_request17996
2020-02-23 22:13:18serhiy.storchakasetmessages: + msg362543
2020-02-23 18:26:07ethan.furmansetmessages: + msg362532
2020-02-23 18:24:47ethan.furmansetmessages: + msg362531
2020-02-23 07:31:54serhiy.storchakasetmessages: + msg362492
2020-02-22 22:00:45serhiy.storchakasetnosy: + benjamin.peterson
messages: + msg362481
2020-02-22 21:05:03ethan.furmancreate