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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,16 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased

### Fixed

- `Frame.filename` no longer incorrectly set to an incorrect absolute path if the original path cannot be found.

### Changed

- Traceback filename/path is always shown, even if the file cannot be found.

## [13.9.2] - 2024-10-04

### Fixed
Expand Down
1 change: 1 addition & 0 deletions CONTRIBUTORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ The following people have contributed to the development of Rich:
<!-- Add your name below, sort alphabetically by surname. Link to GitHub profile / your home page. -->

- [Patrick Arminio](https://github.com/patrick91)
- [Andreas Backx](https://github.com/AndreasBackx)
- [Gregory Beauregard](https://github.com/GBeauregard/pyffstream)
- [Artur Borecki](https://github.com/pufereq)
- [Pedro Aaron](https://github.com/paaaron)
Expand Down
35 changes: 15 additions & 20 deletions rich/traceback.py
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,13 @@ def get_locals(
(end_line, end_column),
)

if filename and not filename.startswith("<"):
if not os.path.isabs(filename):
filename = os.path.join(_IMPORT_CWD, filename)
if (
filename
and not filename.startswith("<")
and not os.path.isabs(filename)
and os.path.exists(filename)
):
filename = os.path.join(_IMPORT_CWD, filename)
if frame_summary.f_locals.get("_rich_traceback_omit", False):
continue

Expand Down Expand Up @@ -695,23 +699,14 @@ def render_locals(frame: Frame) -> Iterable[ConsoleRenderable]:
frame_filename = frame.filename
suppressed = any(frame_filename.startswith(path) for path in self.suppress)

if os.path.exists(frame.filename):
text = Text.assemble(
path_highlighter(Text(frame.filename, style="pygments.string")),
(":", "pygments.text"),
(str(frame.lineno), "pygments.number"),
" in ",
(frame.name, "pygments.function"),
style="pygments.text",
)
else:
text = Text.assemble(
"in ",
(frame.name, "pygments.function"),
(":", "pygments.text"),
(str(frame.lineno), "pygments.number"),
style="pygments.text",
)
text = Text.assemble(
path_highlighter(Text(frame.filename, style="pygments.string")),
(":", "pygments.text"),
(str(frame.lineno), "pygments.number"),
" in ",
(frame.name, "pygments.function"),
style="pygments.text",
)
if not frame.filename.startswith("<") and not first:
yield ""
yield text
Expand Down
77 changes: 76 additions & 1 deletion tests/test_traceback.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
from __future__ import annotations

import io
import os
import re
import sys
from typing import List
import tempfile
from collections.abc import Mapping
from dataclasses import dataclass, field
from typing import Any, Iterable, List

import pytest

Expand Down Expand Up @@ -358,3 +364,72 @@ def test_traceback_finely_grained() -> None:
start, end = last_instruction
print(start, end)
assert start[0] == end[0]


@dataclass(frozen=True)
class FakeTraceback:
tb_frame: FakeFrameType
tb_lineno: int = 0
tb_next: FakeTraceback | None = None


@dataclass(frozen=True)
class FakeFrameType:
f_code: FakeCodeType
f_lasti: int = 0
f_locals: Mapping[str, Any] = field(default_factory=dict)


@dataclass(frozen=True)
class FakeCodeType:
co_filename: str
co_name: str = "co_name"

def co_positions(
self,
) -> Iterable[tuple[int | None, int | None, int | None, int | None]]:
return [(None, None, None, None)]


def test_traceback_non_existent_paths() -> None:
existing_absolute_file = tempfile.NamedTemporaryFile()
existing_relative_file = tempfile.NamedTemporaryFile(dir=".")
nonexisting_absolute_filename = "/absolute_nonexisting_file"
nonexisting_relative_filename = "relative_nonexisting_file"
traceback_filenames = [
# Existing absolute path should be unchanged.
existing_absolute_file.name,
# The relative path should be converted into an absolute path below.
os.path.basename(existing_relative_file.name),
# Nonexisting absolute path should be unchanged.
nonexisting_absolute_filename,
# Nonexisting relative path should be unchanged.
nonexisting_relative_filename,
]
expected_traceback_filenames = [
existing_absolute_file.name,
# This is the absolute path, not the relative path as above.
existing_relative_file.name,
nonexisting_absolute_filename,
nonexisting_relative_filename,
]
for traceback_filename, expected_traceback_filename in zip(
traceback_filenames, expected_traceback_filenames
):
fake_traceback = FakeTraceback(
tb_frame=FakeFrameType(
f_code=FakeCodeType(
co_filename=traceback_filename,
)
),
)
rich_traceback = Traceback.extract(
exc_type=Exception,
exc_value=Exception(),
traceback=fake_traceback,
)
assert len(rich_traceback.stacks) == 1
stack = rich_traceback.stacks[0]
assert len(stack.frames) == 1
frame = stack.frames[0]
assert frame.filename == expected_traceback_filename