Auto merge of #2453 - micbou:handle-server-exception, r=puremourning

[READY] Catch and log all server exceptions

A lot of different errors may occur when sending a request to ycmd and receiving its response:
 - [any exception from the requests module](http://docs.python-requests.org/en/master/_modules/requests/exceptions/);
 - exceptions from ycmd: `ServerError` and `UnknownExtraConf`;
 - `RuntimeError` exception from invalid HMAC;
 - and possibly others.

Thanks to PR #2430, we can now catch and log these exceptions.

I am marking this PR as WIP because I'd like to add tests but I am not sure on how to implement them. Should we use actual code for these tests or mock the response from the ycmd server?

Fixes #2216.
Fixes #2272.

<!-- 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/2453)
<!-- Reviewable:end -->
This commit is contained in:
Homu 2016-12-23 09:20:22 +09:00
commit e1e7e648d4
14 changed files with 318 additions and 93 deletions

View File

@ -23,6 +23,8 @@ from future import standard_library
standard_library.install_aliases()
from builtins import * # noqa
import contextlib
import logging
import requests
import urllib.parse
import json
@ -41,6 +43,7 @@ _EXECUTOR = UnsafeThreadPoolExecutor( max_workers = 30 )
# Setting this to None seems to screw up the Requests/urllib3 libs.
_DEFAULT_TIMEOUT_SEC = 30
_HMAC_HEADER = 'x-ycm-hmac'
_logger = logging.getLogger( __name__ )
class BaseRequest( object ):
@ -193,7 +196,48 @@ def JsonFromFuture( future ):
return None
def HandleServerException( exception, truncate = False ):
@contextlib.contextmanager
def HandleServerException( display = True, truncate = False ):
"""Catch any exception raised through server communication. If it is raised
because of a unknown .ycm_extra_conf.py file, load the file or ignore it after
asking the user. Otherwise, log the exception and display its message to the
user on the Vim status line. Unset the |display| parameter to hide the message
from the user. Set the |truncate| parameter to avoid hit-enter prompts from
this message.
The GetDataFromHandler, PostDataToHandler, and JsonFromFuture functions should
always be wrapped by this function to avoid Python exceptions bubbling up to
the user.
Example usage:
with HandleServerException():
response = BaseRequest.PostDataToHandler( ... )
"""
try:
yield
except UnknownExtraConf as e:
if vimsupport.Confirm( str( e ) ):
_LoadExtraConfFile( e.extra_conf_file )
else:
_IgnoreExtraConfFile( e.extra_conf_file )
except Exception as e:
_logger.exception( 'Error while handling server response' )
if display:
DisplayServerException( e, truncate )
def _LoadExtraConfFile( filepath ):
BaseRequest.PostDataToHandler( { 'filepath': filepath },
'load_extra_conf_file' )
def _IgnoreExtraConfFile( filepath ):
BaseRequest.PostDataToHandler( { 'filepath': filepath },
'ignore_extra_conf_file' )
def DisplayServerException( exception, truncate = False ):
serialized_exception = str( exception )
# We ignore the exception about the file already being parsed since it comes

View File

@ -23,9 +23,6 @@ from future import standard_library
standard_library.install_aliases()
from builtins import * # noqa
from requests.exceptions import ReadTimeout
from ycmd.responses import ServerError
from ycm.client.base_request import ( BaseRequest, BuildRequestData,
HandleServerException )
from ycm import vimsupport
@ -53,11 +50,9 @@ class CommandRequest( BaseRequest ):
'completer_target': self._completer_target,
'command_arguments': self._arguments
} )
try:
with HandleServerException():
self._response = self.PostDataToHandler( request_data,
'run_completer_command' )
except ( ServerError, ReadTimeout ) as e:
HandleServerException( e )
def Response( self ):

View File

@ -23,11 +23,8 @@ from future import standard_library
standard_library.install_aliases()
from builtins import * # noqa
from requests.exceptions import ReadTimeout
from ycm.client.base_request import ( BaseRequest, BuildRequestData,
HandleServerException )
from ycmd.responses import ServerError
class CompleterAvailableRequest( BaseRequest ):
@ -40,11 +37,9 @@ class CompleterAvailableRequest( BaseRequest ):
def Start( self ):
request_data = BuildRequestData()
request_data.update( { 'filetypes': self.filetypes } )
try:
with HandleServerException():
self._response = self.PostDataToHandler( request_data,
'semantic_completion_available' )
except ( ServerError, ReadTimeout ) as e:
HandleServerException( e )
def Response( self ):

