Incorrect OSError raised when redirecting to NUL on Windows

[uranusjr]

Reproducible for both Python 3.6 and 3.7. I did not test against older versions.

Tested on the Click 6.x series. Windows 10, Build 17134.

Steps to reproduce:

# t.py
import click

@click.command()
def main():
    click.echo('run')

if __name__ == '__main__':
    main()

> python t.py
run

> python t.py >NUL
Traceback (most recent call last):
  File "t.py", line 10, in <module>
    main()
  File ".venv\lib\site-packages\click\core.py", line 722, in __call__
    return self.main(*args, **kwargs)
  File ".venv\lib\site-packages\click\core.py", line 697, in main
    rv = self.invoke(ctx)
  File ".venv\lib\site-packages\click\core.py", line 895, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File ".venv\lib\site-packages\click\core.py", line 535, in invoke
    return callback(*args, **kwargs)
  File "t.py", line 6, in main
    click.echo('run')
  File ".venv\lib\site-packages\click\utils.py", line 259, in echo
    file.write(message)
  File ".venv\lib\site-packages\click\_winconsole.py", line 180, in write
    return self._text_stream.write(x)
  File ".venv\lib\site-packages\click\_winconsole.py", line 164, in write
    raise OSError(self._get_error_message(GetLastError()))
OSError: Windows error 1

I believe the problem is that Click tries to be helpful, and raises an error when there are not bytes written when there should be. This is, however, exactly what is expected to happen when you redirect things to NUL on Windows.

Curiously, however, Windows seems to act a little differently for stderr. 2>NUL works as expected.

[uranusjr]

I added an additional check based on MSDN’s suggestion:

--- a/click/_winconsole.py
+++ b/click/_winconsole.py
@@ -33,6 +33,8 @@ kernel32 = windll.kernel32
 GetStdHandle = kernel32.GetStdHandle
 ReadConsoleW = kernel32.ReadConsoleW
 WriteConsoleW = kernel32.WriteConsoleW
+WriteFile = kernel32.WriteFile
+GetConsoleMode = kernel32.GetConsoleMode
 GetLastError = kernel32.GetLastError
 GetCommandLineW = WINFUNCTYPE(LPWSTR)(
     ('GetCommandLineW', windll.kernel32))
@@ -156,7 +158,11 @@ class _WindowsConsoleWriter(_WindowsConsoleRawIOBase):
                                        MAX_BYTES_WRITTEN) // 2
         code_units_written = c_ulong()

-        WriteConsoleW(self.handle, buf, code_units_to_be_written,
+        if GetConsoleMode(self.handle, byref(c_ulong())):
+            WriteConsoleW(self.handle, buf, code_units_to_be_written,
+                          byref(code_units_written), None)
+        else:
+            WriteFile(self.handle, buf, code_units_to_be_written,
                       byref(code_units_written), None)
         bytes_written = 2 * code_units_written.value

This checks if the handle is actually a console with GetConsoleMode. If that succeeds (no matter what the mode is). it is a console, and we can safely use WriteConsoleW. Otherwise we should use WriteFile instead. This seems to work, according to my simple tests.

I am not very familiar with Windows API, and would like some input whether this is actually a good idea.

[segevfiner]

We shouldn't even be using this code if stdio is redirected, but Windows, being Windows, catches us off guard here: https://bugs.python.org/issue28654.

In essence, isatty returns True for the NUL device on Windows!

CPython itself uses GetConsoleMode to test if the HANDLE/fd is a console, so the right fix will be to use GetConsoleMode instead of isatty when checking if we should use our Unicode console hack implementation.

P.S. @davidism Why is this code used on CPython 3.6+ where the Windows Unicode console hack is implemented by CPython itself?