Do not crash server if non-mutating request fails to determine language

src/LanguageServer/Microsoft.CommonLanguageServerProtocol.Framework/IQueueItem.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -37,6 +38,12 @@ internal interface IQueueItem<TRequestContext>
     /// </summary>
     Task<(TRequestContext, TRequest)?> CreateRequestContextAsync<TRequest>(IMethodHandler handler, RequestHandlerMetadata requestHandlerMetadata, AbstractLanguageServer<TRequestContext> languageServer, CancellationToken cancellationToken);
 
+    /// <summary>
+    /// Handles when the queue needs to manually fail a request before the
+    /// handler is invoked without shutting down the entire queue.
+    /// </summary>
+    void FailRequest(Exception ex);
+
     /// <summary>
     /// Provides access to LSP services.
     /// </summary>

src/LanguageServer/Microsoft.CommonLanguageServerProtocol.Framework/QueueItem.cs

@@ -6,11 +6,7 @@
 #nullable enable
 
 using System;
-using System.Collections.Frozen;
 using System.Diagnostics.CodeAnalysis;
-using System.Diagnostics.Contracts;
-using System.Linq;
-using System.Reflection;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.VisualStudio.Threading;
@@ -30,6 +26,8 @@ internal class QueueItem<TRequestContext> : IQueueItem<TRequestContext>
     private readonly ILspLogger _logger;
     private readonly AbstractRequestScope? _requestTelemetryScope;
 
+    private bool _requestHandlingStarted = false;
+
     /// <summary>
     /// A task completion source representing the result of this queue item's work.
     /// This is the task that the client is waiting on.
@@ -158,6 +156,7 @@ private bool TryDeserializeRequest<TRequest>(
     /// </summary>
     public async Task StartRequestAsync<TRequest, TResponse>(TRequest request, TRequestContext? context, IMethodHandler handler, string language, CancellationToken cancellationToken)
     {
+        _requestHandlingStarted = true;
         _logger.LogStartContext($"{MethodName}");
 
         try
@@ -240,4 +239,17 @@ public async Task StartRequestAsync<TRequest, TResponse>(TRequest request, TRequ
         // so it can decide how to handle the result / exception.
         await _completionSource.Task.ConfigureAwait(false);
     }
+
+    public void FailRequest(Exception exception)
+    {
+        if (_requestHandlingStarted)
+        {
+            throw new InvalidOperationException("Cannot manually fail queue item after it has started");
+        }
+        _requestTelemetryScope?.RecordException(exception);
+        _logger.LogException(exception);
+
+        _completionSource.TrySetException(exception);
+        _requestTelemetryScope?.Dispose();
+    }
 }

src/LanguageServer/Microsoft.CommonLanguageServerProtocol.Framework/RequestExecutionQueue.cs

@@ -10,6 +10,7 @@
 using System.Collections.Frozen;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Reflection;
 using System.Threading;
@@ -245,11 +246,22 @@ private async Task ProcessQueueAsync()
                     // notifications have been completed by the time we attempt to determine the language, so we have the up to date map of URI to language.
                     // Since didOpen notifications are marked as mutating, the queue will not advance to the next request until the server has finished processing
                     // the didOpen, ensuring that this line will only run once all prior didOpens have completed.
-                    var language = _languageServer.GetLanguageForRequest(work.MethodName, work.SerializedRequest);
+                    var language = TryGetLanguageForRequest(work, out var exception);
 
                     // Now that we know the actual language, we can deserialize the request and start creating the request context.
                     var (metadata, handler, methodInfo) = GetHandlerForRequest(work, language);
 
+                    // We had an exception determining the language.  Generally this is very rare and only occurs
+                    // if there is a mis-behaving client that sends us requests for files where we haven't saved the languageId.
+                    // We should only crash if this was a mutating method, otherwise we should just fail the single request. 
+                    if (exception != null)
+                    {
+                        if (handler.MutatesSolutionState)
+                            throw exception;
+                        else
+                            work.FailRequest(exception);
+                    }
+
                     // We now have the actual handler and language, so we can process the work item using the concrete types defined by the metadata.
                     await InvokeProcessCoreAsync(work, metadata, handler, methodInfo, concurrentlyExecutingTasks, currentWorkCts, cancellationToken).ConfigureAwait(false);
                 }
@@ -382,6 +394,21 @@ private async Task ProcessQueueCoreAsync<TRequest, TResponse>(
         }
     }
 
+    private string TryGetLanguageForRequest(IQueueItem<TRequestContext> work,
+        out Exception? ex)
+    {
+        try
+        {
+            var language = _languageServer.GetLanguageForRequest(work.MethodName, work.SerializedRequest);
+            ex = null;
+            return language;
+        }
+        catch (Exception exception)
+        {
+            ex = exception;
+            return LanguageServerConstants.DefaultLanguageName;
+        }
+    }
     /// <summary>
     /// Allows an action to happen before the request runs, for example setting the current thread culture.
     /// </summary>

