Add try/catch blocks for request handlers (#408)

* Add try/catch for request handlers

* Fix test break

src/Microsoft.SqlTools.ServiceLayer/DisasterRecovery/DisasterRecoveryService.cs

@@ -87,6 +87,8 @@ namespace Microsoft.SqlTools.ServiceLayer.DisasterRecovery
         public static async Task HandleBackupConfigInfoRequest(
             DefaultDatabaseInfoParams optionsParams,
             RequestContext<BackupConfigInfoResponse> requestContext)
+        {
+            try
             {
                 var response = new BackupConfigInfoResponse();
                 ConnectionInfo connInfo;
@@ -108,6 +110,11 @@ namespace Microsoft.SqlTools.ServiceLayer.DisasterRecovery
 
                 await requestContext.SendResult(response);
             }
+            catch (Exception ex)
+            {
+                await requestContext.SendError(ex.ToString());
+            }
+        }
 
         /// <summary>
         /// Handles a restore request
@@ -115,6 +122,8 @@ namespace Microsoft.SqlTools.ServiceLayer.DisasterRecovery
         internal async Task HandleRestorePlanRequest(
             RestoreParams restoreParams,
             RequestContext<RestorePlanResponse> requestContext)
+        {
+            try
             {
                 RestorePlanResponse response = new RestorePlanResponse();
                 ConnectionInfo connInfo;
@@ -131,7 +140,11 @@ namespace Microsoft.SqlTools.ServiceLayer.DisasterRecovery
                     response.ErrorMessage = SR.RestoreNotSupported;
                 }
                 await requestContext.SendResult(response);
-
+            }
+            catch (Exception ex)
+            {
+                await requestContext.SendError(ex.ToString());
+            }
         }
 
         /// <summary>
@@ -140,6 +153,8 @@ namespace Microsoft.SqlTools.ServiceLayer.DisasterRecovery
         internal async Task HandleRestoreRequest(
             RestoreParams restoreParams,
             RequestContext<RestoreResponse> requestContext)
+        {
+            try
             {
                 RestoreResponse response = new RestoreResponse();
                 ConnectionInfo connInfo;
@@ -182,6 +197,11 @@ namespace Microsoft.SqlTools.ServiceLayer.DisasterRecovery
 
                 await requestContext.SendResult(response);
             }
+            catch (Exception ex)
+            {
+                await requestContext.SendError(ex.ToString());
+            }
+        }
 
         /// <summary>
         /// Handles a backup request
@@ -189,6 +209,8 @@ namespace Microsoft.SqlTools.ServiceLayer.DisasterRecovery
         internal static async Task HandleBackupRequest(
             BackupParams backupParams,
             RequestContext<BackupResponse> requestContext)
+        {     
+            try
             {       
                 ConnectionInfo connInfo;
                 DisasterRecoveryService.ConnectionServiceInstance.TryFindConnection(
@@ -219,6 +241,11 @@ namespace Microsoft.SqlTools.ServiceLayer.DisasterRecovery
             
                 await requestContext.SendResult(new BackupResponse());
             }
+            catch (Exception ex)
+            {
+                await requestContext.SendError(ex.ToString());
+            }
+        }
 
         internal static SqlConnection GetSqlConnection(ConnectionInfo connInfo)
         {

src/Microsoft.SqlTools.ServiceLayer/Formatter/TSqlFormatterService.cs

@@ -63,7 +63,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Formatter
             EventContext eventContext)
         {
             // update the current settings to reflect any changes (assuming formatter settings exist)
-            settings = newSettings.SqlTools.Format ?? settings;
+            settings = newSettings?.SqlTools?.Format ?? settings;
             return Task.FromResult(true);
         }
 

src/Microsoft.SqlTools.ServiceLayer/LanguageServices/LanguageService.cs

@@ -287,6 +287,8 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
         internal async Task HandleCompletionRequest(
             TextDocumentPosition textDocumentPosition,
             RequestContext<CompletionItem[]> requestContext)
+        {            
+            try
             {            
                 // check if Intellisense suggestions are enabled
                 if (ShouldSkipIntellisense(textDocumentPosition.TextDocument.Uri))
@@ -315,6 +317,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
                     await requestContext.SendResult(completionItems);       
                 }
             }
+            catch (Exception ex)
+            {
+                await requestContext.SendError(ex.ToString());
+            }
+        }
 
         /// <summary>
         /// Handle the resolve completion request event to provide additional
@@ -326,6 +333,8 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
         internal async Task HandleCompletionResolveRequest(
             CompletionItem completionItem,
             RequestContext<CompletionItem> requestContext)
+        {
+            try
             {
                 // check if Intellisense suggestions are enabled
                 // Note: Do not know file, so no need to check for MSSQL flavor
@@ -339,8 +348,15 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
                     await requestContext.SendResult(completionItem);
                 }
             }
+            catch (Exception ex)
+            {
+                await requestContext.SendError(ex.ToString());
+            }
+        }
 
         internal async Task HandleDefinitionRequest(TextDocumentPosition textDocumentPosition, RequestContext<Location[]> requestContext)
+        {
+            try 
             {
                 DocumentStatusHelper.SendStatusChange(requestContext, textDocumentPosition, DocumentStatusHelper.DefinitionRequested);
 
@@ -381,6 +397,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
 
                 DocumentStatusHelper.SendStatusChange(requestContext, textDocumentPosition, DocumentStatusHelper.DefinitionRequestCompleted);
             }
+            catch (Exception ex)
+            {
+                await requestContext.SendError(ex.ToString());
+            }
+        }
 
         private static TelemetryProperties CreatePeekTelemetryProps(bool succeeded, bool connected)
         {
@@ -415,6 +436,8 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
         internal async Task HandleSignatureHelpRequest(
             TextDocumentPosition textDocumentPosition,
             RequestContext<SignatureHelp> requestContext)
+        {
+            try
             {
                 // check if Intellisense suggestions are enabled
                 if (ShouldSkipNonMssqlFile(textDocumentPosition))
@@ -440,10 +463,17 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
                     }
                 }
             }
+            catch (Exception ex)
+            {
+                await requestContext.SendError(ex.ToString());
+            }
+        }
 
         private async Task HandleHoverRequest(
             TextDocumentPosition textDocumentPosition,
             RequestContext<Hover> requestContext)
+        {
+            try
             {
                 // check if Quick Info hover tooltips are enabled
                 if (CurrentWorkspaceSettings.IsQuickInfoEnabled
@@ -465,6 +495,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
 
                 await requestContext.SendResult(new Hover());
             }
+            catch (Exception ex)
+            {
+                await requestContext.SendError(ex.ToString());
+            }
+        }
 
         #endregion
 
@@ -479,6 +514,8 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
         public async Task HandleDidOpenTextDocumentNotification(
             ScriptFile scriptFile,
             EventContext eventContext)
+        {
+            try
             {
                 // if not in the preview window and diagnostics are enabled then run diagnostics
                 if (!IsPreviewWindow(scriptFile)
@@ -491,6 +528,12 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
 
                 await Task.FromResult(true);
             }
+            catch (Exception ex)
+            {
+                Logger.Write(LogLevel.Error, "Unknown error " + ex.ToString());
+                // TODO: need mechanism return errors from event handlers
+            }
+        }
 
         /// <summary>
         /// Handles text document change events
@@ -498,6 +541,8 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
         /// <param name="textChangeParams"></param>
         /// <param name="eventContext"></param>
         public async Task HandleDidChangeTextDocumentNotification(ScriptFile[] changedFiles, EventContext eventContext)
+        {
+            try
             {
                 if (CurrentWorkspaceSettings.IsDiagnosticsEnabled)
                 {
@@ -509,6 +554,12 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
 
                 await Task.FromResult(true);
             }
+            catch (Exception ex)
+            {
+                Logger.Write(LogLevel.Error, "Unknown error " + ex.ToString());
+                // TODO: need mechanism return errors from event handlers
+            }
+        }
 
         /// <summary>
         /// Handle the rebuild IntelliSense cache notification
@@ -593,6 +644,8 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
             SqlToolsSettings newSettings,
             SqlToolsSettings oldSettings,
             EventContext eventContext)
+        {
+            try
             {
                 bool oldEnableIntelliSense = oldSettings.SqlTools.IntelliSense.EnableIntellisense;
                 bool? oldEnableDiagnostics = oldSettings.SqlTools.IntelliSense.EnableErrorChecking;
@@ -621,6 +674,50 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
                     }
                 }
             }
+            catch (Exception ex)
+            {
+                Logger.Write(LogLevel.Error, "Unknown error " + ex.ToString());
+                // TODO: need mechanism return errors from event handlers
+            }
+        }
+
+        /// <summary>
+        /// Handles language flavor changes by disabling intellisense on a file if it does not match the specific
+        /// "MSSQL" language flavor returned by our service
+        /// </summary>
+        /// <param name="info"></param>
+        public async Task HandleDidChangeLanguageFlavorNotification(
+            LanguageFlavorChangeParams changeParams,
+            EventContext eventContext) 
+        {
+            try
+            {
+                Validate.IsNotNull(nameof(changeParams), changeParams);
+                Validate.IsNotNull(nameof(changeParams), changeParams.Uri);
+                bool shouldBlock = false;
+                if (SQL_LANG.Equals(changeParams.Language, StringComparison.OrdinalIgnoreCase)) {
+                    shouldBlock = !ServiceHost.ProviderName.Equals(changeParams.Flavor, StringComparison.OrdinalIgnoreCase);
+                }
+
+                if (shouldBlock) {
+                    this.nonMssqlUriMap.AddOrUpdate(changeParams.Uri, true, (k, oldValue) => true);
+                    if (CurrentWorkspace.ContainsFile(changeParams.Uri))
+                    {
+                        await DiagnosticsHelper.ClearScriptDiagnostics(changeParams.Uri, eventContext);
+                    }
+                }
+                else
+                {
+                    bool value;
+                    this.nonMssqlUriMap.TryRemove(changeParams.Uri, out value);
+                }
+            }
+            catch (Exception ex)
+            {
+                Logger.Write(LogLevel.Error, "Unknown error " + ex.ToString());
+                // TODO: need mechanism return errors from event handlers
+            }
+        }
 
         #endregion
 
