Issue 23763: Chain exceptions in C (original) (raw)

Created on 2015-03-24 14:41 by vstinner, last changed 2022-04-11 14:58 by admin. This issue is now closed.

| Messages (14) | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------ | ---------------------------------------------------------------------- | --------------------------- | ---------------------------------------------------------- | | ------------------------------------------------------------------- | | ---------------------------------- | ------------------------------------ | -------------------------------------------------------------------------- | -------------------------------------------------------- | ---------------------------------- | ------------------------------------ | -------------------------------------------------- | --------------------------------------------------------- | -------------------------------------------------------- | | ------------------------------------------------------------------- | | ---------------------------------- | ------------------------------------ | --------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------ | | ------------------------------------------------------------------- | | ---------------------------------- | ----------------------------------- | ----------------------------------------------------------------------------------------------------- | ---------------------------------------- | | ------------------------------------------------------------------- | | ---------------------------------- | ----------------------------------- | --------------------------------------------------- | ------------------------------ | | ------------------------------------------------------------------- | | ---------------------------------- | ----------------------------------- | ------------------------------------------- | | msg239130 - (view) | Author: STINNER Victor (vstinner) * | Date: 2015-03-24 14:41 | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In Python 3, it becomes possible to chain two exceptions. It's one of the killer feature of Python 3, it helps debugging. In Python, exceptions are chained by default. Example: try: raise TypeError("old message") except TypeError: raise ValueError("new message") Output: Traceback (most recent call last): File "x.py", line 2, in raise TypeError("old message") TypeError: old message During handling of the above exception, another exception occurred: Traceback (most recent call last): File "x.py", line 4, in raise ValueError("new message") ValueError: new message In C, using the public PyErr_SetNone(), PyErr_Format(), PyErr_SetString(), ... functions, exceptions are *not* chained by default. You have to call explicitly the new private _PyErr_ChainExceptions() function introduced in Python 3.4. It is not trivial to use it: you have to call PyErr_Fetch() and check if an exception was already raised. In Python, the following examples are bad practice because they may hide important exceptions like MemoryError or KeyboardInterrupt: try: .... except: pass or try: .... except: raise ValueError(...) In C extensions, it's common to write such code, few functions check which exception was raised. Last months, I enhanced Python to detect exceptions ignored by mistake: I added assert(!PyErr_Occurred()) in many functions which can clear the current exception (ex: call PyErr_Clear()) or raise a new exception (ex: call PyErr_SetString(...)). The last step is the issue #23571 which now implements in release mode. For the next step, I propose to explicitly clear the current exception before raising a new exception. I don't know yet if it would be a good idea to modify PyErr_*() functions to automatically chain exceptions. | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | msg239131 - (view) | Author: STINNER Victor (vstinner) * | Date: 2015-03-24 14:44 | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | pyerr_assert.patch: modify PyErr_SetObject() and PyErr_Format() to fail with an assertion error if these functions are called with an exception set. This patch detects where an exception is raised while another exception was already raised (like "try: ... except: raise ..."). I'm using this patch to explicitly not chain exceptions where chaining exceptions would be pointless (ex: replace a TypeError with a new TypeError with a better error message). | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | msg239132 - (view) | Author: STINNER Victor (vstinner) * | Date: 2015-03-24 14:46 | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | > For the next step, I propose to explicitly clear the current exception before raising a new exception. Attached pyerr_match_clear.patch implements this. It's only a work-in-progress. I prefer to get feedback on the patch before finishing it. The patch checks also which exception was raised using PyErr_ExceptionMatches() to avoid hiding import exceptions. Since my patch makes assumption on which exception is expected, it can change the behaviour of functions if I forgot a different exception which is also supposed to be replaced. Example: only catch ValueError and replace it with ValueError, whereas OverflowError must also be replaced with ValueError. | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | msg239133 - (view) | Author: STINNER Victor (vstinner) * | Date: 2015-03-24 14:48 | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | pyerr_match_assertion.patch: Modify PyErr_ExceptionMatches() to raise an exception if it is called with no exception set. This patch can be used to ensure that pyerr_match_clear.patch doesn't introduce regression. Example: - PyErr_Format(PyExc_TypeError, - "expected %s instance instead of %s", - ((PyTypeObject *)type)->tp_name, - Py_TYPE(value)->tp_name); + + if (PyErr_ExceptionMatches(PyExc_AttributeError)) { + PyErr_Clear(); + PyErr_Format(PyExc_TypeError, + "expected %s instance instead of %s", + ((PyTypeObject *)type)->tp_name, + Py_TYPE(value)->tp_name); + } This change is wrong is not exception is set, because PyErr_ExceptionMatches() returns 0 if no exception was raised. | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | msg239134 - (view) | Author: STINNER Victor (vstinner) * | Date: 2015-03-24 14:51 | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | See also issue #21715: "Chaining exceptions at C level". The changeset 9af21752ea2a added the new _PyErr_ChainExceptions() function. | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | msg239137 - (view) | Author: STINNER Victor (vstinner) * | Date: 2015-03-24 15:04 | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While working on the PEP 475, I modified _Py_fopen_obj() to raise the OSError (instead of raising the exception from the call site). The zipimport uses _Py_fopen_obj() but it didn't raise OSError, only ZipImportError. I modified the zipimport module to raise a chained exception: OSError chained to ZipImportError: see issue #23696. Other functions using _PyErr_ChainExceptions(): - open() (io.open): if open() failed and f.close() raised an exception, chain the two exceptions - io.FileIO.close - io.TextIOWrapper.close - io.BufferedReader.close, io.BufferedWriter.close - _Py_CheckFunctionResult(), new function introduced in the issue #23571 | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | msg239200 - (view) | Author: STINNER Victor (vstinner) * | Date: 2015-03-25 00:32 | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | pyerr_match_clear-2.patch: Updated patch, more complete (I also removed the assertions, I only added to debug). > Since my patch makes assumption on which exception is expected, it can change the behaviour of functions if I forgot a different exception which is also supposed to be replaced. To reduce risks, we can just remove the new PyErr_ExceptionMatches() checks from the patch in a first time, and add them later, while being very careful. Calling PyErr_Clear() where exceptions must not be chained should be enough for a first step. | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | msg239201 - (view) | Author: STINNER Victor (vstinner) * | Date: 2015-03-25 00:40 | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | New issue #23770: "Rework of exceptions are handled in the parser module (in validate_repeating_list())" to fix error handling in the parser module. | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | msg239203 - (view) | Author: STINNER Victor (vstinner) * | Date: 2015-03-25 00:46 | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Oh, I had a private local commit when I generated pyerr_match_clear-2.patch and so Rietveld failed to find the branch. I regenerated the patch. | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | msg239205 - (view) | Author: STINNER Victor (vstinner) * | Date: 2015-03-25 00:52 | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | pyerr_chain.patch: The real patch to chain automatically exceptions when using PyErr_*() functions. pyerr_match_clear-2.patch hides the original exception when it is useless, too low level like OverflowError when casting to C types, etc. Examples without pyerr_match_clear-2.patch. int, before: | $ python3 -c 'int("123z")' | Traceback (most recent call last): | File "", line 1, in | ValueError: invalid literal for int() with base 10: '123z' int, after: | $ ./python -c 'int("123z")' | ValueError: invalid literal for int() with base 10: '123z' | | During handling of the above exception, another exception occurred: | | Traceback (most recent call last): | File "", line 1, in | ValueError: invalid literal for int() with base 10: '123z' struct, before: | $ python3 -c 'import struct; struct.pack("b", 2**100)' | Traceback (most recent call last): | File "", line 1, in | struct.error: argument out of range struct, after: | $ ./python -c 'import struct; struct.pack("b", 2**100)' | OverflowError: Python int too large to convert to C long | | During handling of the above exception, another exception occurred: | | Traceback (most recent call last): | File "", line 1, in | struct.error: argument out of range More examples, after (still without pyerr_match_clear-2.patch): int(str): | TypeError: a bytes-like object is required, not 'type' | | During handling of the above exception, another exception occurred: | | Traceback (most recent call last): | File "", line 1, in | TypeError: int() argument must be a string, a bytes-like object or a number, not 'type' ''.join(str): | TypeError: 'type' object is not iterable | | During handling of the above exception, another exception occurred: | | Traceback (most recent call last): | File "", line 1, in | TypeError: can only join an iterable b"%f" % "abc": | TypeError: a float is required | | During handling of the above exception, another exception occurred: | | Traceback (most recent call last): | File "", line 1, in | TypeError: float argument required, not str | | msg239207 - (view) | Author: STINNER Victor (vstinner) * | Date: 2015-03-25 00:56 | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Sorry, I had issues with my local repository. Third try to upload pyerr_match_clear-2.patch for Rietveld. | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | msg239227 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2015-03-25 06:49 | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I think it would be safer to defer automatically chaining exceptions to 3.6. After releasing 3.5 we can got reports about exceptions ignored by mistake. After fixing all bugs not covered by the testsuite, we could try automatically chain exceptions. May be this will require longer transient period, 3.6-3.7. | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | msg239233 - (view) | Author: STINNER Victor (vstinner) * | Date: 2015-03-25 07:53 | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Le mercredi 25 mars 2015, Serhiy Storchaka <report@bugs.python.org> a écrit : > > I think it would be safer to defer automatically chaining exceptions to > 3.6. After releasing 3.5 we can got reports about exceptions ignored by > mistake. > Hum, what change can ignore exceptions by mistake? | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | msg297112 - (view) | Author: STINNER Victor (vstinner) * | Date: 2017-06-28 01:23 | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I lost track of this issue and my PEP 490 was more and less rejected, so I just close this issue. I was decided that chaining or not exceptions should be done on a case by case basis. | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | |