Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrectly converting relative to absolute of files and always show paths in traceback in case file cannot be found #3429

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

AndreasBackx
Copy link

@AndreasBackx AndreasBackx commented Jul 15, 2024

Type of changes

  • Bug fix
  • New feature
  • Documentation / docstrings
  • Tests
  • Other

Checklist

  • I've run the latest black with default args on new code.
  • I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
  • I've added tests for new code.
  • I accept that @willmcgugan may be pedantic in the code review.

Description

Currently Rich assumes that if a path in a traceback frame is relative, that it's relative to the current working directory. This is not the case when the Python application is running from a packaged bundle like a PAR. In these cases the traceback loses the path information or the incorrect absolute path is used. This PR makes it so:

  • Frame.filename is not absolute if the original filename does not exist in the first place. That would indicate that it's not relative to the current working directory.
  • The path/filename is always shown in the traceback, not only if it exists.

I was considering replacing rich.__init__._IMPORT_CWD's value to one that is reflective of __file__ though that resulted in filenames relative to /proc. Those are not readable and this might change other expectations.

The linecache module understands the relative paths and even sometimes can give the lines inside of the PAR. (Though sometimes it cannot and I'm not sure when it can and cannot.) Regardless, this means we at least see some lines and also always see the location.

@AndreasBackx AndreasBackx changed the title Fix incorrectly converting relative to absolute of nonexistent files and always show paths in traceback. Fix incorrectly converting relative to absolute of files and always show paths in traceback in case file cannot be found Jul 15, 2024
@willmcgugan
Copy link
Collaborator

Any chance of tests? Although I can see why tests are tricky with this kind of thing.

@AndreasBackx
Copy link
Author

@willmcgugan Will have a look when I'm back in the office. Currently on long(er) holiday. Could you have a look at #1859?

@AndreasBackx
Copy link
Author

I've added some tests to test the behaviour here. It's a bit annoying to test because of how types.FrameType and similar classes are not instantiatable. This might be a bit fragile and maybe mocks should be used instead of "fake" dataclasses. Though I feel like that's equally ugly and annoying to work with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants