classification
Title: Provide a C helper function to chain raised (but not yet caught) exceptions
Type: enhancement Stage: patch review
Components: Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: chris.jerdonek, eric.snow, ncoghlan, pitrou, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2015-01-08 03:17 by ncoghlan, last changed 2020-05-23 04:08 by chris.jerdonek.

Pull Requests
URL Status Linked Edit
PR 20329 open chris.jerdonek, 2020-05-23 04:06
Messages (17)
msg233619 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-01-08 03:17
There's currently an internal helper API for chaining exceptions from C code, _PyErr_ChainExceptions.

It would be good to have a public API that offered equivalent functionality for use in third party C extensions.
msg233625 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-01-08 08:18
After looking into this further, PyErr_SetObject (and other APIs like PyErr_SetString which call that internally) aim to handle the chaining automatically, but they don't handle exceptions which haven't been normalized yet.

PyErr_SetObject should probably normalise the exception at the start of the call f ithe exception type is set on the thread state, but not the exception value.
msg233626 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-01-08 08:29
The documentation of PyErr_SetObject, PyErr_SetString et al should also be updated to mention exception chaining.
msg233628 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-01-08 08:40
Thinking about it a bit further, I suspect implicit normalisation of chained exceptions could cause problems when we only want to set __cause__ without setting __context__ (e.g. codec exception chaining).

I'm going to ponder this one for a while, but happy to hear anyone else's feedback.

At the very least, I think the C API docs could use a bit more clarity on the intricacies of C level exception chaining.
msg233657 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-01-08 13:55
_PyErr_ChainExceptions() was added because exceptions raised in C code are not implicitly chained. The code for explicit chaining is error prone, so it was extracted in separate function. Even with _PyErr_ChainExceptions() chaining exceptions look complex. May be implicit chaining all exceptions would be better.
msg233733 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-01-09 08:42
The interesting discovery I made while reviewing the patch for issue 22906 is that there apparently *is* implicit chaining support in PyErr_SetObject: https://hg.python.org/cpython/file/default/Python/errors.c#l70

Chris indicates that it doesn't seem to be triggering for his patch, though (even after normalising and then restoring the exception state), and I haven't fully dug into why yet. Preliminary explorations show that the last two functional modifications were a fix for a crash bug in issue 3611, and Antoine's original implementation of exception chaining in issue 3108.

I've added Antoine to the nosy list, as my main takeaway at the moment is that I *don't* currently understand what's going on with the exception chaining, and I'd like to before we merge the PEP 479 patch.
msg233789 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015-01-10 00:07
> The interesting discovery I made while reviewing the patch for issue 
> 22906 is that there apparently *is* implicit chaining support in
> PyErr_SetObject

Indeed, there is, and it should work properly (AFAIR there was quite a bit of debugging to make this work). Also, note that normalizing is already handled.

I'm not sure what you're witnessing that doesn't work as expected. I suspect that you may be confusing "an exception has been caught" (and is ready to be chained from) with "an exception has been raised" (i.e. PyErr_Occurred() is true).
msg233817 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-01-10 11:13
Ah, confusion between "exception has been raised" and "we're in an active exception handler" is almost certainly what is happening. In both issue 22906 and my previous codec exception wrapping work, we're still in the C code that raised the exception, *before* we make it back to the eval loop and any Python level exception handling.

So I think that's the distinction we need to ensure is documented, and potentially a new public helper function provided - if you're in a Python level exception handler, then exception context chaining will be handled automatically for you in PyErr_SetObject, but if you're still in C code and haven't made it back to the eval loop after raising an exception, then you're going to have to do any exception chaining explicitly.
msg233818 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-01-10 11:14
Updated the issue title and type again based on Antoine's explanation.
msg278880 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-10-18 13:56
Via issue 28410, Serhiy is adding a private "_PyErr_FormatFromCause" helper API to make explicit C level exception chaining easier within the implementation of CPython (this helps resolve issue 28214, an exception reporting problem in the new PEP 487 __set_name__ feature).

It may be sufficient to make that API public, and use that as the home of documentation for some of the subtleties discussed here.
msg278967 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-19 08:23
This helper is convenient in many cases, but it is very limited. It raises an exception with single string argument. It doesn't work in cases when the exception doesn't take arguments (PyErr_SetNone) or takes multiple or non-string arguments (PyErr_SetFromErrnoWithFilenameObject, PyErr_SetImportError). I think for public API we need more general solution. Something like this:

    PyObject *cause = get_current_exception();
    PyErr_SetImportError(msg, name, path);
    set_cause_of_current_exception(cause);
