FileNotFoundError escapes from run_formatters()

[Jacob Walls]

A couple coworkers of mine have now run into ​this issue described on the Django forum.

In essence, it's not safe to assume that just because shutil.which("black") returns a path that subprocess.run() can execute that path.

Some ​complications with shutil.which(..., path=None):

• Reads a path variable from one or two fallbacks (either os.environ() or os.defpath)
• Behavior is different on Windows
• Behavior is different on Windows with Python 3.12.0
• Behavior is different on Windows with Python 3.12.1

My impression of the feature was that it was aiming at a frictionless experience -- format if you have a working black install, don't if you don't.

Suggesting we should catch FileNotFoundError (at least) and at most log out an explanation why the file wasn't formatted. Users who only have a copy of black in abandoned environments are unlikely to care about their files not being formatted. :-)

[Mariusz Felisiak]

I cannot reproduce it on Linux. Is it Windows-specific issue? Did you manage to reproduce it? Is not this an issue in Python instead?

[Jacob Walls]

My colleagues were developing on Macs. I just mentioned the Windows information when seeing it in the python docs.

This isn't a solid reproducer, but I managed to hack together an example that at least fails to launch. Something else must be responsible for escaping the FileNotFoundError, since the call to subprocess.run does not include check=True. (I'm not saying this is how my colleagues reproduced it they had black installs from in abandoned environments, unsure what versions of pip, black, homebrew etc were used):

~ % chmod 777 /opt/homebrew/bin/black
~ % chmod 644 /opt/homebrew/bin/black
~ % chmod +u /opt/homebrew/bin/black
~ % chmod -u /opt/homebrew/bin/black
~ % chmod +u /opt/homebrew/bin/black
~ % chmod +x /opt/homebrew/bin/black
~ % python3.12                       
Python 3.12.0 (v3.12.0:0fb18b02c8, Oct  2 2023, 09:45:56) [Clang 13.0.0 (clang-1300.0.29.30)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import shutil
>>> import subprocess
>>> subprocess.run(shutil.which('black'))
/opt/homebrew/Cellar/python@3.11/3.11.5/Frameworks/Python.framework/Versions/3.11/Resources/Python.app/Contents/MacOS/Python: can't open file '/opt/homebrew/bin/black': [Errno 13] Permission denied
CompletedProcess(args='/opt/homebrew/bin/black', returncode=2)

[Natalia Bidart]

I can see value in handling these two exceptions (even in Linux):

• PermissionError: somehow the path is not executable or can not be read by the user executing the formatting.
• FileNotFoundError: black path disappeared between it was calculated by shutil.which and it was executed by subprocess.run (disk full, version upgrade, etc).

As long as we log the fact that we were about to run black but we couldn't, I think this is a welcomed improvement. Accepting on that basis (follow up of #33476).

[Mariusz Felisiak]

I'm torn. Applying formatters should be smooth, on the other hand, hiding errors can make it more difficult to debug/notice potential issues in black installations.

[Natalia Bidart]

I agree regarding visibility of errors, that's why I accepted the ticket as long as errors are "shown" somehow.

[Jacob Walls]

​PR