gh-106922: Support multi-line error locations in traceback (attempt 2)#112097

williamwen42 · 2023-11-15T01:34:05Z

Attempt 2 at #109589 - the C changes are no longer required, so only python files and tests are touched. Otherwise, the method to output multi-line errors remains unchanged compared to the original PR.

Issue: Support multi-line error locations in traceback and other related improvements (PEP-657, 3.11) #106922

bedevere-app · 2023-11-15T01:34:12Z

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

williamwen42 · 2023-11-15T01:34:47Z

cc @pablogsal @1st1 @lysnikolaou

pablogsal · 2023-11-17T03:43:39Z

@williamwen42 Seems that there are a bunch of doctests that still fail, can you take a look?

pablogsal

This looks very good. Great work. I made a first pass over the code.

Lib/traceback.py

pablogsal · 2023-11-18T16:28:02Z

Lib/traceback.py

 returnself._line

+@property
+def_dedented_lines(self):


This also strips the line, not only dedents, no?

@williamwen42 ?

Lib/traceback.py

pablogsal · 2023-11-18T16:44:28Z

Lib/traceback.py

+result= []
+
+# only display first line, last line, and lines around anchor start/end
+significant_lines={0, len(all_lines) -1}


I am not very convinced about this significant_lines treatment. I have the following concerns:
It increases the complexity of the traceback
There are plenty of cases where this hides seemingly arbitrary lines. Consider this code:
def foo(a,b,c,d,e,f,g): bar(b) def bar(b): 1/b foo( 1, 0, 3, 4, 5, 6, 7)
Currently this shows:
Traceback (most recent call last): File "/Users/pgalindo3/github/python/main/lel.py", line 7, in <module> foo( ~~~^ 1, ^^ ...<4 lines>... 6, ^^ 7) ^^ File "/Users/pgalindo3/github/python/main/lel.py", line 2, in foo bar(b) ~~~^^^ File "/Users/pgalindo3/github/python/main/lel.py", line 5, in bar 1/b ~^~ ZeroDivisionError: division by zero
but it's not clear to me why we are hiding 4 lines there. Why are the arguments provided there not important? If you look closely what's causing the exception is that b is 0, but that's not being shown in the call to foo, but is the most important part of the call.

That's true, although I agree with the general direction of the code to hide some lines, in case the error spans many lines and the traceback becomes unreadable. Should we maybe think of another heuristic to figure out which lines to show and which not to show?

We will need some kind of heuristic - I encountered a case where a traceback through a with statement results in the entire block being printed because the instruction being traced points to the entire block.
Perhaps we can show all lines only for binary ops/subscripts/function calls?

Do you have an example of the with statement out of curiosity?

Code:
classC: def__enter__(self): raiseRuntimeError("fail") def__exit__(self, *excinfo): passwithC(): # comment# comment# comment# comment# comment# comment# comment# comment# commentpass
Output:
Traceback (most recent call last): File "/data/users/williamwen/cpython/local_test.py", line 8, in <module> with C(): ...<9 lines>... pass File "/data/users/williamwen/cpython/local_test.py", line 3, in __enter__ raise RuntimeError("fail") RuntimeError: fail
So if we don't skip lines, we would print out all those comments.

Hummm, I think this may be a bug in the code locations more than a problem in the implementation. But I am fine going with this version initially and refine it if people find it confusing. @lysnikolaou WDYT?

Agreed, let's go with this and we can refine it in the future if the need arises.

