Cap the amount of time spent waiting for plugin completions in LSP

Change-Id: Ic60d9936a252a1322becc233d696dfed41af834a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/150927
Commit-Queue: Danny Tuppeny <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>

Author: DanTup

pkg/analysis_server/lib/src/domain_completion.dart

@@ -111,11 +111,11 @@ class CompletionDomainHandler extends AbstractRequestHandler {
     // false) then send empty results
 
     //
-    // Add the fixes produced by plugins to the server-generated fixes.
+    // Add the completions produced by plugins to the server-generated list.
     //
     if (pluginFutures != null) {
       var responses = await waitForResponses(pluginFutures,
-          requestParameters: requestParams);
+          requestParameters: requestParams, timeout: 100);
       for (var response in responses) {
         var result =
             plugin.CompletionGetSuggestionsResult.fromResponse(response);

pkg/analysis_server/lib/src/lsp/handlers/handler_completion.dart

@@ -25,8 +25,8 @@ import 'package:analyzer/src/services/available_declarations.dart';
 import 'package:analyzer_plugin/protocol/protocol_common.dart';
 import 'package:analyzer_plugin/protocol/protocol_generated.dart' as plugin;
 
-// If the client does not provide capabilities.completion.completionItemKind.valueSet
-// then we must never send a kind that's not in this list.
+/// If the client does not provide capabilities.completion.completionItemKind.valueSet
+/// then we must never send a kind that's not in this list.
 final defaultSupportedCompletionKinds = HashSet<CompletionItemKind>.of([
   CompletionItemKind.Text,
   CompletionItemKind.Method,
@@ -163,7 +163,8 @@ class CompletionHandler
     int offset,
   ) async {
     final requestParams = plugin.CompletionGetSuggestionsParams(path, offset);
-    final pluginResponses = await requestFromPlugins(path, requestParams);
+    final pluginResponses =
+        await requestFromPlugins(path, requestParams, timeout: 100);
 
     final pluginResults = pluginResponses
         .map((e) => plugin.CompletionGetSuggestionsResult.fromResponse(e))

pkg/analysis_server/lib/src/lsp/handlers/handlers.dart

@@ -82,12 +82,17 @@ mixin Handler<P, R> {
 
 mixin LspPluginRequestHandlerMixin<T extends AbstractAnalysisServer>
     on RequestHandlerMixin<T> {
-  Future<List<Response>> requestFromPlugins(String path, RequestParams params) {
+  Future<List<Response>> requestFromPlugins(
+    String path,
+    RequestParams params, {
+    int timeout = 500,
+  }) {
     final driver = server.getAnalysisDriver(path);
     final pluginFutures = server.pluginManager
         .broadcastRequest(params, contextRoot: driver.contextRoot);
 
-    return waitForResponses(pluginFutures, requestParameters: params);
+    return waitForResponses(pluginFutures,
+        requestParameters: params, timeout: timeout);
   }
 }
 

pkg/analysis_server/test/lsp/completion_test.dart

@@ -192,6 +192,49 @@ class CompletionTest extends AbstractLspAnalysisServerTest {
     expect(suggestion.label, equals('id'));
   }
 
+  Future<void> test_fromPlugin_tooSlow() async {
+    final content = '''
+    void main() {
+      var x = '';
+      print(^);
+    }
+    ''';
+
+    final pluginResult = plugin.CompletionGetSuggestionsResult(
+      content.indexOf('^'),
+      0,
+      [
+        plugin.CompletionSuggestion(
+          plugin.CompletionSuggestionKind.INVOCATION,
+          100,
+          'x.toUpperCase()',
+          -1,
+          -1,
+          false,
+          false,
+        ),
+      ],
+    );
+    configureTestPlugin(
+      respondWith: pluginResult,
+      // Don't respond within an acceptable time
+      respondAfter: Duration(seconds: 1),
+    );
+
+    await initialize();
+    await openFile(mainFileUri, withoutMarkers(content));
+
+    final res = await getCompletion(mainFileUri, positionFromMarker(content));
+    final fromServer = res.singleWhere((c) => c.label == 'x');
+    final fromPlugin = res.singleWhere((c) => c.label == 'x.toUpperCase()',
+        orElse: () => null);
+
+    // Server results should still be included.
+    expect(fromServer.kind, equals(CompletionItemKind.Variable));
+    // Plugin results are not because they didn't arrive in time.
+    expect(fromPlugin, isNull);
+  }
+
   Future<void> test_gettersAndSetters() async {
     final content = '''
     class MyClass {

pkg/analysis_server/test/lsp/server_abstract.dart

@@ -52,13 +52,15 @@ abstract class AbstractLspAnalysisServerTest
   DiscoveredPluginInfo configureTestPlugin({
     plugin.ResponseResult respondWith,
     plugin.Notification notification,
+    Duration respondAfter = Duration.zero,
   }) {
     final info = DiscoveredPluginInfo('a', 'b', 'c', null, null);
     pluginManager.plugins.add(info);
 
     if (respondWith != null) {
       pluginManager.broadcastResults = <PluginInfo, Future<plugin.Response>>{
-        info: Future.value(respondWith.toResponse('-', 1))
+        info: Future.delayed(respondAfter)
+            .then((_) => respondWith.toResponse('-', 1))
       };
     }
 

pkg/analysis_server/test/mocks.dart

@@ -39,13 +39,33 @@ class MockLspServerChannel implements LspServerCommunicationChannel {
   /// Completer that will be signalled when the input stream is closed.
   final Completer _closed = Completer();
 
+  /// Errors popups sent to the user.
+  final shownErrors = <lsp.ShowMessageParams>[];
+
+  /// Warning popups sent to the user.
+  final shownWarnings = <lsp.ShowMessageParams>[];
+
   MockLspServerChannel(bool _printMessages) {
     if (_printMessages) {
       _serverToClient.stream
           .listen((message) => print('<== ' + jsonEncode(message)));
       _clientToServer.stream
           .listen((message) => print('==> ' + jsonEncode(message)));
     }
+
+    // Keep track of any errors/warnings that are sent to the user with
+    // `window/showMessage`.
+    _serverToClient.stream.listen((message) {
+      if (message is lsp.NotificationMessage &&
+          message.method == Method.window_showMessage &&
+          message.params is lsp.ShowMessageParams) {
+        if (message.params?.type == MessageType.Error) {
+          shownErrors.add(message.params);
+        } else if (message.params?.type == MessageType.Warning) {
+          shownWarnings.add(message.params);
+        }
+      }
+    });
   }
 
   /// Future that will be completed when the input stream is closed.
@@ -167,7 +187,8 @@ class MockLspServerChannel implements LspServerCommunicationChannel {
         (message is lsp.ResponseMessage && message.id == request.id) ||
         (throwOnError &&
             message is lsp.NotificationMessage &&
-            message.method == Method.window_showMessage));
+            message.method == Method.window_showMessage &&
+            message.params?.type == MessageType.Error));
 
     if (response is lsp.ResponseMessage) {
       return response;