View File

@ -23,13 +23,10 @@ from future import standard_library
standard_library.install_aliases()
from builtins import * # noqa
from requests.exceptions import ReadTimeout
from ycmd.utils import ToUnicode
from ycm.client.base_request import ( BaseRequest, JsonFromFuture,
HandleServerException,
MakeServerException )
from ycmd.responses import ServerError
TIMEOUT_SECONDS = 0.5
@ -53,16 +50,15 @@ class CompletionRequest( BaseRequest ):
def RawResponse( self ):
if not self._response_future:
return []
try:
with HandleServerException( truncate = True ):
response = JsonFromFuture( self._response_future )
errors = response[ 'errors' ] if 'errors' in response else []
for e in errors:
HandleServerException( MakeServerException( e ) )
with HandleServerException( truncate = True ):
raise MakeServerException( e )
return JsonFromFuture( self._response_future )[ 'completions' ]
except ( ServerError, ReadTimeout ) as e:
HandleServerException( e, truncate = True )
return response[ 'completions' ]
return []

View File

@ -0,0 +1,52 @@
# Copyright (C) 2016 YouCompleteMe contributors
#
# This file is part of YouCompleteMe.
#
# YouCompleteMe is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# YouCompleteMe is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with YouCompleteMe. If not, see <http://www.gnu.org/licenses/>.
from __future__ import unicode_literals
from __future__ import print_function
from __future__ import division
from __future__ import absolute_import
from future import standard_library
standard_library.install_aliases()
from builtins import * # noqa
from ycm.client.base_request import ( BaseRequest, BuildRequestData,
HandleServerException )
class DebugInfoRequest( BaseRequest ):
def __init__( self ):
super( DebugInfoRequest, self ).__init__()
self._response = None
def Start( self ):
request_data = BuildRequestData()
with HandleServerException( display = False ):
self._response = self.PostDataToHandler( request_data, 'debug_info' )
def Response( self ):
if not self._response:
return 'Server errored, no debug info from server'
return self._response
def SendDebugInfoRequest():
request = DebugInfoRequest()
# This is a blocking call.
request.Start()
return request.Response()

View File

@ -23,10 +23,6 @@ from future import standard_library
standard_library.install_aliases()
from builtins import * # noqa
from requests.exceptions import ReadTimeout
from ycm import vimsupport
from ycmd.responses import UnknownExtraConf, ServerError
from ycm.client.base_request import ( BaseRequest, BuildRequestData,
JsonFromFuture, HandleServerException )
@ -61,16 +57,8 @@ class EventNotification( BaseRequest ):
if not self._response_future or self._event_name != 'FileReadyToParse':
return []
try:
try:
with HandleServerException( truncate = True ):
self._cached_response = JsonFromFuture( self._response_future )
except UnknownExtraConf as e:
if vimsupport.Confirm( str( e ) ):
_LoadExtraConfFile( e.extra_conf_file )
else:
_IgnoreExtraConfFile( e.extra_conf_file )
except ( ServerError, ReadTimeout ) as e:
HandleServerException( e )
return self._cached_response if self._cached_response else []
@ -80,13 +68,3 @@ def SendEventNotificationAsync( event_name,
extra_data = None ):
event = EventNotification( event_name, filepath, extra_data )
event.Start()
def _LoadExtraConfFile( filepath ):
BaseRequest.PostDataToHandler( { 'filepath': filepath },
'load_extra_conf_file' )
def _IgnoreExtraConfFile( filepath ):
BaseRequest.PostDataToHandler( { 'filepath': filepath },
'ignore_extra_conf_file' )

View File

@ -23,9 +23,7 @@ from future import standard_library
standard_library.install_aliases()
from builtins import * # noqa
from requests.exceptions import ReadTimeout
from ycm.client.base_request import BaseRequest
from ycm.client.base_request import BaseRequest, HandleServerException
TIMEOUT_SECONDS = 0.1
@ -36,12 +34,8 @@ class ShutdownRequest( BaseRequest ):
def Start( self ):
try:
self.PostDataToHandler( {},
'shutdown',
TIMEOUT_SECONDS )
except ReadTimeout:
pass
with HandleServerException( display = False ):
self.PostDataToHandler( {}, 'shutdown', TIMEOUT_SECONDS )
def SendShutdownRequest():

