gh-152912: Fix audit hook exception check in sys.addaudithook()

[k00shi]

Python/sysmodule.c:530 used PyExc_Exception to check exceptions
raised by existing audit hooks, catching every Exception subclass.
PySys_AddAuditHook (line 486) and Doc/library/sys.rst both specify
RuntimeError only. Change to PyExc_RuntimeError.

[python-cla-bot]

All commit authors signed the Contributor License Agreement.

[Aniketsy]

please avoid force-push, also for the lint error you can run pre-commit run before pushing.

[StanFromIreland]

It has been like this since the initial implementation in 2019, and isn't specified in PEP 578. While the sys.addaudithook documentation specifies RuntimeError, the equivalent C-API (PySys_AddAuditHook) correctly specifies Exception. I think it we should update the documentation here.

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[k00shi]

Thanks for the review, @StanFromIreland. I'd like to push back on the requested change. The asymmetry looks like a regression from 3.8.1, not original intent. The C-API code does specify RuntimeError.

1. The C-API code uses PyExc_RuntimeError, not PyExc_Exception.

Python/sysmodule.c:486 (in PySys_AddAuditHook):

if (_PyErr_ExceptionMatches(tstate, PyExc_RuntimeError)) {
    /* We do not report errors derived from RuntimeError */
    _PyErr_Clear(tstate);
    return 0;
}

This has been the case since the 3.8.1 cycle. The C-API code and the Python-API documentation (Doc/library/sys.rst:96-101 plus the .. versionchanged:: 3.8.1 note at :108-111) agree on RuntimeError only.

2. The asymmetry came from a partial fix in 3.8.1, not from the original PEP 578 landing.

The original PEP 578 commit (b82e17e6, 2019-05-23, Steve Dower) introduced both checks symmetrically as PyExc_Exception.

The 3.8.1 narrowing commit (bea33f5e1, 2019-11-28, same author, bpo-38920 / GH-17392) narrowed only the C-API code at line 486, and in the same commit it updated Doc/library/sys.rst to say RuntimeError and added the versionchanged:: 3.8.1 note. The intent was to narrow to RuntimeError everywhere. sys_addaudithook_impl at line 530, further down in the same file, was missed.

You can verify with git show bea33f5e1 -- Python/sysmodule.c Doc/library/sys.rst. The commit touches line 486 and the Python-API doc, but not line 530.

3. The one place that still says Exception is Doc/c-api/sys.rst.

Doc/c-api/sys.rst:436 and :450 (C-API doc) still say "Exception" when describing PySys_AddAuditHook's suppression behavior. This is a stale doc. bea33f5e1 updated Doc/library/sys.rst but missed Doc/c-api/sys.rst. That is what led to the impression that "the C-API specifies Exception".

Since the C-API code has used RuntimeError since 3.8.1, I'd like to keep my code change as is. To close the loop on the doc inconsistency, I can also fix Doc/c-api/sys.rst in this PR. Would you prefer that here, or as a separate follow-up?

On PEP 578: agreed it is silent on the exception type. The implementation and docs have converged on RuntimeError since 3.8.1, and this PR restores that contract.

[StanFromIreland]

Oh alright, I thought it was just one path. Indeed, the C-API documentation is wrong, and that should be fixed (we can do it in this PR). However, I'm still not sure about this change, while the current logic contradicts the documentation, I worry changing it now would cause too much breakage. I presume that realistically, people are relying on the current behaviour.