Indeed, looks like all the bytecode instruction for the with include all the comments:
[] Instruction(opname='RESUME', opcode=151, arg=0, argval=0, argrepr='', offset=0, starts_line=0, is_jump_target=False, positions=Positions(lineno=0, end_lineno=1, col_offset=0, end_col_offset=0)) ['class C:', ' def __enter__(self):', ' raise RuntimeError("fail")', '', ' def __exit__(self, *excinfo):'] Instruction(opname='PUSH_NULL', opcode=2, arg=None, argval=None, argrepr='', offset=2, starts_line=1, is_jump_target=False, positions=Positions(lineno=1, end_lineno=6, col_offset=0, end_col_offset=12)) ['class C:', ' def __enter__(self):', ' raise RuntimeError("fail")', '', ' def __exit__(self, *excinfo):'] Instruction(opname='LOAD_BUILD_CLASS', opcode=71, arg=None, argval=None, argrepr='', offset=4, starts_line=None, is_jump_target=False, positions=Positions(lineno=1, end_lineno=6, col_offset=0, end_col_offset=12)) ['class C:', ' def __enter__(self):', ' raise RuntimeError("fail")', '', ' def __exit__(self, *excinfo):'] Instruction(opname='LOAD_CONST', opcode=100, arg=0, argval=<code object C at 0x1049211b0, file "<string>", line 1>, argrepr='<code object C at 0x1049211b0, file "<string>", line 1>', offset=6, starts_line=None, is_jump_target=False, positions=Positions(lineno=1, end_lineno=6, col_offset=0, end_col_offset=12)) ['class C:', ' def __enter__(self):', ' raise RuntimeError("fail")', '', ' def __exit__(self, *excinfo):'] Instruction(opname='MAKE_FUNCTION', opcode=132, arg=0, argval=0, argrepr='', offset=8, starts_line=None, is_jump_target=False, positions=Positions(lineno=1, end_lineno=6, col_offset=0, end_col_offset=12)) ['class C:', ' def __enter__(self):', ' raise RuntimeError("fail")', '', ' def __exit__(self, *excinfo):'] Instruction(opname='LOAD_CONST', opcode=100, arg=1, argval='C', argrepr="'C'", offset=10, starts_line=None, is_jump_target=False, positions=Positions(lineno=1, end_lineno=6, col_offset=0, end_col_offset=12)) ['class C:', ' def __enter__(self):', ' raise RuntimeError("fail")', '', ' def __exit__(self, *excinfo):'] Instruction(opname='PRECALL', opcode=166, arg=2, argval=2, argrepr='', offset=12, starts_line=None, is_jump_target=False, positions=Positions(lineno=1, end_lineno=6, col_offset=0, end_col_offset=12)) ['class C:', ' def __enter__(self):', ' raise RuntimeError("fail")', '', ' def __exit__(self, *excinfo):'] Instruction(opname='CALL', opcode=171, arg=2, argval=2, argrepr='', offset=16, starts_line=None, is_jump_target=False, positions=Positions(lineno=1, end_lineno=6, col_offset=0, end_col_offset=12)) ['class C:', ' def __enter__(self):', ' raise RuntimeError("fail")', '', ' def __exit__(self, *excinfo):'] Instruction(opname='STORE_NAME', opcode=90, arg=0, argval='C', argrepr='C', offset=26, starts_line=None, is_jump_target=False, positions=Positions(lineno=1, end_lineno=6, col_offset=0, end_col_offset=12)) [] Instruction(opname='PUSH_NULL', opcode=2, arg=None, argval=None, argrepr='', offset=28, starts_line=8, is_jump_target=False, positions=Positions(lineno=8, end_lineno=8, col_offset=5, end_col_offset=6)) [] Instruction(opname='LOAD_NAME', opcode=101, arg=0, argval='C', argrepr='C', offset=30, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=8, col_offset=5, end_col_offset=6)) [] Instruction(opname='PRECALL', opcode=166, arg=0, argval=0, argrepr='', offset=32, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=8, col_offset=5, end_col_offset=8)) [] Instruction(opname='CALL', opcode=171, arg=0, argval=0, argrepr='', offset=36, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=8, col_offset=5, end_col_offset=8)) ['with C():', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment'] Instruction(opname='BEFORE_WITH', opcode=53, arg=None, argval=None, argrepr='', offset=46, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=18, col_offset=0, end_col_offset=8)) ['with C():', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment'] Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=48, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=18, col_offset=0, end_col_offset=8)) [] Instruction(opname='NOP', opcode=9, arg=None, argval=None, argrepr='', offset=50, starts_line=18, is_jump_target=False, positions=Positions(lineno=18, end_lineno=18, col_offset=4, end_col_offset=8)) ['with C():', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment'] Instruction(opname='LOAD_CONST', opcode=100, arg=2, argval=None, argrepr='None', offset=52, starts_line=8, is_jump_target=False, positions=Positions(lineno=8, end_lineno=18, col_offset=0, end_col_offset=8)) ['with C():', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment'] Instruction(opname='LOAD_CONST', opcode=100, arg=2, argval=None, argrepr='None', offset=54, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=18, col_offset=0, end_col_offset=8)) ['with C():', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment'] Instruction(opname='LOAD_CONST', opcode=100, arg=2, argval=None, argrepr='None', offset=56, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=18, col_offset=0, end_col_offset=8)) ['with C():', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment'] Instruction(opname='PRECALL', opcode=166, arg=2, argval=2, argrepr='', offset=58, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=18, col_offset=0, end_col_offset=8)) ['with C():', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment'] Instruction(opname='CALL', opcode=171, arg=2, argval=2, argrepr='', offset=62, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=18, col_offset=0, end_col_offset=8)) ['with C():', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment'] Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=72, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=18, col_offset=0, end_col_offset=8)) ['with C():', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment'] Instruction(opname='LOAD_CONST', opcode=100, arg=2, argval=None, argrepr='None', offset=74, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=18, col_offset=0, end_col_offset=8)) ['with C():', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment'] Instruction(opname='RETURN_VALUE', opcode=83, arg=None, argval=None, argrepr='', offset=76, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=18, col_offset=0, end_col_offset=8)) ['with C():', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment'] Instruction(opname='PUSH_EXC_INFO', opcode=35, arg=None, argval=None, argrepr='', offset=78, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=18, col_offset=0, end_col_offset=8)) ['with C():', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment'] Instruction(opname='WITH_EXCEPT_START', opcode=49, arg=None, argval=None, argrepr='', offset=80, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=18, col_offset=0, end_col_offset=8)) ['with C():', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment'] Instruction(opname='POP_JUMP_FORWARD_IF_TRUE', opcode=115, arg=4, argval=92, argrepr='to 92', offset=82, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=18, col_offset=0, end_col_offset=8)) ['with C():', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment', ' # comment'] Instruction(opname='RERAISE', opcode=119, arg=2, argval=2, argrepr='', offset=84, starts_line=None, is_jump_target=False, positions=Positions(lineno=8, end_lineno=18, col_offset=0, end_col_offset=8))
@iritkatriel I think this qualifies as a bug. Fo example the CALL for C() in with C() has end_lineno setted to the last comment in the block.

