[Code] stop respawn process after maxRespawn reached (#38392)

x-pack/plugins/code/common/lsp_error_codes.ts

@@ -11,3 +11,4 @@ export const UnknownErrorCode: number = ErrorCodes.UnknownErrorCode;
 export const UnknownFileLanguage: number = -42404;
 export const LanguageServerNotInstalled: number = -42403;
 export const LanguageDisabled: number = -42402;
+export const LanguageServerStartFailed: number = -42405;

x-pack/plugins/code/server/indexer/lsp_indexer.ts

@@ -11,7 +11,10 @@ import { ResponseError } from 'vscode-jsonrpc';
 
 import { ProgressReporter } from '.';
 import { TEXT_FILE_LIMIT } from '../../common/file';
-import { LanguageServerNotInstalled } from '../../common/lsp_error_codes';
+import {
+  LanguageServerNotInstalled,
+  LanguageServerStartFailed,
+} from '../../common/lsp_error_codes';
 import { toCanonicalUrl } from '../../common/uri_util';
 import { Document, IndexStats, IndexStatsKey, LspIndexRequest, RepositoryUri } from '../../model';
 import { GitOperations } from '../git_operations';
@@ -253,6 +256,10 @@ export class LspIndexer extends AbstractIndexer {
       if (error instanceof ResponseError && error.code === LanguageServerNotInstalled) {
         // TODO maybe need to report errors to the index task and warn user later
         this.log.debug(`Index symbols or references error due to language server not installed`);
+      } else if (error instanceof ResponseError && error.code === LanguageServerStartFailed) {
+        this.log.debug(
+          `Index symbols or references error due to language server can't be started.`
+        );
       } else {
         this.log.warn(`Index symbols or references error.`);
         this.log.warn(error);

x-pack/plugins/code/server/lsp/abstract_launcher.test.ts

@@ -11,7 +11,7 @@ import fs from 'fs';
 
 import { ServerOptions } from '../server_options';
 import { createTestServerOption } from '../test_utils';
-import { AbstractLauncher } from './abstract_launcher';
+import { AbstractLauncher, ServerStartFailed } from './abstract_launcher';
 import { RequestExpander } from './request_expander';
 import { LanguageServerProxy } from './proxy';
 import { ConsoleLoggerFactory } from '../utils/console_logger_factory';
@@ -34,6 +34,8 @@ class MockLauncher extends AbstractLauncher {
     super(name, targetHost, opt, new ConsoleLoggerFactory());
   }
 
+  protected maxRespawn = 3;
+
   createExpander(
     proxy: LanguageServerProxy,
     builtinWorkspace: boolean,
@@ -76,13 +78,13 @@ class PassiveMockLauncher extends MockLauncher {
     name: string,
     targetHost: string,
     opt: ServerOptions,
-    private dieFirstTime: boolean = false
+    private dieBeforeStart: number = 0
   ) {
     super(name, targetHost, opt);
   }
 
   startConnect(proxy: LanguageServerProxy) {
-    proxy.awaitServerConnection();
+    proxy.awaitServerConnection().catch(this.log.debug);
   }
 
   async getPort() {
@@ -98,9 +100,9 @@ class PassiveMockLauncher extends MockLauncher {
     });
     this.childProcess.send(`port ${port}`);
     this.childProcess.send(`host ${this.targetHost}`);
-    if (this.dieFirstTime) {
+    if (this.dieBeforeStart > 0) {
       this.childProcess!.send('quit');
-      this.dieFirstTime = false;
+      this.dieBeforeStart -= 1;
     } else {
       this.childProcess!.send('connect');
     }
@@ -184,7 +186,7 @@ test('launcher can reconnect if process died', async () => {
     expect(mockMonitor.mock.calls[2][0]).toBe('socket connected');
   });
   await proxy.exit();
-  await delay(2000);
+  await delay(1000);
 });
 
 test('passive launcher can start and end a process', async () => {
@@ -204,7 +206,7 @@ test('passive launcher can start and end a process', async () => {
 });
 
 test('passive launcher should restart a process if a process died before connected', async () => {
-  const launcher = new PassiveMockLauncher('mock', 'localhost', options, true);
+  const launcher = new PassiveMockLauncher('mock', 'localhost', options, 1);
   const proxy = await launcher.launch(false, 1, '');
   await delay(100);
   await retryUtil(30000, () => {
@@ -216,3 +218,19 @@ test('passive launcher should restart a process if a process died before connect
   await proxy.exit();
   await delay(1000);
 });
+
+test('launcher should mark proxy unusable after restart 2 times', async () => {
+  const launcher = new PassiveMockLauncher('mock', 'localhost', options, 3);
+  try {
+    await launcher.launch(false, 1, '');
+  } catch (e) {
+    await retryUtil(30000, () => {
+      expect(mockMonitor.mock.calls[0][0]).toBe('process started');
+      // restart 2 times
+      expect(mockMonitor.mock.calls[1][0]).toBe('process started');
+      expect(mockMonitor.mock.calls[2][0]).toBe('process started');
+    });
+    expect(e).toEqual(ServerStartFailed);
+    await delay(1000);
+  }
+});

x-pack/plugins/code/server/lsp/abstract_launcher.ts

@@ -5,22 +5,31 @@
  */
 
 import { ChildProcess } from 'child_process';
+import { ResponseError } from 'vscode-jsonrpc';
 import { ILanguageServerLauncher } from './language_server_launcher';
 import { ServerOptions } from '../server_options';
 import { LoggerFactory } from '../utils/log_factory';
 import { Logger } from '../log';
 import { LanguageServerProxy } from './proxy';
 import { RequestExpander } from './request_expander';
+import { LanguageServerStartFailed } from '../../common/lsp_error_codes';
 
 let seqNo = 1;
 
+export const ServerStartFailed = new ResponseError(
+  LanguageServerStartFailed,
+  'Launch language server failed.'
+);
+
 export abstract class AbstractLauncher implements ILanguageServerLauncher {
   running: boolean = false;
   private _currentPid: number = -1;
   private child: ChildProcess | null = null;
   private _startTime: number = -1;
   private _proxyConnected: boolean = false;
-  private readonly log: Logger;
+  protected readonly log: Logger;
+  private spawnTimes: number = 0;
+  private launchReject?: ((reason?: any) => void);
   protected constructor(
     readonly name: string,
     readonly targetHost: string,
@@ -49,6 +58,7 @@ export abstract class AbstractLauncher implements ILanguageServerLauncher {
       });
     } else {
       child = await this.spawnProcess(installationPath, port, log);
+      this.spawnTimes += 1;
       this.child = child;
       log.debug('spawned a child process ' + child.pid);
       this._currentPid = child.pid;
@@ -79,11 +89,15 @@ export abstract class AbstractLauncher implements ILanguageServerLauncher {
     });
     proxy.listen();
     this.startConnect(proxy);
-    await new Promise(resolve => {
+    await new Promise((resolve, reject) => {
       proxy.onConnected(() => {
         this._proxyConnected = true;
         resolve();
       });
+      this.launchReject = err => {
+        proxy.exit().catch(this.log.debug);
+        reject(err);
+      };
     });
     return this.createExpander(proxy, builtinWorkspace, maxWorkspace);
   }
@@ -106,6 +120,8 @@ export abstract class AbstractLauncher implements ILanguageServerLauncher {
    */
   protected startupTimeout = 10000;
 
+  protected maxRespawn = 3;
+
   /**
    * try reconnect the proxy when disconnected
    */
@@ -122,27 +138,34 @@ export abstract class AbstractLauncher implements ILanguageServerLauncher {
       if (child && !child.killed && !processExpired()) {
         this.startConnect(proxy);
       } else {
-        if (child && this.running) {
-          this.log.debug('killing the old process.');
-          await this.killProcess(child);
+        if (this.spawnTimes < this.maxRespawn) {
+          if (child && this.running) {
+            this.log.debug('killing the old process.');
+            await this.killProcess(child);
+          }
+          const port = await this.getPort();
+          proxy.changePort(port);
+          this.child = await this.spawnProcess(installationPath, port, this.log);
+          this.spawnTimes += 1;
+          this.log.debug('spawned a child process ' + this.child.pid);
+          this._currentPid = this.child.pid;
+          this._startTime = Date.now();
+          this.running = true;
+          this.onProcessExit(this.child, () => this.reconnect(proxy, installationPath, child));
+          this.startConnect(proxy);
+        } else {
+          this.log.warn(`spawned process ${this.spawnTimes} times, mark this proxy unusable.`);
+          proxy.setError(ServerStartFailed);
+          this.launchReject!(ServerStartFailed);
         }
-        const port = await this.getPort();
-        proxy.changePort(port);
-        this.child = await this.spawnProcess(installationPath, port, this.log);
-        this.log.debug('spawned a child process ' + this.child.pid);
-        this._currentPid = this.child.pid;
-        this._startTime = Date.now();
-        this.running = true;
-        this.onProcessExit(this.child, () => this.reconnect(proxy, installationPath, child));
-        this.startConnect(proxy);
       }
     }
   }
 
   abstract async getPort(): Promise<number>;
 
   startConnect(proxy: LanguageServerProxy) {
-    proxy.connect();
+    proxy.connect().catch(this.log.debug);
   }
 
   /**

x-pack/plugins/code/server/lsp/controller.ts

@@ -73,21 +73,17 @@ export class LanguageServerController implements ILanguageServerHandler {
     );
   }
 
-  public async handleRequest(request: LspRequest) {
+  public async handleRequest(request: LspRequest): Promise<ResponseMessage> {
     const file = request.resolvedFilePath;
     if (file) {
       // #todo add test for this
       const lang = await detectLanguage(file.replace('file://', ''));
       if (await this.repoConfigController.isLanguageDisabled(request.documentUri!, lang)) {
-        return Promise.reject(
-          new ResponseError(LanguageDisabled, `language disabled for the file`)
-        );
+        throw new ResponseError(LanguageDisabled, `language disabled for the file`);
       }
-      return this.dispatchRequest(lang, request);
+      return await this.dispatchRequest(lang, request);
     } else {
-      return Promise.reject(
-        new ResponseError(UnknownErrorCode, `can't detect language without a file`)
-      );
+      throw new ResponseError(UnknownErrorCode, `can't detect language without a file`);
     }
   }
 
@@ -102,19 +98,17 @@ export class LanguageServerController implements ILanguageServerHandler {
             this.installManager.installationPath(ls.definition)
           );
         }
-        const handler = ls.languageServerHandlers as Promise<ILanguageServerHandler>;
-        return (await handler).handleRequest(request);
+        const handler = await (ls.languageServerHandlers as Promise<ILanguageServerHandler>);
+        return await handler.handleRequest(request);
       } else {
         const handler = await this.findOrCreateHandler(ls, request);
         handler.lastAccess = Date.now();
-        return handler.handleRequest(request);
+        return await handler.handleRequest(request);
       }
     } else {
-      return Promise.reject(
-        new ResponseError(
-          UnknownFileLanguage,
-          `can't detect language from file:${request.resolvedFilePath}`
-        )
+      throw new ResponseError(
+        UnknownFileLanguage,
+        `can't detect language from file:${request.resolvedFilePath}`
       );
     }
   }

x-pack/plugins/code/server/lsp/java_launcher.ts

@@ -40,7 +40,7 @@ export class JavaLauncher extends AbstractLauncher {
   }
 
   startConnect(proxy: LanguageServerProxy) {
-    proxy.awaitServerConnection();
+    proxy.awaitServerConnection().catch(this.log.debug);
   }
 
   async getPort(): Promise<number> {

x-pack/plugins/code/server/lsp/proxy.ts

@@ -42,6 +42,7 @@ export interface ILanguageServerHandler {
 }
 
 export class LanguageServerProxy implements ILanguageServerHandler {
+  private error: any | null = null;
   public get isClosed() {
     return this.closed;
   }
@@ -78,6 +79,9 @@ export class LanguageServerProxy implements ILanguageServerHandler {
   }
 
   public receiveRequest(method: string, params: any, isNotification: boolean = false) {
+    if (this.error) {
+      return Promise.reject(this.error);
+    }
     const message: RequestMessage = {
       jsonrpc: '2.0',
       id: this.sequenceNumber++,
@@ -105,6 +109,9 @@ export class LanguageServerProxy implements ILanguageServerHandler {
     workspaceFolders: [WorkspaceFolder],
     initOptions?: object
   ): Promise<InitializeResult> {
+    if (this.error) {
+      throw this.error;
+    }
     const clientConn = await this.tryConnect();
     const rootUri = workspaceFolders[0].uri;
     const params = {
@@ -200,9 +207,9 @@ export class LanguageServerProxy implements ILanguageServerHandler {
           server.removeListener('error', rej);
           this.logger.info('Wait langserver connection on port ' + this.targetPort);
         });
-        onCancel!(() => {
+        onCancel!(error => {
           server.close();
-          rej('canceled');
+          rej(error);
         });
       });
     }
@@ -235,7 +242,7 @@ export class LanguageServerProxy implements ILanguageServerHandler {
     }
     this.closed = false;
     if (!this.connectingPromise) {
-      this.connectingPromise = new Cancelable((resolve, reject, onCancel) => {
+      this.connectingPromise = new Cancelable(resolve => {
         this.socket = new net.Socket();
 
         this.socket.on('connect', () => {
@@ -257,9 +264,6 @@ export class LanguageServerProxy implements ILanguageServerHandler {
           this.targetPort,
           this.targetHost
         );
-        onCancel!(() => {
-          reject('canceled');
-        });
       });
     }
     return this.connectingPromise.promise;
@@ -322,4 +326,12 @@ export class LanguageServerProxy implements ILanguageServerHandler {
       }
     }
   }
+
+  public setError(error: any) {
+    if (this.connectingPromise) {
+      this.connectingPromise.cancel(error);
+      this.connectingPromise = null;
+    }
+    this.error = error;
+  }
 }

x-pack/plugins/code/server/routes/lsp.ts

@@ -10,7 +10,11 @@ import { groupBy, last } from 'lodash';
 import { ResponseError } from 'vscode-jsonrpc';
 import { ResponseMessage } from 'vscode-jsonrpc/lib/messages';
 import { Location } from 'vscode-languageserver-types';
-import { ServerNotInitialized, UnknownFileLanguage } from '../../common/lsp_error_codes';
+import {
+  ServerNotInitialized,
+  UnknownFileLanguage,
+  LanguageServerStartFailed,
+} from '../../common/lsp_error_codes';
 import { parseLspUrl } from '../../common/uri_util';
 import { GitOperations } from '../git_operations';
 import { Logger } from '../log';
@@ -51,7 +55,11 @@ export function lspRoute(
           } catch (error) {
             if (error instanceof ResponseError) {
               // hide some errors;
-              if (error.code !== UnknownFileLanguage || error.code !== ServerNotInitialized) {
+              if (
+                error.code !== UnknownFileLanguage ||
+                error.code !== ServerNotInitialized ||
+                error.code !== LanguageServerStartFailed
+              ) {
                 log.debug(error);
               }
               return h