@@ -852,36 +949,6 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
             }
         }
 
-        /// <summary>
-        /// Handles language flavor changes by disabling intellisense on a file if it does not match the specific
-        /// "MSSQL" language flavor returned by our service
-        /// </summary>
-        /// <param name="info"></param>
-        public async Task HandleDidChangeLanguageFlavorNotification(
-            LanguageFlavorChangeParams changeParams,
-            EventContext eventContext) 
-        {
-            Validate.IsNotNull(nameof(changeParams), changeParams);
-            Validate.IsNotNull(nameof(changeParams), changeParams.Uri);
-            bool shouldBlock = false;
-            if (SQL_LANG.Equals(changeParams.Language, StringComparison.OrdinalIgnoreCase)) {
-                shouldBlock = !ServiceHost.ProviderName.Equals(changeParams.Flavor, StringComparison.OrdinalIgnoreCase);
-            }
-
-            if (shouldBlock) {
-                this.nonMssqlUriMap.AddOrUpdate(changeParams.Uri, true, (k, oldValue) => true);
-                if (CurrentWorkspace.ContainsFile(changeParams.Uri))
-                {
-                    await DiagnosticsHelper.ClearScriptDiagnostics(changeParams.Uri, eventContext);
-                }
-            }
-            else
-            {
-                bool value;
-                this.nonMssqlUriMap.TryRemove(changeParams.Uri, out value);
-            }
-        }
-
         private bool ShouldSkipNonMssqlFile(TextDocumentPosition textDocPosition)
         {
             return ShouldSkipNonMssqlFile(textDocPosition.TextDocument.Uri);

src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/ObjectExplorerService.cs

@@ -134,15 +134,16 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer
             EventContext eventContext)
         {
             // update the current settings to reflect any changes (assuming formatter settings exist)
-            settings = newSettings.SqlTools.ObjectExplorer ?? settings;
+            settings = newSettings?.SqlTools?.ObjectExplorer ?? settings;
             return Task.FromResult(true);
         }
 
 
         internal async Task HandleCreateSessionRequest(ConnectionDetails connectionDetails, RequestContext<CreateSessionResponse> context)
+        {
+            try
             {
                 Logger.Write(LogLevel.Verbose, "HandleCreateSessionRequest");
-
                 Func<Task<CreateSessionResponse>> doCreateSession = async () =>
                 {
                     Validate.IsNotNull(nameof(connectionDetails), connectionDetails);
@@ -160,6 +161,11 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer
                 {
                     RunCreateSessionTask(connectionDetails, response.SessionId);
                 }
+            }
+            catch (Exception ex)
+            {
+                await context.SendError(ex.ToString());
+            }
 
         }
 
@@ -195,6 +201,8 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer
         }
 
         internal async Task HandleRefreshRequest(RefreshParams refreshParams, RequestContext<bool> context)
