ycmd icon indicating copy to clipboard operation
ycmd copied to clipboard

Invalid protocol data crashes ycmd

Open sc68cal opened this issue 2 years ago • 6 comments

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 avatar Jan 10 '23 16:01 sc68cal

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

puremourning avatar Jan 10 '23 17:01 puremourning

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.

sc68cal avatar Jan 10 '23 17:01 sc68cal

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()


puremourning avatar Jan 10 '23 17:01 puremourning

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.

sc68cal avatar Jan 10 '23 17:01 sc68cal

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

puremourning avatar Jan 10 '23 17:01 puremourning

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

sc68cal avatar Jan 10 '23 17:01 sc68cal