PyCQA / flake8

flake8 is a python tool that glues together pycodestyle, pyflakes, mccabe, and third-party plugins to check the style and quality of some python code.
https://flake8.pycqa.org
Other
3.39k stars 306 forks source link

Piping flake8 to head causes ungraceful "Broken pipe" handling in formatting/base.py #1943

Closed Torxed closed 2 months ago

Torxed commented 2 months ago

how did you install flake8?

$ pacman -Sy flake8

unmodified output of flake8 --bug-report

{
  "platform": {
    "python_implementation": "CPython",
    "python_version": "3.12.3",
    "system": "Linux"
  },
  "plugins": [
    {
      "plugin": "mccabe",
      "version": "0.7.0"
    },
    {
      "plugin": "pycodestyle",
      "version": "2.11.1"
    },
    {
      "plugin": "pyflakes",
      "version": "3.2.0"
    }
  ],
  "version": "7.0.0"
}

describe the problem

what I expected to happen

Flake8 handling sys.stdout closure gracefully.

sample code

Any code will work, and will generate:

Traceback (most recent call last):
  File "/usr/bin/flake8", line 33, in <module>
    sys.exit(load_entry_point('flake8==7.0.0', 'console_scripts', 'flake8')())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/flake8/main/cli.py", line 23, in main
    app.run(argv)
  File "/usr/lib/python3.12/site-packages/flake8/main/application.py", line 198, in run
    self._run(argv)
  File "/usr/lib/python3.12/site-packages/flake8/main/application.py", line 188, in _run
    self.report()
  File "/usr/lib/python3.12/site-packages/flake8/main/application.py", line 180, in report
    self.report_errors()
  File "/usr/lib/python3.12/site-packages/flake8/main/application.py", line 141, in report_errors
    results = self.file_checker_manager.report()
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/flake8/checker.py", line 190, in report
    results_reported += self._handle_results(filename, results)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/flake8/checker.py", line 166, in _handle_results
    reported_results_count += style_guide.handle_error(
                              ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/flake8/style_guide.py", line 294, in handle_error
    return guide.handle_error(
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/flake8/style_guide.py", line 428, in handle_error
    self.formatter.handle(error)
  File "/usr/lib/python3.12/site-packages/flake8/formatting/base.py", line 99, in handle
    self.write(line, source)
  File "/usr/lib/python3.12/site-packages/flake8/formatting/base.py", line 196, in write
    self._write(source)
  File "/usr/lib/python3.12/site-packages/flake8/formatting/base.py", line 178, in _write
    sys.stdout.buffer.write(output.encode() + self.newline.encode())
BrokenPipeError: [Errno 32] Broken pipe
Exception ignored in: <_io.TextIOWrapper name='<stdout>' mode='w' encoding='utf-8'>
BrokenPipeError: [Errno 32] Broken pipe

commands ran

$ flake8 . | head
asottile commented 2 months ago

this is a common problem with python itself due to some "interesting" choices in the Io implementation

try for instance with a print loop

Torxed commented 2 months ago

Can you elaborate on "try a print loop"? I feel this is a flake8 cli tooling issue, where it's missing something along the lines of:

try:
    sys.stdout.buffer.write(...)
except BrokenPipeError:
    exit()
sigmavirus24 commented 2 months ago

@Torxed printing to a tty (or pty) is not the only use-case of Flake8. It's possible for someone to print to both stdout and to a file simultaneously with CLI flags. If we simply exit when something goes wrong (and do so silently) then we truncate what we write to the file without notice. If instead we just ignore every BrokenPipeError so writing to the file succeeds, what do we do then? Exit with non-zero somehow?

You're proposing a simple solution to something which fits your simplistic mental model which happens to be wrong.

The better solution is to fail loudly so the user knows that something went wrong unexpectedly. The point Anthony is making above is that Python has chosen to implement things this way. Other languages don't break in this particular manner, so fundamentally this is a flaw in Python.