pablogsal · 2023-11-18T16:48:19Z

Lib/traceback.py

+# Binary ops, subscripts, and calls are expressions, so
+# we can wrap them with parentheses to parse them as
+# (possibly multi-line) expressions.
+tree=ast.parse("(\n"+segment+"\n)")


I am not sure I understand the problem here, can you provide some examples. Also, isn't this something we can solve with mode= "eval"?

The problem is that we might have to parse lines such as
x= ( a+b )
The anchor parsing code will only parse
a+b
which is not a valid expression because of the newline, so not even using eval mode will help.
We need to wrap in parentheses so that the segment is a proper expression.
( a+b )

But isn't this going to alter the line locations for single-line expressions?
Add the example to the comment so is clearer
Let's simplify to:
Suggested change
tree=ast.parse("(\n"+segment+"\n)")
tree=ast.parse(f"(\n{segment}\n)")

But isn't this going to alter the line locations for single-line expressions?
My understanding is that there's some post-processing of the tree, which handles this, which looks okay to me.

Lib/traceback.py

pablogsal · 2023-11-18T16:53:15Z

@lysnikolaou Can you take a look when you have some time? This is a considerable ammount of code and I think it needs extra eyes

lysnikolaou

This look very good in general indeed. Thanks @williamwen42! I left some comments, some require further discussion.

lysnikolaou · 2023-11-20T11:37:45Z

Lib/traceback.py

 returnNone
-self._line=linecache.getline(self.filename, self.lineno)
-returnself._line.strip()
+end_lineno=self.linenoifself.end_linenoisNoneelseself.end_lineno


This is probably not needed, since it's being handled in __init__, and self.end_lineno can be used directly, right?

Lib/traceback.py

lysnikolaou · 2023-11-20T12:15:12Z

Lib/traceback.py

+start_offset-=dedent_characters
+end_offset-=dedent_characters
+start_offset=max(0, start_offset)
+end_offset=max(0, end_offset)


Suggested change
start_offset-=dedent_characters
end_offset-=dedent_characters
start_offset=max(0, start_offset)
end_offset=max(0, end_offset)
start_offset=max(0, start_offset-dedent_characters)
end_offset=max(0, end_offset-dedent_characters)

lysnikolaou · 2023-11-20T12:17:06Z

Lib/traceback.py

+segment=segment[start_offset:len(segment) - (len(all_lines[-1]) -end_offset)]
+
+# attempt to parse for anchors
+anchors: Optional[_Anchors] =None


Let's not put type hints in a completely untyped part of the codebase. Optional is also undefined here.

Lib/test/test_traceback.py

williamwen42 · 2023-11-27T18:51:52Z

@pablogsal @lysnikolaou can I get a re-review?

pablogsal · 2023-11-27T19:36:42Z

Will make another pass this week. Thanks for your patience @williamwen42!

pablogsal

LGTM

This is a fantastic job @williamwen42 thanks a lot for your patience and for following along with us. You rock 🤘

I left a nit comment but after @lysnikolaou takes a look I think we can land it.

A heads up: it's possible that we may require some changes from now to the first beta if some users or core devs find some aspects of this confusing or not very legible, but we can address those once we have more concrete aspects to discuss.

pablogsal · 2023-12-01T13:45:02Z

@lysnikolaou can you take a look when you have some time?

lysnikolaou · 2023-12-01T13:45:26Z

@lysnikolaou can you take a look when you have some time?

On it.

lysnikolaou

Let's merge this! Thanks @williamwen42 for doing this. Great work! 🚀

pablogsal · 2023-12-01T22:18:50Z

