From fc7027c76314863bf781e415da5b7c2b62cc75d3 Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Wed, 30 Nov 2022 11:29:56 +0100 Subject: [PATCH 1/4] ENH: Ensure unreported error is chained for import SystemErrors Debugging a small issue in SciPy was made a bit tricky by the error being cleared seemingly unnecessary here. This ensures that module load errors chain their exceptions when this occurs (similar to function calls). Expanded the test to check that the `__cause__` is set where it seems that it should be. --- Lib/test/test_importlib/extension/test_loader.py | 6 +++++- Objects/moduleobject.c | 9 +++++---- Python/importdl.c | 3 +-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_importlib/extension/test_loader.py b/Lib/test/test_importlib/extension/test_loader.py index d69192b56bacb6..218f45ab25f7d8 100644 --- a/Lib/test/test_importlib/extension/test_loader.py +++ b/Lib/test/test_importlib/extension/test_loader.py @@ -351,9 +351,13 @@ def test_bad_modules(self): ]: with self.subTest(name_base): name = self.name + '_' + name_base - with self.assertRaises(SystemError): + with self.assertRaises(SystemError) as cm: self.load_module_by_name(name) + # If there is an unreported exception, it should be chained + if "unreported_exception" in name_base: + self.assertIsNot(cm.exception.__cause__, None) + def test_nonascii(self): # Test that modules with non-ASCII names can be loaded. # punycode behaves slightly differently in some-ASCII and no-ASCII diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 8e03f2446f6fcd..24190e320ee6d6 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -327,9 +327,10 @@ PyModule_FromDefAndSpec2(PyModuleDef* def, PyObject *spec, int module_api_versio goto error; } else { if (PyErr_Occurred()) { - PyErr_Format(PyExc_SystemError, - "creation of module %s raised unreported exception", - name); + _PyErr_FormatFromCause( + PyExc_SystemError, + "creation of module %s raised unreported exception", + name); goto error; } } @@ -431,7 +432,7 @@ PyModule_ExecDef(PyObject *module, PyModuleDef *def) return -1; } if (PyErr_Occurred()) { - PyErr_Format( + _PyErr_FormatFromCause( PyExc_SystemError, "execution of module %s raised unreported exception", name); diff --git a/Python/importdl.c b/Python/importdl.c index 40227674ca47ee..91fa06f49c2897 100644 --- a/Python/importdl.c +++ b/Python/importdl.c @@ -180,8 +180,7 @@ _PyImport_LoadDynamicModuleWithSpec(PyObject *spec, FILE *fp) } goto error; } else if (PyErr_Occurred()) { - PyErr_Clear(); - PyErr_Format( + _PyErr_FormatFromCause( PyExc_SystemError, "initialization of %s raised unreported exception", name_buf); From 55c78d89df24107369100a8e7cc65f6c0aef3fb3 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Fri, 2 Dec 2022 09:31:21 +0000 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/C API/2022-12-02-09-31-19.gh-issue-99947.Ski7OC.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/C API/2022-12-02-09-31-19.gh-issue-99947.Ski7OC.rst diff --git a/Misc/NEWS.d/next/C API/2022-12-02-09-31-19.gh-issue-99947.Ski7OC.rst b/Misc/NEWS.d/next/C API/2022-12-02-09-31-19.gh-issue-99947.Ski7OC.rst new file mode 100644 index 00000000000000..829243cfbac52b --- /dev/null +++ b/Misc/NEWS.d/next/C API/2022-12-02-09-31-19.gh-issue-99947.Ski7OC.rst @@ -0,0 +1 @@ +SystemErrors on import now chain the original unexpected exception From 3308e15d69ea74e10ce02fa4b7fe9cfcab982848 Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Sat, 3 Dec 2022 08:47:27 +0100 Subject: [PATCH 3/4] MAINT: Small code, comment, and blurb fixups based on review Co-authored-by: Brett Cannon --- Lib/test/test_importlib/extension/test_loader.py | 3 ++- .../next/C API/2022-12-02-09-31-19.gh-issue-99947.Ski7OC.rst | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_importlib/extension/test_loader.py b/Lib/test/test_importlib/extension/test_loader.py index 218f45ab25f7d8..b98c4e851e459f 100644 --- a/Lib/test/test_importlib/extension/test_loader.py +++ b/Lib/test/test_importlib/extension/test_loader.py @@ -355,8 +355,9 @@ def test_bad_modules(self): self.load_module_by_name(name) # If there is an unreported exception, it should be chained + # with the `SystemError` if "unreported_exception" in name_base: - self.assertIsNot(cm.exception.__cause__, None) + self.assertIsNotNone(cm.exception.__cause__) def test_nonascii(self): # Test that modules with non-ASCII names can be loaded. diff --git a/Misc/NEWS.d/next/C API/2022-12-02-09-31-19.gh-issue-99947.Ski7OC.rst b/Misc/NEWS.d/next/C API/2022-12-02-09-31-19.gh-issue-99947.Ski7OC.rst index 829243cfbac52b..fbed192d317b34 100644 --- a/Misc/NEWS.d/next/C API/2022-12-02-09-31-19.gh-issue-99947.Ski7OC.rst +++ b/Misc/NEWS.d/next/C API/2022-12-02-09-31-19.gh-issue-99947.Ski7OC.rst @@ -1 +1 @@ -SystemErrors on import now chain the original unexpected exception +Raising SystemError on import will now have its cause be set to the original unexpected exception. From 86f1fa5f8fa4ee8bf9d49692f3481f1abccea6ec Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 23 Dec 2022 15:18:13 -0800 Subject: [PATCH 4/4] Update comment in Lib/test/test_importlib/extension/test_loader.py --- Lib/test/test_importlib/extension/test_loader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_importlib/extension/test_loader.py b/Lib/test/test_importlib/extension/test_loader.py index b98c4e851e459f..3bf2bbdcdcc4e6 100644 --- a/Lib/test/test_importlib/extension/test_loader.py +++ b/Lib/test/test_importlib/extension/test_loader.py @@ -355,7 +355,7 @@ def test_bad_modules(self): self.load_module_by_name(name) # If there is an unreported exception, it should be chained - # with the `SystemError` + # with the `SystemError`. if "unreported_exception" in name_base: self.assertIsNotNone(cm.exception.__cause__)