search

python-lsp / python-lsp-server

Fork of the python-language-server project, maintained by the Spyder IDE team and the community
MIT License
1.76k stars 186 forks source link

Use invalid request handler rather than raising key error for requests after shutdown #432

Closed smacke closed 10 months ago

smacke commented 10 months ago

This is to fix https://github.com/python-lsp/python-lsp-server/issues/314 (for dealing with requests that come in after shutdown, e.g. from neovim). It requires https://github.com/python-lsp/python-lsp-jsonrpc/pull/20 to work properly, which probably means it shouldn't merge without first bumping the min required version of python-lsp-jsonrpc.

Fixes #314.

ccordoba12 commented 10 months ago

It requires https://github.com/python-lsp/python-lsp-jsonrpc/pull/20 to work properly, which probably means it shouldn't merge without first bumping the min required version of python-lsp-jsonrpc.

I just released 1.1.0 of python-lsp-jsonrpc, which includes that PR. So, please bump our requirement to that package here to be >=1.1.0,<2.0.0.

ccordoba12 commented 10 months ago

One last thing: can we use the improvements done here to fix #114 too?

smacke commented 10 months ago

One last thing: can we use the improvements done here to fix https://github.com/python-lsp/python-lsp-server/issues/114 too?

Hmm, seems like it may be a separate issue around clients failing to throttle or debounce signature requests. I'm guessing either pylsp gets overwhelmed and starts dropping requests, which is why it can't find the future for cancellation, or it manages to successfully respond before the cancellation request comes in, but all the requests still degrade performance significantly. Flooding the logs seems like a secondary issue that could be fixed just by adjusting the log level of the warning message down to info or debug.

I suspect the way to fix this would be to implement some kind of server-side throttling to handle misbehaving clients. I use neovim; I'll check later if I can repro there.

ccordoba12 commented 10 months ago

Hmm, seems like it may be a separate issue around clients failing to throttle or debounce signature requests

No problem. I thought we could capture the cancel notifications for unknown messages, like you did with the requests after exit, and pass on them to avoid that error.

Flooding the logs seems like a secondary issue that could be fixed just by adjusting the log level of the warning message down to info or debug.

Ok, that's a good idea.

I suspect the way to fix this would be to implement some kind of server-side throttling to handle misbehaving clients. I use neovim; I'll check later if I can repro there.

Thanks @smacke!