Repository: pypa/pipx
Pull request #1191 (2024-01-05 16:32:27)

fix: nicer tracebacks in run mode on scripts
This is one way to improve the tracebacks as reported in #1187 and mentioned in #1180.

<!-- add an 'x' in the brackets below -->

- [x] I have added an entry to `docs/changelog.md`

## Summary of changes

This is the simplest way I initially see to get filenames. Instead of passing around the contents of the script as a string, scripts are passed around as Paths, which allows the run command to run the path rather than passing the string on the command line.

## Test plan

<!-- provide evidence of testing, preferably with command(s) that can be copy+pasted by others -->

If you start with this script:

```python
import httpx
```

You get this:

```console
$ pipx run ./script.py
Traceback (most recent call last):
  File "<string>", line 1, in <module>
    import httpx
ModuleNotFoundError: No module named 'httpx'
```

After this PR, you get this:

```console
$ pipx run ./script.py
Traceback (most recent call last):
  File "/Users/henryschreiner/git/software/pipx/script.py", line 1, in <module>
    import httpx
ModuleNotFoundError: No module named 'httpx'
```


chrysle: I think we should really enable merge queues, this constant branch updating is annoying.
rsalmaso: Happy for this patch!

Running script instead of string solve another problem: run the script via shebang `#!/usr/bin/env -S pipx run` and get the correct filename `sys.argv[0]`.
henryiii: What should this do if a URI is passed, by the way? Currently that still says "<string>". That could be a future improvement, but something to think about.
dukecat0: > What should this do if a URI is passed, by the way? Currently that still says "<string>". That could be a future improvement, > but something to think about.

Perhaps we can have an additional check here?
https://github.com/pypa/pipx/blob/5563ae68aecda0b36e59979bcbd8e81a96ec3fbd/src/pipx/commands/run.py#L57
henryiii: A check wouldn't help, we are already doing the "right" think (since when getting a URI it needs to be downloaded, possibly from a remote source), it's just then a question of if there's a way to get it to show something nicer than "<string>". This was fixable since loading a local file can just be directly run by the `python` command. I would assume there's a way to customize running on arbitrary input, since things like IPython do it (though at in least some cases, they write out temporary files).

Patch:
diff --git a/changelog.d/1191.fix.md b/changelog.d/1191.fix.md
new file mode 100644
index 0000000..7ea5acd
--- /dev/null
+++ b/changelog.d/1191.fix.md
@@ -0,0 +1,1 @@
+Report correct filename in tracebacks with `pipx run <scriptname>`
diff --git a/src/pipx/commands/run.py b/src/pipx/commands/run.py
index eff9af0..1e036de 100644
--- a/src/pipx/commands/run.py
+++ b/src/pipx/commands/run.py
@@ -8,7 +8,7 @@ import urllib.parse
 import urllib.request
 from pathlib import Path
 from shutil import which
-from typing import List, NoReturn, Optional
+from typing import List, NoReturn, Optional, Union
 
 from packaging.requirements import InvalidRequirement, Requirement
 
@@ -42,14 +42,14 @@ Available executable scripts:
     {app_lines}"""
 
 
-def maybe_script_content(app: str, is_path: bool) -> Optional[str]:
+def maybe_script_content(app: str, is_path: bool) -> Optional[Union[str, Path]]:
     # If the app is a script, return its content.
     # Return None if it should be treated as a package name.
 
     # Look for a local file first.
     app_path = Path(app)
     if app_path.is_file():
-        return app_path.read_text(encoding="utf-8")
+        return app_path
     elif is_path:
         raise PipxError(f"The specified path {app} does not exist")
 
@@ -71,7 +71,7 @@ def maybe_script_content(app: str, is_path: bool) -> Optional[str]:
 
 
 def run_script(
-    content: str,
+    content: Union[str, Path],
     app_args: List[str],
     python: str,
     pip_args: List[str],
@@ -81,7 +81,7 @@ def run_script(
 ) -> NoReturn:
     requirements = _get_requirements_from_script(content)
     if requirements is None:
-        exec_app([python, "-c", content, *app_args])
+        python_path = Path(python)
     else:
         # Note that the environment name is based on the identified
         # requirements, and *not* on the script name. This is deliberate, as
@@ -99,7 +99,12 @@ def run_script(
             venv = Venv(venv_dir, python=python, verbose=verbose)
             venv.create_venv(venv_args, pip_args)
             venv.install_unmanaged_packages(requirements, pip_args)
-        exec_app([venv.python_path, "-c", content, *app_args])
+        python_path = venv.python_path
+
+    if isinstance(content, Path):
+        exec_app([python_path, content, *app_args])
+    else:
+        exec_app([python_path, "-c", content, *app_args])
 
 
 def run_package(
@@ -323,11 +328,14 @@ def _http_get_request(url: str) -> str:
 INLINE_SCRIPT_METADATA = re.compile(r"(?m)^# /// (?P<type>[a-zA-Z0-9-]+)$\s(?P<content>(^#(| .*)$\s)+)^# ///$")
 
 
-def _get_requirements_from_script(content: str) -> Optional[List[str]]:
+def _get_requirements_from_script(content: Union[str, Path]) -> Optional[List[str]]:
     """
     Supports inline script metadata.
     """
 
+    if isinstance(content, Path):
+        content = content.read_text(encoding="utf-8")
+
     name = "script"
 
     # Windows is currently getting un-normalized line endings, so normalize


Test patch:
diff --git a/tests/test_run.py b/tests/test_run.py
index 0de3920..7d93556 100644
--- a/tests/test_run.py
+++ b/tests/test_run.py
@@ -243,6 +243,24 @@ def test_run_with_requirements_old(caplog, pipx_temp_env, tmp_path):
         run_pipx_cli_exit(["run", script.as_uri()])
 
 
+@mock.patch("os.execvpe", new=execvpe_mock)
+def test_run_correct_traceback(capfd, pipx_temp_env, tmp_path):
+    script = tmp_path / "test.py"
+    script.write_text(
+        textwrap.dedent(
+            """
+                raise RuntimeError("Should fail")
+            """
+        ).strip()
+    )
+
+    with pytest.raises(SystemExit):
+        run_pipx_cli(["run", str(script)])
+
+    captured = capfd.readouterr()
+    assert "test.py" in captured.err
+
+
 @mock.patch("os.execvpe", new=execvpe_mock)
 def test_run_with_args(caplog, pipx_temp_env, tmp_path):
     script = tmp_path / "test.py"