Merged! Congratulations @williamwen42! Thanks a lot for your contribution

…empt 2) (python#112097)

…no` changed in 3.13 (#130755) The value taken by `FrameSummary.end_lineno` when passing `end_lineno=None` changed in gh-112097. Previously, a `end_lineno` could be specified to be `None` directly but since 939fc6d, passing None makes the constructor use the value of `lineno` instead.

…d_lineno` changed in 3.13 (pythonGH-130755) The value taken by `FrameSummary.end_lineno` when passing `end_lineno=None` changed in pythongh-112097. Previously, a `end_lineno` could be specified to be `None` directly but since 939fc6d, passing None makes the constructor use the value of `lineno` instead. (cherry picked from commit c6513f7) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

…nd_lineno` changed in 3.13 (GH-130755) (#130767) gh-128481: indicate that the default value for `FrameSummary.end_lineno` changed in 3.13 (GH-130755) The value taken by `FrameSummary.end_lineno` when passing `end_lineno=None` changed in gh-112097. Previously, a `end_lineno` could be specified to be `None` directly but since 939fc6d, passing None makes the constructor use the value of `lineno` instead. (cherry picked from commit c6513f7) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

williamwen42 requested a review from iritkatriel as a code owner November 15, 2023 01:34

bedevere-appbot added the awaiting review label Nov 15, 2023

bedevere-appbot mentioned this pull request Nov 15, 2023
Support multi-line error locations in traceback and other related improvements (PEP-657, 3.11) #106922
Closed

pablogsal requested review from isidentical, lysnikolaou and pablogsal November 15, 2023 16:13

pablogsal self-assigned this Nov 15, 2023

williamwen42 force-pushed the multiline-traceback-attempt-2 branch from 9d9a510 to 4094c93Compare November 17, 2023 19:32

pablogsal reviewed Nov 18, 2023
View reviewed changes

lysnikolaou reviewed Nov 20, 2023
View reviewed changes

williamwen42and others added 4 commits November 21, 2023 14:04

Support multi-line error locations in traceback (attempt 2)
2644341

📜🤖 Added by blurb_it.
c1f8d0b

fix lint and doc test failures
3152844

address pr comments
425c874

williamwen42 force-pushed the multiline-traceback-attempt-2 branch from 4094c93 to 425c874Compare November 21, 2023 22:04

williamwen42 requested review from lysnikolaou and pablogsal November 22, 2023 00:01

pablogsal approved these changes Dec 1, 2023
View reviewed changes

bedevere-appbot added awaiting merge and removed awaiting review labels Dec 1, 2023

lysnikolaou approved these changes Dec 1, 2023
View reviewed changes

Merge branch 'main' into multiline-traceback-attempt-2
873501f

add more comments, address some nits
99094bb

pablogsal merged commit 939fc6d into python:mainDec 1, 2023

bedevere-appbot removed the awaiting merge label Dec 1, 2023

aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
pythongh-106922: Support multi-line error locations in traceback (att…
72da3a8
…empt 2) (python#112097)

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
pythongh-106922: Support multi-line error locations in traceback (att…
88e3c6c
…empt 2) (python#112097)

cretz mentioned this pull request Dec 18, 2024
Drop Python 3.8, add 3.13 temporalio/sdk-python#694
Merged

miss-islington mentioned this pull request Mar 2, 2025
[3.13] gh-128481: indicate that the default value for FrameSummary.end_lineno changed in 3.13 (GH-130755) #130767
Merged

	tree=ast.parse("(\n"+segment+"\n)")
	tree=ast.parse(f"(\n{segment}\n)")

Uh oh!

gh-106922: Support multi-line error locations in traceback (attempt 2)#112097

gh-106922: Support multi-line error locations in traceback (attempt 2) #112097

Uh oh!

Conversation

williamwen42 commented Nov 15, 2023• edited by bedevere-app botLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-appbot commented Nov 15, 2023

Uh oh!

williamwen42 commented Nov 15, 2023

Uh oh!

pablogsal commented Nov 17, 2023

Uh oh!

pablogsal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

williamwen42Nov 20, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

williamwen42Nov 21, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pablogsal commented Nov 18, 2023

Uh oh!

lysnikolaou left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

williamwen42 commented Nov 27, 2023

Uh oh!

pablogsal commented Nov 27, 2023

Uh oh!

pablogsal left a comment

Choose a reason for hiding this comment

Uh oh!

pablogsal commented Dec 1, 2023

Uh oh!

lysnikolaou commented Dec 1, 2023

Uh oh!

williamwen42 commented Nov 15, 2023•
edited by bedevere-app bot
Loading

williamwen42Nov 20, 2023•
edited
Loading

williamwen42Nov 21, 2023•
edited
Loading