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

Invalid protocol data crashes ycmd #1673

Open
sc68cal opened this issue Jan 10, 2023 · 6 comments
Open

Invalid protocol data crashes ycmd #1673

sc68cal opened this issue Jan 10, 2023 · 6 comments

Comments

@sc68cal
Copy link

sc68cal commented Jan 10, 2023

I have configured ansible-language-server as a language server, as follows in my vimrc

let g:ycm_language_server = 
  \ [ 
  \   {
  \     'name': 'ansible',
  \     'cmdline': [ 'ansible-language-server', '--stdio' ],
  \     'filetypes': [ 'yaml' ],
  \     'project_root_files': ['ansible.cfg']
  \ ]

The problem is that YouCompleteMe/ycmd crashes due the fact that when ansible-language-server starts, it has a couple console.debug statements around which linting system it will use. ansible-language-server is built using Node.js and unfortunately only honors the --stdio flag for starting the language server. In Node.js, console.debug is just a redirect to console.log.

As a result, ansible-language-server upon startup emits a line to stdio that is not a valid protocol data, which crashes ycmd.

2023-01-10 11:21:06,067 - ERROR - Received invalid protocol data from server: b'\nValidating u
sing ansible-lint'
Traceback (most recent call last):
  File "/Users/sc68cal/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/completers/language_s
erver/language_server_completer.py", line 564, in _ReadHeaders
    key, value = utils.ToUnicode( line ).split( ':', 1 )
ValueError: not enough values to unpack (expected 2, got 1)
2023-01-10 11:21:06,067 - ERROR - The language server communication channel closed unexpectedly. Issue a RestartServer command to recover.
Traceback (most recent call last):
  File "/Users/sc68cal/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/completers/language_server/language_server_completer.py", line 367, in run
    self._ReadMessages()
  File "/Users/sc68cal/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/completers/language_server/language_server_completer.py", line 486, in _ReadMessages
    data, read_bytes, headers = self._ReadHeaders( data )
  File "/Users/sc68cal/.vim/bundle/YouCompleteMe/third_party/ycmd/ycmd/completers/language_server/language_server_completer.py", line 564, in _ReadHeaders
    key, value = utils.ToUnicode( line ).split( ':', 1 )
ValueError: not enough values to unpack (expected 2, got 1)
sc68cal added a commit to sc68cal/ycmd that referenced this issue Jan 10, 2023
The only caller to this method does handle exceptions from this method,
so instead of crashing, just log the message and continue processing

This is related to ycm-core#1673
sc68cal added a commit to sc68cal/ycmd that referenced this issue Jan 10, 2023
The only caller to this method does not handle exceptions from this method,
so instead of crashing, just log the message and continue processing

This is related to ycm-core#1673
@puremourning
Copy link
Member

I think it is better to raise this as a bug against ansible-language-server, as it's clearly invalid.

@sc68cal
Copy link
Author

sc68cal commented Jan 10, 2023

Sure, but I also think ycmd should be resilient and not crash upon receiving invalid data. The logic for handling the error is in there, but I think the problem is that the raise keyword was incorrectly used or not understood, since they went through the trouble of try/catching invalid protocol responses.

Removing the raise keyword allows ycmd to continue working, in the face of bad behaving language server implementations.

@puremourning
Copy link
Member

Sure, but I also think ycmd should be resilient and not crash upon receiving invalid data

I don't agree that ycmd is crashing. The language server connection thread is aborting due to invalid protocol data (which cannot be recovered from):

    except Exception:
      LOGGER.exception( 'The language server communication channel closed '
                        'unexpectedly. Issue a RestartServer command to '
                        'recover.' )

      # Abort any outstanding requests
      with self._response_mutex:
        for _, response in self._responses.items():
          response.Abort()
        self._responses.clear()

      # Close any remaining sockets or files
      self.Shutdown()

@sc68cal
Copy link
Author

sc68cal commented Jan 10, 2023

See but in this instance the protocol data is just that Validating using ansible-lint debug line that is getting printed to stdout because of Node.js - if we skip over that line or just not attempt to process it, the next line that gets emitted by the language server is valid and my editor starts highlighting and showing recommendations. It's not unrecoverable, at least in some instances.

@puremourning
Copy link
Member

puremourning commented Jan 10, 2023

presumably, you could also convince ansible-language-server to listen on a TCP port instead of using stdio, as it clearly doesn't work properly.

https://github.com/ycm-core/ycmd#lsp-based-completers

@sc68cal
Copy link
Author

sc68cal commented Jan 10, 2023

Yes, I attempted to change my vimrc configuration to use a port instead of stdout but it appears to not honor it. So, implementing that would be a larger exercise than I was willing to commit to at the moment

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

2 participants