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

Daemon dmypy crashes with disable_error_code configuration option #14671

Closed
LukeSavefrogs opened this issue Feb 10, 2023 · 15 comments · Fixed by #15218
Closed

Daemon dmypy crashes with disable_error_code configuration option #14671

LukeSavefrogs opened this issue Feb 10, 2023 · 15 comments · Fixed by #15218
Assignees

Comments

@LukeSavefrogs
Copy link

Description

Evertything was working until i tried to disable some error codes using the disable_error_code = 'no-untyped-def,empty-body' option in the pyproject.toml file.

After restarting the daemon, it crashed with the error Timed out waiting for daemon to start

Log

Mypy extension activated, version 0.2.3
Registering listener for interpreter changed event
Listener registered
[1] Check folder: c:\Users\lucas\Desktop\myFolder
Activation complete
[1] Received python path from Python extension: C:\Python39\python.exe
[1] Running dmypy in folder c:\Users\lucas\Desktop\myFolder
'C:\Python39\Scripts\dmypy.EXE' --status-file 'c:\Users\lucas\AppData\Roaming\Code\User\workspaceStorage\41737679a5dd5649bc304b3214d8b429\matangover.mypy\dmypy-6e071d9aa88ca0124b79d66d19a862665ea10e28-14032.json' run --log-file 'c:\Users\lucas\AppData\Roaming\Code\User\workspaceStorage\41737679a5dd5649bc304b3214d8b429\matangover.mypy\dmypy-6e071d9aa88ca0124b79d66d19a862665ea10e28.log' -- . --show-column-numbers --no-error-summary --no-pretty --no-color-output --python-executable 'C:\Python39\python.exe'
[1] stderr:
Timed out waiting for daemon to start

[1] Error running mypy in c:\Users\lucas\Desktop\myFolder: mypy failed with error: "Timed out waiting for daemon to start
". See Output panel for details.

To Reproduce

Use the disable_error_code configuration option either in the pyproject.toml or in the mypy.ini files.

Your Environment

  • Mypy version used: mypy 1.0.0 (compiled: yes)
  • Mypy command-line flags: I used the vscode extension Mypy
  • Mypy configuration options from mypy.ini (and other config files):
    # Inside `pyproject.toml`...
    [tool.mypy]
    strict = true
    warn_no_return = false
    warn_return_any = false
    disallow_untyped_defs = false
    warn_incomplete_stub = false
    
    disable_error_code = 'no-untyped-def,empty-body'
  • Python version used: Python 3.9.6
  • Operating system and version: Windows 10 (version 10.0.19044 Build 19044)
@ilevkivskyi
Copy link
Member

What happens when you run mypy with the same config in normal (non-daemon) mode? I.e. if you use mypy instead of dmypy (note you don't need --status-file, --log-file etc in non-daemon mode). This issue, as well as some other issues you linked seem Windows-specific, and I don't have a Windows machine to reproduce this.

@LukeSavefrogs
Copy link
Author

LukeSavefrogs commented Mar 19, 2023

From a first look at it, it seems like using mypy works as expected. I already used mypy with CLI flags and it worked, but i loved the fact that it was integrated in the IDE.

mypy .\src\
[...]
Found 1 error in 1 file (checked 10 source files)

Same pyproject.toml as before.

@ilevkivskyi
Copy link
Member

Hm, did you check it actually picked up the config file when you use mypy? (i.e. do you have the errors there that are actually ignored?)

@LukeSavefrogs
Copy link
Author

LukeSavefrogs commented Mar 20, 2023

Hm, did you check it actually picked up the config file when you use mypy? (i.e. do you have the errors there that are actually ignored?)

Yes, i confirm that removing the disable_error_code = 'no-untyped-def,empty-body' line the errors show up, so the configuration file seem to be picked up.

In fact, i added other codes as well to further customize the linting task.

@JukkaL JukkaL self-assigned this Apr 11, 2023
@svalentin
Copy link
Collaborator

svalentin commented May 9, 2023

Interesting...
Installing the current version in pypi (1.2.0) via python -m pip install -U mypy reproduces this issue.
Installing from zipball works however! Tried the following:
master (1.4.0+dev) via python -m pip install -U https://github.com/python/mypy/zipball/master seems to work.
release-1.2 (1.2.0) via python -m pip install -U https://github.com/python/mypy/zipball/release-1.2 seems to work.
release-1.1 (1.1.0) via python -m pip install -U https://github.com/python/mypy/zipball/release-1.1 seems to work.
(Stopped existing daemons, and uninstalled mypy between each step. Also tried reinstalling pypi mypy and the issue happens again)

pypi wheel it's using: mypy-1.2.0-cp311-cp311-win_amd64.whl

@svalentin
Copy link
Collaborator

svalentin commented May 9, 2023

The only significant difference between zipball release-1.2 and pypi 1.2 is the *.pyc and *.pyd files. Removing those files from sites_packages makes dmypy work!

@svalentin
Copy link
Collaborator

More findings:

  • Starting the daemon in the foreground works: python -m mypy.dmypy daemon.
  • The only problematic option is the disable_error_code in the toml. Anything else there can be removed and the bug still reproduces. Any valid error code causes issues there.
  • I think the problem is with the client using pickle to pass the start options to the daemon.
  • Printing the options in the client before pickle prints (among all the other options): 'disabled_error_codes': {<mypy.errorcodes.ErrorCode object at 0x000001F1F6472700>},

@svalentin svalentin assigned svalentin and unassigned JukkaL May 9, 2023
@ilevkivskyi
Copy link
Member

Yes, this must be it. We had a similar crash in regular (non-daemon) incremental mode, when it was trying to compute hash of options. That one I fixed by switching codes to plain strings. Something similar should work here. Thanks for investigating this!

@ilevkivskyi
Copy link
Member

(This is the I meant PR https://github.com/python/mypy/pull/13523/files)

@svalentin
Copy link
Collaborator

Managed to produce the options data and pass it to the daemon in the foreground. This is the error:

TypeError: __init__() missing required argument 'code' (pos 1)

Looks like it's not properly passing in the ErrorCode.

(I just now saw your comment as well, looking into the PR)

@ilevkivskyi
Copy link
Member

ilevkivskyi commented May 9, 2023

Yeah, making pickle work with this may be tricky (especially with sub_code_of). It will be easier to turn them into strings, and on the way back (when loading from args.options_data in dmypy/client.py) use mypy.errorcodes.error_codes to convert strings back to ErrorCode objects.

@svalentin
Copy link
Collaborator

I think that's the easiest way. I still don't understand why this works with interpreted python, but not with compiled python, on Windows. (works with pyd files removed, doesn't work with them) The options.snapshot() is probably not working correctly. (but still playing with it)

@svalentin
Copy link
Collaborator

Actually, I think it might be as simple as mypyc not being able to pickle the ErrorCode class - https://mypyc.readthedocs.io/en/latest/differences_from_python.html#pickling-and-copying-objects
And we just need a decorator.

Will play around with mypyc and see if I can compile it on windows and make sure that works.

JukkaL pushed a commit that referenced this issue May 11, 2023
When starting mypy daemon, we pickle options. Error codes are part of
options and need to be pickle-able. This decorator is needed for this to
be possible with mypyc.

Fixes #14671

---------

Co-authored-by: Valentin Stanciu <250871+svalentin@users.noreply.github>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
@LukeSavefrogs
Copy link
Author

Thank you all for the attention and effort!

Has this change been release to the public or will be included in future releases?

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

Successfully merging a pull request may close this issue.

5 participants