msg317307 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-05-22 16:24
FTR, see PEP 490 ("Chain exceptions at C level") which proposed implicitly chaining exceptions in the PyErr_* API.

While that PEP was rejected (not all exceptions should be chained), it does make a good point about the clunkiness of using _PyErr_ChainExceptions():

    PyObject *exc, *val, *tb;
    PyErr_Fetch(&exc, &val, &tb);
    PyErr_Format(ZipImportError, "can't open Zip file: %R", archive);
    _PyErr_ChainExceptions(exc, val, tb);

So if we are going to add a public helper function, let's consider adding one that simplifies usage.  For instance, how about one that indicates the next exception set should chain:

    PyErr_ChainNext();
    PyErr_Format(ZipImportError, "can't open Zip file: %R", archive);

Or perhaps we should revive PEP 490 with an opt out mechanism for the cases where we don't want chaining:

    PyErr_NoChainNext();
    PyErr_Format(PyExc_RuntimeError, "uh-oh");
msg317314 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-22 17:44
There is usually more complex code between PyErr_Fetch() and _PyErr_ChainExceptions():

        PyObject *exc, *val, *tb, *close_result;
        PyErr_Fetch(&exc, &val, &tb);
        close_result = _PyObject_CallMethodId(result, &PyId_close, NULL);
        _PyErr_ChainExceptions(exc, val, tb);
        Py_XDECREF(close_result);

Many exceptions can be raised and silenced or overridden between, but we are interesting in chaining the only latest exception (or restoring the original exception if all exceptions between were silenced).
msg317335 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-05-22 19:47
good point :)
msg317402 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-05-23 13:06
Also see https://github.com/python/cpython/blob/55edd0c185ad2d895b5d73e47d67049bc156b654/Objects/exceptions.c#L2713 for the version we use in a few places to implicitly update the exception message, while keeping the exception type and state the same (and giving up and letting the exception through unchained if we can't work out how to do that in a reliable way).
msg369657 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-05-22 22:04
I just want to point out one difference between _PyErr_ChainExceptions and PyErr_SetObject that I encountered while working on this issue:
https://bugs.python.org/issue40696

While both functions set the context, only PyErr_SetObject does a check to prevent cycles from forming in the context chain (albeit an incomplete check, which can lead to hangs, which I mention in the issue linked above).
msg369691 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-05-23 04:08
> The documentation of PyErr_SetObject, PyErr_SetString et al should also be updated to mention exception chaining.

I just posted a PR to do the above:
https://github.com/python/cpython/pull/20329
History
Date User Action Args
2020-05-23 04:08:02chris.jerdoneksetmessages: + msg369691
2020-05-23 04:06:44chris.jerdoneksetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request19597
2020-05-22 22:04:09chris.jerdoneksetmessages: + msg369657
2020-05-21 14:51:47chris.jerdoneksetnosy: + chris.jerdonek
2018-05-23 13:06:40ncoghlansetmessages: + msg317402
2018-05-22 19:47:22eric.snowsetmessages: + msg317335
2018-05-22 17:44:16serhiy.storchakasetmessages: + msg317314
2018-05-22 16:24:32eric.snowsetnosy: + vstinner, eric.snow

messages: + msg317307
versions: + Python 3.8, - Python 3.7
2016-10-19 08:23:27serhiy.storchakasetmessages: + msg278967
2016-10-18 13:56:57ncoghlansetmessages: + msg278880
versions: + Python 3.7, - Python 3.5
2015-06-28 03:02:54ncoghlansetassignee: ncoghlan ->
2015-01-10 11:14:36ncoghlansettype: behavior -> enhancement
messages: + msg233818
title: Exception chaining should trigger for non-normalised exceptions -> Provide a C helper function to chain raised (but not yet caught) exceptions
2015-01-10 11:13:07ncoghlansetmessages: + msg233817
2015-01-10 00:07:34pitrousetmessages: + msg233789
2015-01-09 08:42:58ncoghlansetnosy: + pitrou
messages: + msg233733
2015-01-08 13:55:23serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg233657
2015-01-08 08:40:54ncoghlansetassignee: ncoghlan
messages: + msg233628
2015-01-08 08:29:56ncoghlansetmessages: + msg233626
2015-01-08 08:18:39ncoghlansettype: enhancement -> behavior
messages: + msg233625
title: Convert _PyErr_ChainExceptions to a public API -> Exception chaining should trigger for non-normalised exceptions
2015-01-08 03:17:21ncoghlancreate