View File

@ -25,7 +25,7 @@ from builtins import * # noqa
import time
from threading import Thread
from ycm.client.base_request import BaseRequest
from ycm.client.base_request import BaseRequest, HandleServerException
# This class can be used to keep the ycmd server alive for the duration of the
@ -46,9 +46,5 @@ class YcmdKeepalive( object ):
while True:
time.sleep( self._ping_interval_seconds )
# We don't care if there's an intermittent problem in contacting the
# server; it's fine to just skip this ping.
try:
with HandleServerException( display = False ):
BaseRequest.GetDataFromHandler( 'healthy' )
except:
pass

View File

@ -26,7 +26,6 @@ from builtins import * # noqa
import vim
from ycm import vimsupport
from ycmd import utils
from ycmd.responses import ServerError
from ycmd.completers.completer import Completer
from ycm.client.base_request import BaseRequest, HandleServerException
@ -115,9 +114,7 @@ class OmniCompleter( Completer ):
'query': query
}
try:
with HandleServerException():
return BaseRequest.PostDataToHandler( request_data,
'filter_and_sort_candidates' )
except ServerError as e:
HandleServerException( e )
return candidates

View File

@ -0,0 +1,44 @@
# Copyright (C) 2016 YouCompleteMe contributors
#
# This file is part of YouCompleteMe.
#
# YouCompleteMe is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# YouCompleteMe is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with YouCompleteMe. If not, see <http://www.gnu.org/licenses/>.
from __future__ import unicode_literals
from __future__ import print_function
from __future__ import division
from __future__ import absolute_import
from future import standard_library
standard_library.install_aliases()
from builtins import * # noqa
from ycm.tests.test_utils import ( MockVimModule, MockVimBuffers, VimBuffer )
MockVimModule()
from hamcrest import assert_that, equal_to
from mock import patch
from ycm.tests import YouCompleteMeInstance
@YouCompleteMeInstance()
def SendCommandRequest_test( ycm ):
current_buffer = VimBuffer( 'buffer' )
with MockVimBuffers( [ current_buffer ], current_buffer ):
with patch( 'ycm.client.base_request.JsonFromFuture',
return_value = 'Some response' ):
assert_that(
ycm.SendCommandRequest( 'GoTo', 'python' ),
equal_to( 'Some response' )
)

View File

@ -25,13 +25,15 @@ from future import standard_library
standard_library.install_aliases()
from builtins import * # noqa
from ycm.tests.test_utils import ( CurrentWorkingDirectory, MockVimModule,
MockVimBuffers, VimBuffer )
from ycm.tests.test_utils import ( CurrentWorkingDirectory, ExtendedMock,
MockVimModule, MockVimBuffers, VimBuffer )
MockVimModule()
from hamcrest import assert_that, empty, has_entries
from hamcrest import assert_that, contains, empty, has_entries
from mock import call, patch
from ycm.tests import PathToTestFile, YouCompleteMeInstance
from ycmd.responses import ServerError
@YouCompleteMeInstance()
@ -40,7 +42,7 @@ def CreateCompletionRequest_UnicodeWorkingDirectory_test( ycm ):
current_buffer = VimBuffer( PathToTestFile( 'uni¢𐍈d€', 'current_buffer' ) )
with CurrentWorkingDirectory( unicode_dir ):
with MockVimBuffers( [ current_buffer ], current_buffer, ( 5, 2 ) ):
with MockVimBuffers( [ current_buffer ], current_buffer ):
ycm.CreateCompletionRequest(),
results = ycm.GetCompletions()
@ -52,3 +54,86 @@ def CreateCompletionRequest_UnicodeWorkingDirectory_test( ycm ):
'refresh': 'always'
} )
)
@YouCompleteMeInstance()
@patch( 'ycm.client.base_request._logger', autospec = True )
@patch( 'ycm.vimsupport.PostVimMessage', new_callable = ExtendedMock )
def CreateCompletionRequest_ResponseContainingError_test( ycm,
post_vim_message,
logger ):
current_buffer = VimBuffer( 'buffer' )
with MockVimBuffers( [ current_buffer ], current_buffer ):
ycm.CreateCompletionRequest(),
response = {
'completions': [ {
'insertion_text': 'insertion_text',
'menu_text': 'menu_text',
'extra_menu_info': 'extra_menu_info',
'detailed_info': 'detailed_info',
'kind': 'kind',
'extra_data': {
'doc_string': 'doc_string'
}
} ],
'completion_start_column': 3,
'errors': [ {
'exception': {
'TYPE': 'Exception'
},
'message': 'message',
'traceback': 'traceback'
} ]
}
with patch( 'ycm.client.completion_request.JsonFromFuture',
return_value = response ):
results = ycm.GetCompletions()
logger.exception.assert_called_with( 'Error while handling server response' )
post_vim_message.assert_has_exact_calls( [
call( 'Exception: message', truncate = True )
] )
assert_that(
results,
has_entries( {
'words': contains( has_entries( {
'word': 'insertion_text',
'abbr': 'menu_text',
'menu': 'extra_menu_info',
'info': 'detailed_info\ndoc_string',
'kind': 'k',
'dup': 1,
'empty': 1
} ) ),
'refresh': 'always'
} )
)
@YouCompleteMeInstance()
@patch( 'ycm.client.base_request._logger', autospec = True )
@patch( 'ycm.vimsupport.PostVimMessage', new_callable = ExtendedMock )
def CreateCompletionRequest_ErrorFromServer_test( ycm,
post_vim_message,
logger ):
current_buffer = VimBuffer( 'buffer' )
with MockVimBuffers( [ current_buffer ], current_buffer ):
ycm.CreateCompletionRequest(),
with patch( 'ycm.client.completion_request.JsonFromFuture',
side_effect = ServerError( 'Server error' ) ):
results = ycm.GetCompletions()
logger.exception.assert_called_with( 'Error while handling server response' )
post_vim_message.assert_has_exact_calls( [
call( 'Server error', truncate = True )
] )
assert_that(
results,
has_entries( {
'words': empty(),
'refresh': 'always'
} )
)

