Improve server initialization and error handling

CHANGELOG.md

@@ -4,6 +4,13 @@
 - Debug from .csproj and .sln [#5876](https://github.com/dotnet/vscode-csharp/issues/5876)
 
 # 2.123.x
+* Improve error reporting UX when server encounters an error (PR: [#8982](https://github.com/dotnet/vscode-csharp/pull/8982))
+* Update Roslyn to 5.5.0-2.26117.2 (PR: [#8982](https://github.com/dotnet/vscode-csharp/pull/8982))
+  * Improve error handling when the server hits an unrecoverable error (PR: [#82376](https://github.com/dotnet/roslyn/pull/82376))
+  * Remove deprecated IntelliCode starred-completion support from Roslyn Language Server (PR: [#82411](https://github.com/dotnet/roslyn/pull/82411))
+  * Fix `GetDeconstructionInfo` on converted deconstruction assignment (PR: [#82324](https://github.com/dotnet/roslyn/pull/82324))
+  * Fix crash in simplify linq expression (PR: [#82392](https://github.com/dotnet/roslyn/pull/82392))
+  * Do not suggest using with-element in pre-C# 15 (PR: [#82389](https://github.com/dotnet/roslyn/pull/82389))
 * Collect Razor logs (PR: [#8988](https://github.com/dotnet/vscode-csharp/pull/8988))
 * Update Razor to 10.0.0-preview.26115.1 (PR: [#9007](https://github.com/dotnet/vscode-csharp/pull/9007))
   * Fix one formatting bug, and prevent another from crashing the formatter (PR: [#12786](https://github.com/dotnet/razor/pull/12786))

l10n/bundle.l10n.json

@@ -157,9 +157,10 @@
   "Do not load any": "Do not load any",
   "Reload C# Extension": "Reload C# Extension",
   "Detected change in telemetry settings. These will not take effect until the language server is restarted, would you like to restart?": "Detected change in telemetry settings. These will not take effect until the language server is restarted, would you like to restart?",
-  "IntelliCode features will not be available, {0} failed to activate.": "IntelliCode features will not be available, {0} failed to activate.",
   "Go to output": "Go to output",
-  "Suppress notification": "Suppress notification",
+  "Report Issue": "Report Issue",
+  "The C# language server has crashed. Restart extensions to re-enable C# functionality.": "The C# language server has crashed. Restart extensions to re-enable C# functionality.",
+  "Restart extensions": "Restart extensions",
   "Restore {0}": "Restore {0}",
   "Restore already in progress": "Restore already in progress",
   "Select context": "Select context",

package.json

@@ -40,7 +40,7 @@
     "workspace"
   ],
   "defaults": {
-    "roslyn": "5.5.0-2.26109.18",
+    "roslyn": "5.5.0-2.26117.2",
     "omniSharp": "1.39.14",
     "razor": "10.0.0-preview.26115.1",
     "razorOmnisharp": "7.0.0-preview.23363.1",

src/lsptoolshost/server/roslynLanguageClient.ts

@@ -3,31 +3,53 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import * as vscode from 'vscode';
 import { CancellationToken, MessageSignature } from 'vscode-jsonrpc';
-import { LanguageClient } from 'vscode-languageclient/node';
-import { LanguageClientOptions, ServerOptions } from 'vscode-languageclient';
+import { LanguageClient, ServerOptions, State } from 'vscode-languageclient/node';
+import { ErrorHandler, LanguageClientOptions } from 'vscode-languageclient';
 import CompositeDisposable from '../../compositeDisposable';
 import { IDisposable } from '../../disposable';
 import { languageServerOptions } from '../../shared/options';
 import { RoslynLspErrorCodes } from './roslynProtocol';
+import { showErrorMessageWithOptions } from '../../shared/observers/utils/showMessage';
 
 /**
  * Implementation of the base LanguageClient type that allows for additional items to be disposed of
  * when the base LanguageClient instance is disposed.
  */
 export class RoslynLanguageClient extends LanguageClient {
     private readonly _disposables: CompositeDisposable;
+    private readonly _csharpOutputWindow: vscode.OutputChannel;
+
+    /**
+     * Tracks if we've shown a connection close notification for the server session to
+     * prevent notification spam when the server crashes.
+     * This is reset when the server restarts.
+     */
+    private _hasShownConnectionClose = false;
 
     constructor(
         id: string,
         name: string,
         serverOptions: ServerOptions,
         clientOptions: LanguageClientOptions,
+        csharpOutputWindow: vscode.OutputChannel,
         forceDebug?: boolean
     ) {
         super(id, name, serverOptions, clientOptions, forceDebug);
 
         this._disposables = new CompositeDisposable();
+        this._csharpOutputWindow = csharpOutputWindow;
+
+        this.registerStateChangeHandler();
+    }
+
+    private registerStateChangeHandler() {
+        this.onDidChangeState((e) => {
+            if (e.newState === State.Running) {
+                this._hasShownConnectionClose = false;
+            }
+        });
     }
 
     override async dispose(timeout?: number | undefined): Promise<void> {
@@ -57,13 +79,105 @@ export class RoslynLanguageClient extends LanguageClient {
         if (languageServerOptions.suppressLspErrorToasts) {
             return super.handleFailedRequest(type, token, error, defaultValue, false);
         }
+
         return super.handleFailedRequest(type, token, error, defaultValue, showNotification);
     }
 
+    /**
+     * The default error handler handles server crashes and connection lost issues.
+     * This is not to be confused with the override of the error method specifically, which handles
+     * display any error (including both request failures and critical errors from here).
+     */
+    override createDefaultErrorHandler(maxRestartCount?: number): ErrorHandler {
+        const defaultHandler = super.createDefaultErrorHandler(maxRestartCount);
+
+        // the error function here is called for errors writing or reading from the connection.  the closed function is called when the connection is closed.
+        // note that both of these can be called in the crash scenario, so we de-dupe notifications here.
+        return {
+            error: async (error, message, count) => {
+                this.showCrashNotification();
+                // The default error handler will determine if the server should be restarted.  We just want to ensure a good notification, so we defer to the default handler for that logic.
+                const defaultResult = await defaultHandler.error(error, message, count);
+                // The handled property indicates to the default handling that we've displayed our own notification.
+                defaultResult.handled = true;
+                return defaultResult;
+            },
+            closed: async () => {
+                this.showCrashNotification();
+                const defaultResult = await defaultHandler.closed();
+                defaultResult.handled = true;
+                return defaultResult;
+            },
+        };
+    }
+
+    /**
+     * Handles displaying any errors reported by the language client.  This is called for both standard request failures
+     * as well as critical server errors (e.g. crashes).
+     */
+    override error(message: string, data?: any, showNotification?: boolean | 'force'): void {
+        // When the server crashes, we may get single method request failures due to the closed connection.
+        // To avoid spamming users, don't display error toasts for these.
+        if (this._hasShownConnectionClose) {
+            showNotification = false;
+        }
+
+        // We have an error but we're not in a crash scenario.  Override the default error toast with one that includes the report issue command.
+        if (showNotification) {
+            showNotification = false;
+            showErrorMessageWithOptions(
+                vscode,
+                message,
+                { modal: false },
+                {
+                    title: vscode.l10n.t('Go to output'),
+                    action: async () => {
+                        this._csharpOutputWindow.show(true);
+                    },
+                },
+                {
+                    title: vscode.l10n.t('Report Issue'),
+                    command: 'csharp.reportIssue',
+                }
+            );
+        }
+
+        super.error(message, data, showNotification);
+    }
+
     /**
      * Adds a disposable that should be disposed of when the LanguageClient instance gets disposed.
      */
     public addDisposable(disposable: IDisposable) {
         this._disposables.add(disposable);
     }
+
+    private showCrashNotification() {
+        if (this._hasShownConnectionClose) {
+            return;
+        }
+
+        this._hasShownConnectionClose = true;
+        this.showCrashNotificationCore();
+    }
+
+    private showCrashNotificationCore() {
+        showErrorMessageWithOptions(
+            vscode,
+            vscode.l10n.t('The C# language server has crashed. Restart extensions to re-enable C# functionality.'),
+            { modal: false },
+            {
+                title: vscode.l10n.t('Restart extensions'),
+                command: 'workbench.action.restartExtensionHost',
+            },
+            {
+                title: vscode.l10n.t('Report Issue'),
+                action: async () => {
+                    vscode.commands.executeCommand('csharp.reportIssue');
+                    // Re-show the notification so the user can still restart extensions after reporting.
+                    this.showCrashNotificationCore();
+                },
+            }
+        );
+    }
 }