Skip to content

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

Closed

Description

@williamwen42

Feature or enhancement

We propose a few improvements and fixes to PEP-657, namely:

  1. Support underlining errors that span across multiple lines instead of only showing the first line
  2. Use caret anchors for function calls as well
  3. Fix bracket/binary op heuristic in the caret anchor computation function

Pitch

We already implemented these items in PyTorch here: pytorch/pytorch#104676. We're seeing if these may be worth adding to CPython.

Rationale for 1

Multi-line expressions can negate the utility of PEP-657, for example:

really_long_expr_1 = 1
really_long_expr_2 = 2
really_long_expr_3 = 0
really_long_expr_4 = 4

y = (
    (
        really_long_expr_1 +
        really_long_expr_2
    ) /
    really_long_expr_2 /
    really_long_expr_3
)

Current traceback:

Traceback (most recent call last):
  File "/scratch/williamwen/work/pytorch/playground5.py", line 25, in <module>
    (
ZeroDivisionError: float division by zero

Better traceback:

Traceback (most recent call last):
  File "/scratch/williamwen/work/pytorch/playground5.py", line 25, in <module>
    (
    ~
        really_long_expr_1 +
        ~~~~~~~~~~~~~~~~~~~~
        really_long_expr_2
        ~~~~~~~~~~~~~~~~~~
    ) /
    ~~^
    really_long_expr_2 /
    ~~~~~~~~~~~~~~~~~~
ZeroDivisionError: float division by zero

Rationale for 2

Helpful for identifying function calls that cause errors in chained function calls. We may as well do it since we're already doing it for subscripts. For example:

def f1(x1):
    def f2(x2):
        raise RuntimeError()
    return f2
y = f1(1)(2)(3)

Current traceback:

Traceback (most recent call last):
  File "/scratch/williamwen/work/pytorch/playground5.py", line 22, in <module>
    y = f1(1)(2)(3)
        ^^^^^^^^
  File "/scratch/williamwen/work/pytorch/playground5.py", line 6, in f2
    raise RuntimeError()
RuntimeError

Better traceback:

Traceback (most recent call last):
  File "/scratch/williamwen/work/pytorch/playground5.py", line 22, in <module>
    y = f1(1)(2)(3)
        ~~~~~^^^
  File "/scratch/williamwen/work/pytorch/playground5.py", line 6, in f2
    raise RuntimeError()
RuntimeError

Rationale for 3

The binary op anchors are computed by taking the next non-space character after the left subexpression. This is incorrect in some simple cases:

    x = (3) / 0
        ~~^~~~~
ZeroDivisionError: division by zero

We should expect

    x = (3) / 0
        ~~~~^~~
ZeroDivisionError: division by zero

The fix is that we should continue advancing the anchor until we find a non-space character that is also not in )\# (for \#, we should move the anchor to the next line).

