Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
randomstuff opened this issue Jan 2, 2019 · 7 comments

Comments

@randomstuff
Copy link

randomstuff commented Jan 2, 2019

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
Copy link
Author

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 randomstuff changed the title Some mypy errors are not capture and end up directly in the process stdout Some mypy errors are not captured and end up directly in the process stdout Jan 2, 2019
@randomstuff randomstuff changed the title Some mypy errors are not captured and end up directly in the process stdout mypy errors are not captured and end up directly in the process stdout Jan 2, 2019
@elkhadiy
Copy link

elkhadiy commented Jan 3, 2019

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

@tomv564
Copy link
Owner

tomv564 commented Jan 3, 2019

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

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

@randomstuff
Copy link
Author

@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.

@remorses
Copy link

remorses commented Feb 7, 2019

upgrading mypy to the latest version now solves the issue

edit:
no it does not

@randomstuff
Copy link
Author

@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.

@hrouault
Copy link

hrouault commented Mar 9, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants