Auto merge of #2750 - micbou:no-stderr-read, r=Valloric

[READY] Do not read server standard error

When the ycmd server crashes, we read its standard error and log the result. Unfortunately, this blocks Vim if a sub-server is still running after the crash (e.g. JediHTTP). In that case, the only way to unblock Vim is to manually kill the sub-server.

We could fix the issue by writing some (non-trivial) code that reads ycmd `stderr` without blocking but instead we simply don't read it at all. The drawback is small: [ycmd immediately redirects its `stdout` and `stderr` to logfiles at startup](ff255aed4a/ycmd/__main__.py (L150-L153)). In other words, reading its `stderr` is only useful if ycmd crashed before the redirection. This is something that should never happen.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/2750)
<!-- Reviewable:end -->
This commit is contained in:
zzbot 2017-08-18 04:41:58 -07:00 committed by GitHub
commit 9a76fd2e9a
2 changed files with 1 additions and 16 deletions

View File

@ -50,7 +50,6 @@ def RunNotifyUserIfServerCrashed( ycm, test, post_vim_message ):
ycm._logger = MagicMock( autospec = True )
ycm._server_popen = MagicMock( autospec = True )
ycm._server_popen.poll.return_value = test[ 'return_code' ]
ycm._server_popen.stderr.read.return_value = test[ 'stderr_output' ]
ycm._NotifyUserIfServerCrashed()
@ -69,7 +68,6 @@ def YouCompleteMe_NotifyUserIfServerCrashed_UnexpectedCore_test():
"Use the ':YcmToggleLogs' command to check the logs." )
RunNotifyUserIfServerCrashed( {
'return_code': 3,
'stderr_output' : '',
'expected_logs': [ message ],
'expected_vim_message': message
} )
@ -81,7 +79,6 @@ def YouCompleteMe_NotifyUserIfServerCrashed_MissingCore_test():
"using it. Follow the instructions in the documentation." )
RunNotifyUserIfServerCrashed( {
'return_code': 4,
'stderr_output': '',
'expected_logs': [ message ],
'expected_vim_message': message
} )
@ -94,7 +91,6 @@ def YouCompleteMe_NotifyUserIfServerCrashed_Python2Core_test():
"interpreter path." )
RunNotifyUserIfServerCrashed( {
'return_code': 5,
'stderr_output': '',
'expected_logs': [ message ],
'expected_vim_message': message
} )
@ -107,7 +103,6 @@ def YouCompleteMe_NotifyUserIfServerCrashed_Python3Core_test():
"interpreter path." )
RunNotifyUserIfServerCrashed( {
'return_code': 6,
'stderr_output': '',
'expected_logs': [ message ],
'expected_vim_message': message
} )
@ -119,7 +114,6 @@ def YouCompleteMe_NotifyUserIfServerCrashed_OutdatedCore_test():
"install.py script. See the documentation for more details." )
RunNotifyUserIfServerCrashed( {
'return_code': 7,
'stderr_output': '',
'expected_logs': [ message ],
'expected_vim_message': message
} )
@ -131,11 +125,7 @@ def YouCompleteMe_NotifyUserIfServerCrashed_UnexpectedExitCode_test():
"check the logs." )
RunNotifyUserIfServerCrashed( {
'return_code': 1,
'stderr_output': 'First line\r\n'
'Second line',
'expected_logs': [ 'First line\n'
'Second line',
message ],
'expected_logs': [ message ],
'expected_vim_message': message
} )

View File

@ -251,11 +251,6 @@ class YouCompleteMe( object ):
else:
error_message = EXIT_CODE_UNEXPECTED_MESSAGE.format( code = return_code )
server_stderr = '\n'.join(
utils.ToUnicode( self._server_popen.stderr.read() ).splitlines() )
if server_stderr:
self._logger.error( server_stderr )
error_message = SERVER_SHUTDOWN_MESSAGE + ' ' + error_message
self._logger.error( error_message )
vimsupport.PostVimMessage( error_message )