Subscript has a similar issue as well. We should continue advancing the left anchor until we find [, and the right anchor should be the end of the entire subscript expression.

cc @pablogsal @isidentical @ammaraskar @iritkatriel @ezyang

Linked PRs

Activity

  1. pablogsal commented on Jul 22, 2023

    @pablogsal
    Member

    Hi @williamwen42! Thanks a lot for opening the issue. I think all of these make sense and would be great improvements! Do you plan to contribute these improvements or the issue is just for the proposal itself?

  2. williamwen42 commented on Jul 24, 2023

    @williamwen42
    ContributorAuthor

    @pablogsal I can contribute these improvements since I already have prototypes written. I also want to double check if we're okay with changing the default traceback behavior from one line to multiline.

  3. pablogsal commented on Jul 28, 2023

    @pablogsal
    Member

    @pablogsal I can contribute these improvements since I already have prototypes written.

    Ok, let's do it then. Please, ping me in the PR as soon as is available! Just to make this clear: this needs changes in both the traceback module and the default traceback logic in C.

    I also want to double-check if we're okay with changing the default traceback behavior from one line to multiline.

    I will be supportive and have the feeling that it won't be controversial but we may need to discuss it. In any case the discussion would be much easier if we have a PR to play with. Maybe, if is not too bothersome, you can split that particular aspect in a separate PR, but is not a requirement.

    Thanks a lot for the help ❤️

  4. pablogsal commented on Sep 5, 2023

    @pablogsal
    Member

    @williamwen42 any news on this?

  5. self-assigned this
    on Sep 5, 2023
  6. added a commit that references this issue on Sep 8, 2023
  7. added 2 commits that reference this issue on Sep 8, 2023
  8. added 2 commits that reference this issue on Sep 8, 2023
  9. williamwen42 commented on Sep 8, 2023

    @williamwen42
    ContributorAuthor

    @pablogsal I'm still working on the C implementation. I've been distracted with work/travel in the last month but I'm planning to return to complete this in the near future.

  10. pablogsal commented on Sep 8, 2023

    @pablogsal
    Member

    Ok, just be aware that I have treated (3) as a bugfix in #108959 so that's not required anymore. I can take care of (2) if I get some spare cycles but I will certainly leave (1) to you :)

  11. added a commit that references this issue on Dec 1, 2023
  12. pablogsal commented on Dec 4, 2023

    @pablogsal
    Member

    @williamwen42 I think there is one thing we should consider. Think of the following example:

    def baz(*args):
        return 1/0
    
    def bar():
        return baz(1,
               2, 3
               , 4) + baz(1,
               2, 3
               , 4)
    
    bar()
    

    This currently shows:

    Traceback (most recent call last):
      File "/Users/pgalindo3/github/python/main/lel.py", line 11, in <module>
        bar()
        ~~~^^
      File "/Users/pgalindo3/github/python/main/lel.py", line 5, in bar
        return baz(1,
               ~~~^^^
               2, 3
               ^^^^
               , 4)
               ^^^^
      File "/Users/pgalindo3/github/python/main/lel.py", line 2, in baz
        return 1/0
               ~^~
    ZeroDivisionError: division by zero
    

    but that is odd as the line that shows , 4) doesn't show the rest of the line , 4) + baz(1,. I think we should ensure we show the actual line to show what happens on the single line case.

  13. williamwen42 commented on Dec 5, 2023

    @williamwen42
    ContributorAuthor

    @pablogsal hmm, I currently get

    Traceback (most recent call last):
      File "/data/users/williamwen/cpython/local_test.py", line 11, in <module>
        bar()
        ~~~^^
      File "/data/users/williamwen/cpython/local_test.py", line 5, in bar
        return baz(1,
               ~~~^^^
               2, 3
               ^^^^
               , 4) + baz(1,
               ^^^^
      File "/data/users/williamwen/cpython/local_test.py", line 2, in baz
        return 1/0
               ~^~
    ZeroDivisionError: division by zero
    

    on main

  14. pablogsal commented on Dec 6, 2023

    @pablogsal
    Member

    Ah, apologies. I was testing this on top of #112732 and looks I screwed up something on that PR because that part is missing.

    This means this is working on main but we don't cover it with a test. Would you like to add a test for it?

  15. williamwen42 commented on Dec 6, 2023

    @williamwen42
    ContributorAuthor

    I think test_caret_for_call_multiline in test_traceback covers it:

                'Traceback (most recent call last):\n'
                f'  File "{__file__}", line {self.callable_line}, in get_exception\n'
                '    callable()\n'
                '    ~~~~~~~~^^\n'
                f'  File "{__file__}", line {lineno_f+8}, in f_with_call\n'
                '    a = (g(1).y)(\n'
                '        ~~~~~~~~~\n'
                '        2\n'
                '        ~\n'
                '    )(3)(4)\n'
                '    ~^^^\n'
                f'  File "{__file__}", line {lineno_f+4}, in f\n'
                '    raise RuntimeError("fail")\n'
  16. pablogsal commented on Dec 6, 2023

    @pablogsal
    Member

    I think test_caret_for_call_multiline in test_traceback covers it:

                'Traceback (most recent call last):\n'
    
                f'  File "{__file__}", line {self.callable_line}, in get_exception\n'
    
                '    callable()\n'
    
                '    ~~~~~~~~^^\n'
    
                f'  File "{__file__}", line {lineno_f+8}, in f_with_call\n'
    
                '    a = (g(1).y)(\n'
    
                '        ~~~~~~~~~\n'
    
                '        2\n'
    
                '        ~\n'
    
                '    )(3)(4)\n'
    
                '    ~^^^\n'
    
                f'  File "{__file__}", line {lineno_f+4}, in f\n'
    
                '    raise RuntimeError("fail")\n'

    I don't think it does because my PR was showing the wrong output and the CI was passing. I can look closer tomorrow

  17. pablogsal commented on Dec 6, 2023

    @pablogsal
    Member

    Ah forget that, the problem is that the colorising code in my PR was just being used outside most tests in the test suite. This means I should add more tests on my side. 😅

    Sorry for the misunderstanding

  18. added a commit that references this issue on Feb 11, 2024
  19. indiVar0508 commented on Apr 19, 2024

    @indiVar0508

    Hi
    i stumbled upon this issue from 72da3a8, which seem to fix issue i am facing in below mentioned discussion in py3.13
    https://discuss.python.org/t/how-can-i-pass-linecache-over-an-exec-mthod-local-global-scope/51192/2

    tl;dr: for python3 < 3.13 frameSummary.line attribute becomes None when i create a method using exec with given code string due to which i couldn't see content of traceback line in my error traceback.
    i tried adding linecache but when my codestring runs as task in distributed system source becomes None and FrameSummary.line is empty.

    Wanted to know if you have any suggestion for solving this to point me in right direction

    Thanks

  20. williamwen42 commented on Apr 19, 2024

    @williamwen42
    ContributorAuthor

    @pablogsal will #112097 be backported to 3.11 and 3.12?

  21. pablogsal commented on Apr 19, 2024

    @pablogsal
    Member

    Unfortunately no since it's a new feature

  22. added a commit that references this issue on Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

interpreter-core(Objects, Python, Grammar, and Parser dirs)type-featureA feature request or enhancement

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions