◐ Shell
reader mode source ↗
Skip to content

gh-108082: Add PyErr_FormatUnraisable() function#111086

Merged
serhiy-storchaka merged 6 commits into
python:mainfrom
serhiy-storchaka:capi-PyErr_FormatUnraisable
Oct 31, 2023
Merged

gh-108082: Add PyErr_FormatUnraisable() function#111086
serhiy-storchaka merged 6 commits into
python:mainfrom
serhiy-storchaka:capi-PyErr_FormatUnraisable

Conversation

@serhiy-storchaka

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

Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka requested review from a team and encukou as code owners October 19, 2023 19:41
@bedevere-app bedevere-app Bot mentioned this pull request Oct 19, 2023
Closed
vstinner
vstinner reviewed Oct 20, 2023
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

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
vstinner reviewed Oct 20, 2023
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

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

serhiy-storchaka commented Oct 29, 2023

Copy link
Copy Markdown
Member Author

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?

vstinner
vstinner approved these changes Oct 31, 2023
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

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

vstinner commented Oct 31, 2023

Copy link
Copy Markdown
Member

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

serhiy-storchaka commented Oct 31, 2023

Copy link
Copy Markdown
Member Author

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

vstinner commented Oct 31, 2023

Copy link
Copy Markdown
Member

Thanks for the short example in the doc ;-)

@serhiy-storchaka serhiy-storchaka merged commit f6a0232 into python:main Oct 31, 2023
@serhiy-storchaka 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
@serhiy-storchaka @FullteaR
pythongh-108082: Add PyErr_FormatUnraisable() function (pythonGH-111086)
33462e8
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@serhiy-storchaka @aisk
pythongh-108082: Add PyErr_FormatUnraisable() function (pythonGH-111086)
7af0c34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes

@encukou encukou Awaiting requested review from encukou

Assignees

No one assigned

Labels

topic-C-API

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@serhiy-storchaka @vstinner