src/LanguageServer/ProtocolUnitTests/HandlerTests.cs

@@ -17,6 +17,7 @@
 using Roslyn.Test.Utilities;
 using Xunit;
 using Xunit.Abstractions;
+using static Microsoft.CodeAnalysis.LanguageServer.UnitTests.LocaleTests;
 
 namespace Microsoft.CodeAnalysis.LanguageServer.UnitTests
 {
@@ -295,6 +296,24 @@ await Assert.ThrowsAnyAsync<Exception>(async ()
             Assert.False(didReport);
         }
 
+        [Theory, CombinatorialData]
+        public async Task TestMutatingHandlerCrashesIfUnableToDetermineLanguage(bool mutatingLspWorkspace)
+        {
+            await using var testLspServer = await CreateTestLspServerAsync(string.Empty, mutatingLspWorkspace, new InitializationOptions { ServerKind = WellKnownLspServerKinds.CSharpVisualBasicLspServer });
+
+            // Run a mutating request against a file which we have no saved languageId for
+            // and where the language cannot be determined from the URI.
+            // This should crash the server.
+            var looseFileUri = ProtocolConversions.CreateAbsoluteUri(@"untitled:untitledFile");
+            var request = new TestRequestTypeOne(new TextDocumentIdentifier
+            {
+                Uri = looseFileUri
+            });
+
+            await Assert.ThrowsAnyAsync<Exception>(async() => await testLspServer.ExecuteRequestAsync<TestRequestTypeOne, string>(TestDocumentHandler.MethodName, request, CancellationToken.None)).ConfigureAwait(false);
+            await testLspServer.AssertServerShuttingDownAsync();
+        }
+
         internal record TestRequestTypeOne([property: JsonPropertyName("textDocument"), JsonRequired] TextDocumentIdentifier TextDocumentIdentifier);
 
         internal record TestRequestTypeTwo([property: JsonPropertyName("textDocument"), JsonRequired] TextDocumentIdentifier TextDocumentIdentifier);

src/LanguageServer/ProtocolUnitTests/UriTests.cs

@@ -295,6 +295,34 @@ await Assert.ThrowsAnyAsync<Exception>(async ()
                 new CustomResolveParams(new LSP.TextDocumentIdentifier { Uri = lowerCaseUri }), CancellationToken.None));
     }
 
+    [Theory, CombinatorialData]
+    public async Task TestDoesNotCrashIfUnableToDetermineLanguageInfo(bool mutatingLspWorkspace)
+    {
+        // Create a server that supports LSP misc files and verify no misc files present.
+        await using var testLspServer = await CreateTestLspServerAsync(string.Empty, mutatingLspWorkspace, new InitializationOptions { ServerKind = WellKnownLspServerKinds.CSharpVisualBasicLspServer });
+
+        // Open an empty loose file that hasn't been saved with a name.
+        var looseFileUri = ProtocolConversions.CreateAbsoluteUri(@"untitled:untitledFile");
+        await testLspServer.OpenDocumentAsync(looseFileUri, "hello", languageId: "csharp").ConfigureAwait(false);
+
+        // Verify file is added to the misc file workspace.
+        var (workspace, _, document) = await testLspServer.GetManager().GetLspDocumentInfoAsync(new LSP.TextDocumentIdentifier { Uri = looseFileUri }, CancellationToken.None);
+        Assert.True(workspace is LspMiscellaneousFilesWorkspace);
+        AssertEx.NotNull(document);
+        Assert.Equal(looseFileUri, document.GetURI());
+        Assert.Equal(looseFileUri.OriginalString, document.FilePath);
+
+        // Close the document (deleting the saved language information)
+        await testLspServer.CloseDocumentAsync(looseFileUri);
+
+        // Assert that the request throws but the server does not crash.
+        await Assert.ThrowsAnyAsync<Exception>(async ()
+            => await testLspServer.ExecuteRequestAsync<CustomResolveParams, ResolvedDocumentInfo>(CustomResolveHandler.MethodName,
+                new CustomResolveParams(new LSP.TextDocumentIdentifier { Uri = looseFileUri }), CancellationToken.None));
+        Assert.False(testLspServer.GetServerAccessor().HasShutdownStarted());
+        Assert.False(testLspServer.GetQueueAccessor()!.Value.IsComplete());
+    }
+
     private record class ResolvedDocumentInfo(string WorkspaceKind, string ProjectLanguage);
     private record class CustomResolveParams([property: JsonPropertyName("textDocument")] LSP.TextDocumentIdentifier TextDocument);