+        {
+            try
             {
                 Logger.Write(LogLevel.Verbose, "HandleRefreshRequest");
                 Validate.IsNotNull(nameof(refreshParams), refreshParams);
@@ -218,6 +226,11 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer
                 }
                 await context.SendResult(true);
             }
+            catch (Exception ex)
+            {
+                await context.SendError(ex.ToString());
+            }
+        }
 
         internal async Task HandleCloseSessionRequest(CloseSessionParams closeSessionParams, RequestContext<CloseSessionResponse> context)
         {

src/Microsoft.SqlTools.ServiceLayer/Workspace/WorkspaceService.cs

@@ -232,8 +232,9 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
                 var handlers = TextDocChangeCallbacks.Select(t => t(changedFiles.ToArray(), eventContext));
                 return Task.WhenAll(handlers);
             }
-            catch
+            catch (Exception ex)
             {
+                Logger.Write(LogLevel.Error, "Unknown error " + ex.ToString());
                 // Swallow exceptions here to prevent us from crashing
                 // TODO: this probably means the ScriptFile model is in a bad state or out of sync with the actual file; we should recover here
                 return Task.FromResult(true);
@@ -243,6 +244,8 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
         internal async Task HandleDidOpenTextDocumentNotification(
             DidOpenTextDocumentNotification openParams,
             EventContext eventContext)
+        {
+            try
             {
                 Logger.Write(LogLevel.Verbose, "HandleDidOpenTextDocumentNotification");
 
@@ -263,10 +266,20 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
 
                 await Task.WhenAll(textDocOpenTasks);
             }
+            catch (Exception ex)
+            {
+                Logger.Write(LogLevel.Error, "Unknown error " + ex.ToString());
+                // Swallow exceptions here to prevent us from crashing
+                // TODO: this probably means the ScriptFile model is in a bad state or out of sync with the actual file; we should recover here
+                return;
+            }
+        }
 
         internal async Task HandleDidCloseTextDocumentNotification(
            DidCloseTextDocumentParams closeParams,
            EventContext eventContext)
+        {
+            try
             {
                 Logger.Write(LogLevel.Verbose, "HandleDidCloseTextDocumentNotification");
 
@@ -289,6 +302,14 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
                 var textDocClosedTasks = TextDocCloseCallbacks.Select(t => t(closedFile, eventContext));
                 await Task.WhenAll(textDocClosedTasks);
             }
+            catch (Exception ex)
+            {
+                Logger.Write(LogLevel.Error, "Unknown error " + ex.ToString());
+                // Swallow exceptions here to prevent us from crashing
+                // TODO: this probably means the ScriptFile model is in a bad state or out of sync with the actual file; we should recover here
+                return;
+            }
+        }
 
         /// <summary>
         /// Handles the configuration change event
@@ -296,6 +317,8 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
         internal async Task HandleDidChangeConfigurationNotification(
             DidChangeConfigurationParams<TConfig> configChangeParams,
             EventContext eventContext)
+        {
+            try
             {
                 Logger.Write(LogLevel.Verbose, "HandleDidChangeConfigurationNotification");
 
@@ -304,6 +327,14 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
                     t => t(configChangeParams.Settings, CurrentSettings, eventContext));
                 await Task.WhenAll(configUpdateTasks);
             }
+            catch (Exception ex)
+            {
+                Logger.Write(LogLevel.Error, "Unknown error " + ex.ToString());
+                // Swallow exceptions here to prevent us from crashing
+                // TODO: this probably means the ScriptFile model is in a bad state or out of sync with the actual file; we should recover here
+                return;
+            }
+        }  
 
         #endregion
 

test/Microsoft.SqlTools.ServiceLayer.UnitTests/Workspace/WorkspaceTests.cs

@@ -82,10 +82,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Workspace
                 TextDocument = new TextDocumentItem {Uri = TestObjects.ScriptUri}
             };
             // Then:
-            // ... There should be a file not found exception thrown
+            await workspaceService.HandleDidCloseTextDocumentNotification(requestParams, eventContext);
-            // TODO: This logic should be changed to not create the ScriptFile
-            await Assert.ThrowsAnyAsync<IOException>(
-                () => workspaceService.HandleDidCloseTextDocumentNotification(requestParams, eventContext));
 
             // ... There should still be no open files
             // ... The callback should not have been called