From 8311931ba08352fcdf45dff7270d7531c942f5cf Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Sat, 9 Sep 2023 21:41:06 +0100 Subject: [PATCH 1/5] gh-109179: Fix traceback display for SyntaxErrors with notes --- Lib/test/test_traceback.py | 42 +++++++++++++++++++------------------- Python/pythonrun.c | 24 ++++++++++------------ 2 files changed, 32 insertions(+), 34 deletions(-) diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py index be81082bb19472..baddebee1818c3 100644 --- a/Lib/test/test_traceback.py +++ b/Lib/test/test_traceback.py @@ -1599,27 +1599,27 @@ def __repr__(self): err_msg = "b'please do not show me as numbers'" self.assertEqual(self.get_report(e), vanilla + err_msg + '\n') - def test_exception_with_note_with_multiple_notes(self): - e = ValueError(42) - vanilla = self.get_report(e) - - e.add_note('Note 1') - e.add_note('Note 2') - e.add_note('Note 3') - - self.assertEqual( - self.get_report(e), - vanilla + 'Note 1\n' + 'Note 2\n' + 'Note 3\n') - - del e.__notes__ - e.add_note('Note 4') - del e.__notes__ - e.add_note('Note 5') - e.add_note('Note 6') - - self.assertEqual( - self.get_report(e), - vanilla + 'Note 5\n' + 'Note 6\n') + def test_exception_with_multiple_notes(self): + for e in [ValueError(42), SyntaxError('bad syntax')]: + vanilla = self.get_report(e) + + e.add_note('Note 1') + e.add_note('Note 2') + e.add_note('Note 3') + + self.assertEqual( + self.get_report(e), + vanilla + 'Note 1\n' + 'Note 2\n' + 'Note 3\n') + + del e.__notes__ + e.add_note('Note 4') + del e.__notes__ + e.add_note('Note 5') + e.add_note('Note 6') + + self.assertEqual( + self.get_report(e), + vanilla + 'Note 5\n' + 'Note 6\n') def test_exception_qualname(self): class A: diff --git a/Python/pythonrun.c b/Python/pythonrun.c index ddd8951cedb1f1..e3d03a8c95d767 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -1125,21 +1125,16 @@ print_exception_suggestions(struct exception_print_context *ctx, } static int -print_exception_notes(struct exception_print_context *ctx, PyObject *value) +print_exception_notes(struct exception_print_context *ctx, PyObject *notes) { PyObject *f = ctx->file; - if (!PyExceptionInstance_Check(value)) { + if (notes == NULL) { return 0; } - PyObject *notes; - int res = PyObject_GetOptionalAttr(value, &_Py_ID(__notes__), ¬es); - if (res <= 0) { - return res; - } if (!PySequence_Check(notes) || PyUnicode_Check(notes) || PyBytes_Check(notes)) { - res = 0; + int res = 0; if (write_indented_margin(ctx, f) < 0) { res = -1; } @@ -1152,7 +1147,6 @@ print_exception_notes(struct exception_print_context *ctx, PyObject *value) res = PyFile_WriteObject(s, f, Py_PRINT_RAW); Py_DECREF(s); } - Py_DECREF(notes); if (PyFile_WriteString("\n", f) < 0) { res = -1; } @@ -1197,17 +1191,16 @@ print_exception_notes(struct exception_print_context *ctx, PyObject *value) } } - Py_DECREF(notes); return 0; error: Py_XDECREF(lines); - Py_DECREF(notes); return -1; } static int print_exception(struct exception_print_context *ctx, PyObject *value) { + PyObject *notes = NULL; PyObject *f = ctx->file; if (!PyExceptionInstance_Check(value)) { @@ -1221,8 +1214,11 @@ print_exception(struct exception_print_context *ctx, PyObject *value) goto error; } - /* grab the type now because value can change below */ + /* grab the type and notes now because value can change below */ PyObject *type = (PyObject *) Py_TYPE(value); + if (PyObject_GetOptionalAttr(value, &_Py_ID(__notes__), ¬es) < 0) { + goto error; + } if (print_exception_file_and_line(ctx, &value) < 0) { goto error; @@ -1236,14 +1232,16 @@ print_exception(struct exception_print_context *ctx, PyObject *value) if (PyFile_WriteString("\n", f) < 0) { goto error; } - if (print_exception_notes(ctx, value) < 0) { + if (print_exception_notes(ctx, notes) < 0) { goto error; } + Py_XDECREF(notes); Py_DECREF(value); assert(!PyErr_Occurred()); return 0; error: + Py_XDECREF(notes); Py_DECREF(value); return -1; } From 6509e855d703be718dfd596cf0e5c61248c48ca1 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Sat, 9 Sep 2023 22:03:22 +0100 Subject: [PATCH 2/5] use subtest Co-authored-by: Alex Waygood --- Lib/test/test_traceback.py | 39 +++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py index baddebee1818c3..539f31084d7fc2 100644 --- a/Lib/test/test_traceback.py +++ b/Lib/test/test_traceback.py @@ -1601,25 +1601,26 @@ def __repr__(self): def test_exception_with_multiple_notes(self): for e in [ValueError(42), SyntaxError('bad syntax')]: - vanilla = self.get_report(e) - - e.add_note('Note 1') - e.add_note('Note 2') - e.add_note('Note 3') - - self.assertEqual( - self.get_report(e), - vanilla + 'Note 1\n' + 'Note 2\n' + 'Note 3\n') - - del e.__notes__ - e.add_note('Note 4') - del e.__notes__ - e.add_note('Note 5') - e.add_note('Note 6') - - self.assertEqual( - self.get_report(e), - vanilla + 'Note 5\n' + 'Note 6\n') + with self.subTest(e=e): + vanilla = self.get_report(e) + + e.add_note('Note 1') + e.add_note('Note 2') + e.add_note('Note 3') + + self.assertEqual( + self.get_report(e), + vanilla + 'Note 1\n' + 'Note 2\n' + 'Note 3\n') + + del e.__notes__ + e.add_note('Note 4') + del e.__notes__ + e.add_note('Note 5') + e.add_note('Note 6') + + self.assertEqual( + self.get_report(e), + vanilla + 'Note 5\n' + 'Note 6\n') def test_exception_qualname(self): class A: From 8f07eeb5c7b615b00e19951a575dbc301abd5bd9 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Sat, 9 Sep 2023 22:15:16 +0100 Subject: [PATCH 3/5] whitespace --- Lib/test/test_traceback.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py index 539f31084d7fc2..aa8405bd25d120 100644 --- a/Lib/test/test_traceback.py +++ b/Lib/test/test_traceback.py @@ -1603,21 +1603,21 @@ def test_exception_with_multiple_notes(self): for e in [ValueError(42), SyntaxError('bad syntax')]: with self.subTest(e=e): vanilla = self.get_report(e) - + e.add_note('Note 1') e.add_note('Note 2') e.add_note('Note 3') - + self.assertEqual( self.get_report(e), vanilla + 'Note 1\n' + 'Note 2\n' + 'Note 3\n') - + del e.__notes__ e.add_note('Note 4') del e.__notes__ e.add_note('Note 5') e.add_note('Note 6') - + self.assertEqual( self.get_report(e), vanilla + 'Note 5\n' + 'Note 6\n') From b13861766f846e0f65ead08ea86547045dcf28fc Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sat, 9 Sep 2023 21:17:21 +0000 Subject: [PATCH 4/5] =?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 --- .../2023-09-09-21-17-18.gh-issue-109179.ZR8qs2.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-09-09-21-17-18.gh-issue-109179.ZR8qs2.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-09-09-21-17-18.gh-issue-109179.ZR8qs2.rst b/Misc/NEWS.d/next/Core and Builtins/2023-09-09-21-17-18.gh-issue-109179.ZR8qs2.rst new file mode 100644 index 00000000000000..d26240ffb84525 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-09-09-21-17-18.gh-issue-109179.ZR8qs2.rst @@ -0,0 +1 @@ +Fix bug where the C traceback display drops notes from :exc:`SyntaxError`s. From f83ffa8410472d530ab8f698bbdcf03f052ee1f8 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Sat, 9 Sep 2023 22:51:25 +0100 Subject: [PATCH 5/5] doc formatting --- .../2023-09-09-21-17-18.gh-issue-109179.ZR8qs2.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-09-09-21-17-18.gh-issue-109179.ZR8qs2.rst b/Misc/NEWS.d/next/Core and Builtins/2023-09-09-21-17-18.gh-issue-109179.ZR8qs2.rst index d26240ffb84525..dd95a8ec7920aa 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-09-09-21-17-18.gh-issue-109179.ZR8qs2.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-09-09-21-17-18.gh-issue-109179.ZR8qs2.rst @@ -1 +1 @@ -Fix bug where the C traceback display drops notes from :exc:`SyntaxError`s. +Fix bug where the C traceback display drops notes from :exc:`SyntaxError`.