fix: handle shutdown lifecycle properly (#536)

.mocharc.yaml

@@ -1,8 +1,7 @@
-exit: true
 require:
     - 'source-map-support/register'
     - 'ts-node/register'
 spec: './src/**/*.spec.ts'
 # Set to 0 to disable timeout when debugging.
-timeout: 10000
+timeout: 20000
 watch-files: './src/**/*.ts'

package.json

@@ -24,7 +24,7 @@
     "clean": "rimraf lib *.tsbuildinfo",
     "test": "cross-env CONSOLE_LOG_LEVEL=warn TS_NODE_PROJECT=./tsconfig.json mocha",
     "test:watch": "cross-env CONSOLE_LOG_LEVEL=warn TS_NODE_PROJECT=./tsconfig.json mocha --watch",
-    "test:compiled": "mocha \"./lib/**/*.spec.js\"",
+    "test:compiled": "cross-env CONSOLE_LOG_LEVEL=warn mocha \"./lib/**/*.spec.js\"",
     "lint": "eslint --ext \".js,.ts\" src",
     "build": "concurrently -n compile,lint -c blue,green \"yarn compile\" \"yarn lint\"",
     "compile": "tsc -b",

src/file-lsp-server.spec.ts

@@ -19,10 +19,16 @@ before(async () => {
         publishDiagnostics: () => { }
     });
 });
+
 beforeEach(() => {
     server.closeAll();
 });
 
