pyls-mypy icon indicating copy to clipboard operation
pyls-mypy copied to clipboard

mypy errors are not captured and end up directly in the process stdout

Open randomstuff opened this issue 6 years ago • 7 comments

While debugging an error appearing in Atom IDE-Python plugin, we found that some errors reported by mypy sometimes end up appearing directly in the process stdout (outside on the JSON body) which breaks the protocol:

Content-Length: 43
Content-Type: application/vscode-jsonrpc; charset=utf8

{"jsonrpc": "2.0", "id": 5, "result": null}
:1:1: error: Cannot find module named 'cryptography.hazmat.primitives'
:1:1: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
:2:1: error: Cannot find module named 'cryptography.hazmat.primitives.kdf.hkdf'
:3:1: error: Cannot find module named 'cryptography.hazmat.backends.openssl'
:4:1: error: Cannot find module named 'cryptography.hazmat.primitives.ciphers'
:11:1: error: Cannot find module named 'filetype'

You can easily check the output of pyls with sysdig:

sysdig -s6000 'proc.cmdline="python -m pyls"' -c stdout

AFAIU, this happens because mypy API currently works by temporarily overriding sys.stdout and sys.stderr which is not thread-safe:

def _run(f: Callable[[], None]) -> Tuple[str, str, int]:
    old_stdout = sys.stdout
    new_stdout = StringIO()
    sys.stdout = new_stdout

    old_stderr = sys.stderr
    new_stderr = StringIO()
    sys.stderr = new_stderr

    try:
        f()
        exit_status = 0
    except SystemExit as system_exit:
        exit_status = system_exit.code
    finally:
        sys.stdout = old_stdout
        sys.stderr = old_stderr

    return new_stdout.getvalue(), new_stderr.getvalue(), exit_status

def run(args: List[str]) -> Tuple[str, str, int]:
    # Lazy import to avoid needing to import all of mypy to call run_dmypy
    from mypy.main import main
    return _run(lambda: main(None, args=args))

The linter tasks are handled in separate threads because of the debouncing feature of python-language-server:

def debounce(interval_s, keyed_by=None):
    """Debounce calls to this function until interval_s seconds have passed."""
    def wrapper(func):
        timers = {}
        lock = threading.Lock()

        @functools.wraps(func)
        def debounced(*args, **kwargs):
            call_args = inspect.getcallargs(func, *args, **kwargs)
            key = call_args[keyed_by] if keyed_by else None

            def run():
                with lock:
                    del timers[key]
                return func(*args, **kwargs)

            with lock:
                old_timer = timers.get(key)
                if old_timer:
                    old_timer.cancel()

                timer = threading.Timer(interval_s, run)
                timers[key] = timer
                timer.start()
        return debounced
    return wrapper

Because stdout overriding is not thread safe, sys.stout may not be correctly restored and some errors are actually reported in the real/original/system stdout instead of the StringIO one.

randomstuff avatar Jan 02 '19 13:01 randomstuff

One workaround might be to use a mutex in order to prevent concurrent mypy linting. Ultimately, sys.stdout and sys.stderr overriding could create some other issues and this should be fixed in mypy API instead.

randomstuff avatar Jan 02 '19 13:01 randomstuff

This will most likely be resolved in python/mypy#6125. As I said in python/mypy/pull/6129 if people are having trouble with this, in the meantime, a quick duct-tape solution could be to replace the call of mypy.api.run with something like this:

res = subprocess.run(
    ["python", "-m", "mypy", *args],
    stdout=subprocess.PIPE, stderr=subprocess.PIPE
    )
report = res.stdout
errors = res.stderr

elkhadiy avatar Jan 03 '19 13:01 elkhadiy

@randomstuff amazing tenacity narrowing own the cause of this issue!

Let me know if we need an alternative to the pull request solution.

tomv564 avatar Jan 03 '19 21:01 tomv564

@elkhadiy started working on a PR on mypy. (@elkhadiy: Tell me if you want some help on the PR.)

In the meanwhile, I used the workaround of executing the body of _run() while holding a mutex:

_lock = Lock()

def _run(f: Callable[[], None]) -> Tuple[str, str, int]:
    with _lock:
        old_stdout = sys.stdout
        new_stdout = StringIO()
        sys.stdout = new_stdout
    
        old_stderr = sys.stderr
        new_stderr = StringIO()
        sys.stderr = new_stderr
    
        try:
            f()
            exit_status = 0
        except SystemExit as system_exit:
            exit_status = system_exit.code
        finally:
            sys.stdout = old_stdout
            sys.stderr = old_stderr
    
        return new_stdout.getvalue(), new_stderr.getvalue(), exit_status

It's working alright but I don't think it's worth merging this hack as the correct fix (in mypy) should be comparatively simple.

randomstuff avatar Jan 15 '19 22:01 randomstuff

upgrading mypy to the latest version now solves the issue

edit: no it does not

remorses avatar Feb 07 '19 10:02 remorses

@remorses, mypy's bug is not marked as fixed and at first glance I don't see any change in the latest release in this regard.

randomstuff avatar Feb 07 '19 10:02 randomstuff

This issue appeared when I activated dmypy in the configuration file:

Here is my pylsp-mypy.cfg:

{
    "enabled": True,
    "live_mode": False,
    "dmypy": True,
    "strict": False
}

When I set dmypy to False, the error disappears

hrouault avatar Mar 09 '22 10:03 hrouault