gh-108082: Add PyErr_FormatUnraisable() function #111086

serhiy-storchaka · 2023-10-19T19:41:42Z

Issue: [C API] Enhance PyErr_WriteUnraisable() API to pass an error message #108082

📚 Documentation preview 📚: https://cpython-previews--111086.org.readthedocs.build/

vstinner

Variadic argument ... cannot be used in some programming languages like Go, see: capi-workgroup/problems#35 Would it be possible to add also a variant which takes an already formatted string?

Something like: PyErr_WriteUnraisableMsg(PyObject *msg) (I'm not sure about the name, nor the API).

vstinner · 2023-10-20T22:43:31Z

Doc/c-api/exceptions.rst

+   Similar to :c:func:`PyErr_WriteUnraisable`, but the *format* and subsequent
+   parameters help format the warning message; they have the same meaning and
+   values as in :c:func:`PyUnicode_FromFormat`.
+   If *format* is ``NULL``, only the traceback is printed.


Would you mind to give examples? I'm not sure how to use this API.

I understood that PyErr_WriteUnraisable(obj) can be written as:

PyErr_FormatUnraisable("Exception ignored in: %R", obj);

And without ~~an exception~~ an object, it should be:

PyErr_FormatUnraisable("Exception ignored in:");

Do the format must end with a newline character?

If format is NULL, only the traceback is printed.

What's the usage of this mode? Is the caller expected to log something like "Exception ignored in xxx" manually?

PyErr_FormatUnraisable(NULL) is the same as PyErr_WriteUnraisable(NULL). It just prints the traceback, without "Exception ignored ...".

I simply documented the existing behavior for PyErr_WriteUnraisable(), and for PyErr_FormatUnraisable(NULL) the most natural way is to do the same. What alternative do you propose?

Sorry, my remark is not specific to NULL value, but more about the whole function. I don't know which string to pass to the function. I suggest to add an example to the doc. Like:

For example, ``PyErr_WriteUnraisable(obj)`` is similar to ``PyErr_FormatUnraisable("Exception ignored in: %R", obj);``.

vstinner

Would it be possible to not add this function to the limited C API in Python 3.13, but wait for Python 3.14?

serhiy-storchaka · 2023-10-29T08:51:00Z

Would it be possible to add also a variant which takes an already formatted string?

What if change PyErr_WriteUnraisable() so that if the argument is string, it prints it as is, instead of of adding "Exception ignored in:" and printing the repr?

serhiy-storchaka · 2023-10-29T08:41:02Z

Doc/c-api/exceptions.rst

+   Similar to :c:func:`PyErr_WriteUnraisable`, but the *format* and subsequent
+   parameters help format the warning message; they have the same meaning and
+   values as in :c:func:`PyUnicode_FromFormat`.
+   If *format* is ``NULL``, only the traceback is printed.


PyErr_FormatUnraisable(NULL) is the same as PyErr_WriteUnraisable(NULL). It just prints the traceback, without "Exception ignored ...".

I simply documented the existing behavior for PyErr_WriteUnraisable(), and for PyErr_FormatUnraisable(NULL) the most natural way is to do the same. What alternative do you propose?

serhiy-storchaka · 2023-10-29T08:44:38Z

Lib/test/test_capi/test_exceptions.py

@@ -270,6 +274,104 @@ def test_setfromerrnowithfilename(self):
                         (ENOENT, 'No such file or directory', 'file'))
        # CRASHES setfromerrnowithfilename(ENOENT, NULL, b'error')

+    def test_err_writeunraisable(self):


TODO: This test should be backported.

See #111455.

vstinner

Overall, the change LGTM. I just suggest to add a small example in the doc. I also propose to change how errors are handled, but that can be done later, and I can propose a followu-p PR for that if you want.

vstinner · 2023-10-31T15:41:55Z

Doc/c-api/exceptions.rst

+   Similar to :c:func:`PyErr_WriteUnraisable`, but the *format* and subsequent
+   parameters help format the warning message; they have the same meaning and
+   values as in :c:func:`PyUnicode_FromFormat`.
+   If *format* is ``NULL``, only the traceback is printed.


Sorry, my remark is not specific to NULL value, but more about the whole function. I don't know which string to pass to the function. I suggest to add an example to the doc. Like:

For example, ``PyErr_WriteUnraisable(obj)`` is similar to ``PyErr_FormatUnraisable("Exception ignored in: %R", obj);``.

vstinner · 2023-10-31T15:51:01Z

Lib/test/test_capi/test_exceptions.py

+            self.assertEqual(str(cm.unraisable.exc_value), 'oops!')
+            self.assertEqual(cm.unraisable.exc_traceback.tb_lineno,
+                             firstline + 15)
+            self.assertIsNone(cm.unraisable.err_msg)


I would prefer to log an error in the error handler in this case. Ignoring silently errors doesn't help :-(

_PyErr_WriteUnraisableMsg() does the same: silently ignores decoding and memory errors.

I minimized changes. Other changes can be made in a separate PR.

vstinner · 2023-10-31T15:52:21Z

Python/errors.c

-_PyErr_WriteUnraisableMsg(const char *err_msg_str, PyObject *obj)
+
+static void
+format_unraisable_v(PyObject *obj, const char *format, va_list va)


Would it be possible to move obj to end, last parameter?