+after(() => {
+    server.closeAll();
+    server.shutdown();
+});
+
 describe('documentHighlight', () => {
     it('simple test', async () => {
         const doc = {

src/lsp-server.spec.ts

@@ -34,6 +34,7 @@ before(async () => {
         } as TypeScriptWorkspaceSettings
     });
 });
+
 beforeEach(() => {
     server.closeAll();
     // "closeAll" triggers final publishDiagnostics with an empty list so clear last.
@@ -43,6 +44,7 @@ beforeEach(() => {
 
 after(() => {
     server.closeAll();
+    server.shutdown();
 });
 
 describe('completion', () => {
@@ -1630,17 +1632,31 @@ export function factory() {
 });
 
 describe('diagnostics (no client support)', () => {
+    let localServer: TestLspServer;
+
     before(async () => {
         // Remove the "textDocument.publishDiagnostics" client capability.
         const clientCapabilitiesOverride = getDefaultClientCapabilities();
         delete clientCapabilitiesOverride.textDocument?.publishDiagnostics;
-        server = await createServer({
+        localServer = await createServer({
             rootUri: null,
             publishDiagnostics: args => diagnostics.set(args.uri, args),
             clientCapabilitiesOverride
         });
     });
 
+    beforeEach(() => {
+        localServer.closeAll();
+        // "closeAll" triggers final publishDiagnostics with an empty list so clear last.
+        diagnostics.clear();
+        localServer.workspaceEdits = [];
+    });
+
+    after(() => {
+        localServer.closeAll();
+        localServer.shutdown();
+    });
+
     it('diagnostics are published', async () => {
         const doc = {
             uri: uri('diagnosticsBar.ts'),
@@ -1652,11 +1668,11 @@ describe('diagnostics (no client support)', () => {
         }
       `
         };
-        server.didOpenTextDocument({
+        localServer.didOpenTextDocument({
             textDocument: doc
         });
 
-        await server.requestDiagnostics();
+        await localServer.requestDiagnostics();
         await new Promise(resolve => setTimeout(resolve, 200));
         const resultsForFile = diagnostics.get(doc.uri);
         assert.isDefined(resultsForFile);
@@ -1665,25 +1681,39 @@ describe('diagnostics (no client support)', () => {
 });
 
 describe('jsx/tsx project', () => {
+    let localServer: TestLspServer;
+
     before(async () => {
-        server = await createServer({
+        localServer = await createServer({
             rootUri: uri('jsx'),
             publishDiagnostics: args => diagnostics.set(args.uri, args)
         });
     });
 
+    beforeEach(() => {
+        localServer.closeAll();
+        // "closeAll" triggers final publishDiagnostics with an empty list so clear last.
+        diagnostics.clear();
+        localServer.workspaceEdits = [];
+    });
+
+    after(() => {
+        localServer.closeAll();
+        localServer.shutdown();
+    });
+
     it('includes snippet completion for element prop', async () => {
         const doc = {
             uri: uri('jsx', 'app.tsx'),
             languageId: 'typescriptreact',
             version: 1,
             text: readContents(filePath('jsx', 'app.tsx'))
         };
-        server.didOpenTextDocument({
+        localServer.didOpenTextDocument({
             textDocument: doc
         });
 
-        const completion = await server.completion({ textDocument: doc, position: position(doc, 'title') });
+        const completion = await localServer.completion({ textDocument: doc, position: position(doc, 'title') });
         assert.isNotNull(completion);
         const item = completion!.items.find(i => i.label === 'title');
         assert.isDefined(item);
@@ -1692,44 +1722,31 @@ describe('jsx/tsx project', () => {
 });
 
 describe('inlayHints', () => {
-    it('inlayHints', async () => {
-        const doc = {
-            uri: uri('module.ts'),
-            languageId: 'typescript',
-            version: 1,
-            text: `
-        export function foo() {
-          return 3
-        }
-      `
-        };
-        await server.initialize({
-            initializationOptions: {
-                preferences: {
-                    includeInlayFunctionLikeReturnTypeHints: true
+    before(async () => {
+        server.didChangeConfiguration({
+            settings: {
+                typescript: {
+                    inlayHints: {
+                        includeInlayFunctionLikeReturnTypeHints: true
+                    }
                 }
-            },
-            processId: null,
-            capabilities: getDefaultClientCapabilities(),
-            workspaceFolders: [],
-            rootUri: ''
-        });
-        server.didOpenTextDocument({
-            textDocument: doc
+            }
         });
+    });
 
-        const { inlayHints } = await server.inlayHints({
-            textDocument: doc
+    after(() => {
+        server.didChangeConfiguration({
+            settings: {
+                typescript: {
+                    inlayHints: {
+                        includeInlayFunctionLikeReturnTypeHints: false
+                    }
+                }
+            }
         });
-
-        assert.isDefined(inlayHints);
-        assert.strictEqual(inlayHints.length, 1);
-        assert.strictEqual(inlayHints[0].text, ': number');
-        assert.strictEqual(inlayHints[0].kind, 'Type');
-        assert.deepStrictEqual(inlayHints[0].position, { line: 1, character: 29 });
     });
 
-    it('inlayHints options set through workspace configuration ', async () => {
+    it('inlayHints', async () => {
         const doc = {
             uri: uri('module.ts'),
             languageId: 'typescript',
@@ -1740,31 +1757,8 @@ describe('inlayHints', () => {
         }
       `
         };
-        await server.initialize({
-            processId: null,
-            capabilities: getDefaultClientCapabilities(),
-            workspaceFolders: [],
-            rootUri: ''
-        });
-
-        server.didChangeConfiguration({
-            settings: {
-                typescript: {
-                    inlayHints: {
-                        includeInlayFunctionLikeReturnTypeHints: true
-                    }
-                }
-            }
-        });
-
-        server.didOpenTextDocument({
-            textDocument: doc
-        });
-
-        const { inlayHints } = await server.inlayHints({
-            textDocument: doc
-        });
-
+        server.didOpenTextDocument({ textDocument: doc });
+        const { inlayHints } = await server.inlayHints({ textDocument: doc });
         assert.isDefined(inlayHints);
         assert.strictEqual(inlayHints.length, 1);
         assert.strictEqual(inlayHints[0].text, ': number');

src/lsp-server.ts

@@ -111,15 +111,15 @@ class ServerInitializingIndicator {
 }
 
 export class LspServer {
+    private _tspClient: TspClient | null;
+    private _loadingIndicator: ServerInitializingIndicator | null;
     private initializeParams: TypeScriptInitializeParams;
     private initializeResult: TypeScriptInitializeResult;
-    private tspClient: TspClient;
     private diagnosticQueue?: DiagnosticEventQueue;
     private logger: Logger;
     private workspaceConfiguration: TypeScriptWorkspaceSettings;
     private workspaceRoot: string | undefined;
     private typeScriptAutoFixProvider: TypeScriptAutoFixProvider;
-    private loadingIndicator: ServerInitializingIndicator;
     private features: SupportedFeatures = {};
 
     private readonly documents = new LspDocuments();
@@ -135,6 +135,31 @@ export class LspServer {
         }
     }
 
+    shutdown(): void {
+        if (this._tspClient) {
+            this._tspClient.shutdown();
+            this._tspClient = null;
+        }
+        if (this._loadingIndicator) {
+            this._loadingIndicator.reset();
+            this._loadingIndicator = null;
+        }
+    }
+
+    private get tspClient(): TspClient {
+        if (!this._tspClient) {
+            throw new Error('TS client not created. Did you forget to send the "initialize" request?');
+        }
+        return this._tspClient;
+    }
+
+    private get loadingIndicator(): ServerInitializingIndicator {
+        if (!this._loadingIndicator) {
+            throw new Error('Loading indicator not created. Did you forget to send the "initialize" request?');
+        }
+        return this._loadingIndicator;
+    }
+
     private findTypescriptVersion(): TypeScriptVersion | null {
         const typescriptVersionProvider = new TypeScriptVersionProvider(this.options, this.logger);
         // User-provided tsserver path.
@@ -162,10 +187,13 @@ export class LspServer {
 
     async initialize(params: TypeScriptInitializeParams): Promise<TypeScriptInitializeResult> {
         this.logger.log('initialize', params);
+        if (this._tspClient) {
+            throw new Error('The "initialize" request has already called before.');
+        }
         this.initializeParams = params;
         const clientCapabilities = this.initializeParams.capabilities;
         this.options.lspClient.setClientCapabilites(clientCapabilities);
-        this.loadingIndicator = new ServerInitializingIndicator(this.options.lspClient);
+        this._loadingIndicator = new ServerInitializingIndicator(this.options.lspClient);
         this.workspaceRoot = this.initializeParams.rootUri ? uriToPath(this.initializeParams.rootUri) : this.initializeParams.rootPath || undefined;
         this.diagnosticQueue = new DiagnosticEventQueue(
             diagnostics => this.options.lspClient.publishDiagnostics(diagnostics),
@@ -212,7 +240,7 @@ export class LspServer {
             ...{ useLabelDetailsInCompletionEntries: this.features.labelDetails }
         };
 
-        this.tspClient = new TspClient({
+        this._tspClient = new TspClient({
             tsserverPath: typescriptVersion.tsServerPath,
             logFile,
             logVerbosity,
@@ -226,8 +254,7 @@ export class LspServer {
             onEvent: this.onTsEvent.bind(this),
             onExit: (exitCode, signal) => {
                 this.logger.error(`tsserver process has exited (exit code: ${exitCode}, signal: ${signal}). Stopping the server.`);
-                // Allow the log to be dispatched to the client.
-                setTimeout(() => process.exit(1));
+                this.shutdown();
             }
         });
 
@@ -236,10 +263,7 @@ export class LspServer {
             throw new Error('tsserver process has failed to start.');
         }
         process.on('exit', () => {
-            this.tspClient.shutdown();
-            if (this.loadingIndicator) {
-                this.loadingIndicator.reset();
-            }
+            this.shutdown();
         });
         process.on('SIGINT', () => {
             process.exit();

src/tsp-client.spec.ts

@@ -15,10 +15,17 @@ import { TypeScriptVersionProvider } from './utils/versionProvider';
 const assert = chai.assert;
 const typescriptVersionProvider = new TypeScriptVersionProvider();
 const bundled = typescriptVersionProvider.bundledVersion();
+let server: TspClient;
 
-const server = new TspClient({
-    logger: new ConsoleLogger(),
-    tsserverPath: bundled!.tsServerPath
+before(() => {
+    server = new TspClient({
+        logger: new ConsoleLogger(),
+        tsserverPath: bundled!.tsServerPath
+    });
+});
+
+after(() => {
+    server.shutdown();
 });
 
 describe('ts server client', () => {

src/tsp-client.ts

@@ -68,8 +68,8 @@ interface TypeScriptRequestTypes {
 }
 
 export class TspClient {
+    private tsserverProc: cp.ChildProcess | null;
     private readlineInterface: readline.ReadLine;
-    private tsserverProc: cp.ChildProcess;
     private seq = 0;
     private readonly deferreds: { [seq: number]: Deferred<any>; } = {};
     private logger: Logger;
@@ -154,8 +154,10 @@ export class TspClient {
 
     shutdown(): void {
         this.readlineInterface?.close();
-        this.tsserverProc.stdin?.destroy();
-        this.tsserverProc.kill();
+        if (this.tsserverProc) {
+            this.tsserverProc.stdin?.destroy();
+            this.tsserverProc.kill('SIGTERM');
+        }
     }
 
     notify(command: CommandTypes.Open, args: protocol.OpenRequestArgs): void;
@@ -205,8 +207,12 @@ export class TspClient {
             request.arguments = args;
         }
         const serializedRequest = JSON.stringify(request) + '\n';
-        this.tsserverProc.stdin!.write(serializedRequest);
-        this.logger.log(notification ? 'notify' : 'request', request);
+        if (this.tsserverProc) {
+            this.tsserverProc.stdin!.write(serializedRequest);
+            this.logger.log(notification ? 'notify' : 'request', request);
+        } else {
+            this.logger.error(`Message "${command}" couldn't be sent. Tsserver process not started.`);
+        }
     }
 
     protected processMessage(untrimmedMessageString: string): void {