View File

@ -128,13 +128,13 @@ def EventNotification_FileReadyToParse_NonDiagnostic_Error_test(
# The first call raises a warning
post_vim_message.assert_has_exact_calls( [
call( ERROR_TEXT, truncate = False )
call( ERROR_TEXT, truncate = True )
] )
# Subsequent calls don't re-raise the warning
ycm.HandleFileParseRequest()
post_vim_message.assert_has_exact_calls( [
call( ERROR_TEXT, truncate = False )
call( ERROR_TEXT, truncate = True )
] )
# But it does if a subsequent event raises again
@ -142,8 +142,8 @@ def EventNotification_FileReadyToParse_NonDiagnostic_Error_test(
ok_( ycm.FileParseRequestReady() )
ycm.HandleFileParseRequest()
post_vim_message.assert_has_exact_calls( [
call( ERROR_TEXT, truncate = False ),
call( ERROR_TEXT, truncate = False )
call( ERROR_TEXT, truncate = True ),
call( ERROR_TEXT, truncate = True )
] )
@ -159,9 +159,9 @@ def EventNotification_FileReadyToParse_NonDiagnostic_Error_NonNative_test(
vim_command.assert_not_called()
@patch( 'ycm.client.event_notification._LoadExtraConfFile',
@patch( 'ycm.client.base_request._LoadExtraConfFile',
new_callable = ExtendedMock )
@patch( 'ycm.client.event_notification._IgnoreExtraConfFile',
@patch( 'ycm.client.base_request._IgnoreExtraConfFile',
new_callable = ExtendedMock )
@YouCompleteMeInstance()
def EventNotification_FileReadyToParse_NonDiagnostic_ConfirmExtraConf_test(

View File

@ -30,10 +30,12 @@ MockVimModule()
import os
import sys
from hamcrest import assert_that, is_in, is_not, has_length, matches_regexp
from hamcrest import ( assert_that, contains, empty, is_in, is_not, has_length,
matches_regexp )
from mock import call, MagicMock, patch
from ycm.tests import YouCompleteMeInstance
from ycmd.responses import ServerError
@YouCompleteMeInstance()
@ -234,3 +236,54 @@ def YouCompleteMe_ToggleLogs_WithoutParameters_test( ycm, post_vim_message ):
'ycmd_\d+_stderr_.+.log\n'
'ycmd_\d+_stdout_.+.log' )
)
@YouCompleteMeInstance()
def YouCompleteMe_GetDefinedSubcommands_ListFromServer_test( ycm ):
current_buffer = VimBuffer( 'buffer' )
with MockVimBuffers( [ current_buffer ], current_buffer ):
with patch( 'ycm.client.base_request.JsonFromFuture',
return_value = [ 'SomeCommand', 'AnotherCommand' ] ):
assert_that(
ycm.GetDefinedSubcommands(),
contains(
'SomeCommand',
'AnotherCommand'
)
)
@YouCompleteMeInstance()
@patch( 'ycm.client.base_request._logger', autospec = True )
@patch( 'ycm.vimsupport.PostVimMessage', new_callable = ExtendedMock )
def YouCompleteMe_GetDefinedSubcommands_ErrorFromServer_test( ycm,
post_vim_message,
logger ):
current_buffer = VimBuffer( 'buffer' )
with MockVimBuffers( [ current_buffer ], current_buffer ):
with patch( 'ycm.client.base_request.JsonFromFuture',
side_effect = ServerError( 'Server error' ) ):
result = ycm.GetDefinedSubcommands()
logger.exception.assert_called_with( 'Error while handling server response' )
post_vim_message.assert_has_exact_calls( [
call( 'Server error', truncate = False )
] )
assert_that( result, empty() )
@YouCompleteMeInstance()
@patch( 'ycm.vimsupport.PostVimMessage', new_callable = ExtendedMock )
def YouCompleteMe_ShowDetailedDiagnostic_MessageFromServer_test(
ycm, post_vim_message ):
current_buffer = VimBuffer( 'buffer' )
with MockVimBuffers( [ current_buffer ], current_buffer ):
with patch( 'ycm.client.base_request.JsonFromFuture',
return_value = { 'message': 'some_detailed_diagnostic' } ):
ycm.ShowDetailedDiagnostic(),
post_vim_message.assert_has_exact_calls( [
call( 'some_detailed_diagnostic', warning = False )
] )

View File

@ -38,16 +38,17 @@ from ycm import base, paths, vimsupport
from ycmd import utils
from ycmd import server_utils
from ycmd.request_wrap import RequestWrap
from ycmd.responses import ServerError
from ycm.diagnostic_interface import DiagnosticInterface
from ycm.omni_completer import OmniCompleter
from ycm import syntax_parse
from ycm.client.ycmd_keepalive import YcmdKeepalive
from ycm.client.base_request import BaseRequest, BuildRequestData
from ycm.client.base_request import ( BaseRequest, BuildRequestData,
HandleServerException )
from ycm.client.completer_available_request import SendCompleterAvailableRequest
from ycm.client.command_request import SendCommandRequest
from ycm.client.completion_request import ( CompletionRequest,
ConvertCompletionDataToVimData )
from ycm.client.debug_info_request import SendDebugInfoRequest
from ycm.client.omni_completion_request import OmniCompletionRequest
from ycm.client.event_notification import ( SendEventNotificationAsync,
EventNotification )
@ -302,12 +303,9 @@ class YouCompleteMe( object ):
def GetDefinedSubcommands( self ):
if self.IsServerAlive():
try:
with HandleServerException():
return BaseRequest.PostDataToHandler( BuildRequestData(),
'defined_subcommands' )
except ServerError:
return []
else:
return []
@ -636,14 +634,13 @@ class YouCompleteMe( object ):
def ShowDetailedDiagnostic( self ):
if not self.IsServerAlive():
return
try:
debug_info = BaseRequest.PostDataToHandler( BuildRequestData(),
'detailed_diagnostic' )
if 'message' in debug_info:
vimsupport.PostVimMessage( debug_info[ 'message' ],
with HandleServerException():
detailed_diagnostic = BaseRequest.PostDataToHandler(
BuildRequestData(), 'detailed_diagnostic' )
if 'message' in detailed_diagnostic:
vimsupport.PostVimMessage( detailed_diagnostic[ 'message' ],
warning = False )
except ServerError as e:
vimsupport.PostVimMessage( str( e ) )
def DebugInfo( self ):
@ -651,8 +648,7 @@ class YouCompleteMe( object ):
if self._client_logfile:
debug_info += 'Client logfile: {0}\n'.format( self._client_logfile )
if self.IsServerAlive():
debug_info += BaseRequest.PostDataToHandler( BuildRequestData(),
'debug_info' )
debug_info += SendDebugInfoRequest()
else:
debug_info += 'Server crashed, no debug info from server'
debug_info += '\nServer running at: {0}\n'.format(