Sure. But I would make the order of arguments inconsistent in format_unraisable() and format_unraisable_v().

vstinner · 2023-10-31T15:52:58Z

Python/errors.c

-    if (err_msg_str != NULL) {
-        err_msg = PyUnicode_FromFormat("Exception ignored %s", err_msg_str);
+    if (format != NULL) {
+        err_msg = PyUnicode_FromFormatV(format, va);
        if (err_msg == NULL) {
            PyErr_Clear();


I don't think that it's the best behavior. I would prefer to goto error. What do you think?

Maybe PyUnicode_FromString("Exception ignored in sys.unraisablehook") can become a _Py_DECLARE_STR(exc_ignored_msg, "...") + _Py_STR(exc_ignored_msg) static immortal string to avoid error 3 while handling error 2 while handling error 1.

It is the same as in _PyErr_WriteUnraisableMsg(). Changing the behavior of _PyErr_WriteUnraisableMsg() is out of the scope of this PR.

vstinner · 2023-10-31T16:00:19Z

The main limitation of the new function is that it doesn't allow to pass an object. But I'm not sure that it's really useful. Maybe we should just deprecated this attribute of sys.unraisablehook. I don't know. I mostly added it to delegate the string formatting to the hook.

serhiy-storchaka · 2023-10-31T21:02:37Z

The main limitation of the new function is that it doesn't allow to pass an object.

Only 6 of about 70 use cases in CPython code actually pass an object. If it is needed, we can add yet one function, but the function which do not require to pass mandatory NULL argument in 90% of cases is more convenient.

vstinner · 2023-10-31T21:29:52Z

Thanks for the short example in the doc ;-)

pythongh-108082: Add PyErr_FormatUnraisable() function

de02658

serhiy-storchaka added the topic-C-API label Oct 19, 2023

serhiy-storchaka requested a review from vstinner October 19, 2023 19:41

serhiy-storchaka requested review from a team and encukou as code owners October 19, 2023 19:41

bedevere-app bot mentioned this pull request Oct 19, 2023

[C API] Enhance PyErr_WriteUnraisable() API to pass an error message #108082

Closed

bedevere-app bot added the awaiting core review label Oct 19, 2023

vstinner reviewed Oct 20, 2023

View reviewed changes

Merge branch 'main' into capi-PyErr_FormatUnraisable

20a11b3

serhiy-storchaka commented Oct 29, 2023

View reviewed changes

serhiy-storchaka added 2 commits October 29, 2023 11:33

Remove from the limited C API.

bceb8c5

Merge branch 'main' into capi-PyErr_FormatUnraisable

bda2510

vstinner approved these changes Oct 31, 2023

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting core review labels Oct 31, 2023

Merge branch 'main' into capi-PyErr_FormatUnraisable

38b5ba2

Address review comments.

b7d68e7

serhiy-storchaka merged commit f6a0232 into python:main Oct 31, 2023
25 checks passed

bedevere-app bot removed the awaiting merge label Oct 31, 2023

serhiy-storchaka deleted the capi-PyErr_FormatUnraisable branch October 31, 2023 21:42

FullteaR pushed a commit to FullteaR/cpython that referenced this pull request Nov 3, 2023

pythongh-108082: Add PyErr_FormatUnraisable() function (pythonGH-111086)

33462e8

aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024

pythongh-108082: Add PyErr_FormatUnraisable() function (pythonGH-111086)

7af0c34

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024

pythongh-108082: Add PyErr_FormatUnraisable() function (pythonGH-111086)

7a60adc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-108082: Add PyErr_FormatUnraisable() function #111086

gh-108082: Add PyErr_FormatUnraisable() function #111086

serhiy-storchaka commented Oct 19, 2023 •

edited by github-actions bot

Loading

vstinner left a comment

vstinner Oct 20, 2023 •

edited

Loading

serhiy-storchaka Oct 29, 2023

vstinner Oct 31, 2023

vstinner left a comment

serhiy-storchaka commented Oct 29, 2023

serhiy-storchaka Oct 29, 2023

serhiy-storchaka Oct 29, 2023

serhiy-storchaka Oct 29, 2023

vstinner left a comment

vstinner Oct 31, 2023

vstinner Oct 31, 2023

serhiy-storchaka Oct 31, 2023

vstinner Oct 31, 2023

serhiy-storchaka Oct 31, 2023

vstinner Oct 31, 2023

serhiy-storchaka Oct 31, 2023

vstinner commented Oct 31, 2023

serhiy-storchaka commented Oct 31, 2023

vstinner commented Oct 31, 2023

gh-108082: Add PyErr_FormatUnraisable() function #111086

gh-108082: Add PyErr_FormatUnraisable() function #111086

Conversation

serhiy-storchaka commented Oct 19, 2023 • edited by github-actions bot Loading

vstinner left a comment

Choose a reason for hiding this comment

vstinner Oct 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vstinner left a comment

Choose a reason for hiding this comment

serhiy-storchaka commented Oct 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vstinner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vstinner commented Oct 31, 2023

serhiy-storchaka commented Oct 31, 2023

vstinner commented Oct 31, 2023

serhiy-storchaka commented Oct 19, 2023 •

edited by github-actions bot

Loading

vstinner Oct 20, 2023 •

edited

Loading