Send Error Object on SendError (#304)

This change ensures that when calling `requestContext.SendError` you are only able to supply parameters that match the language service beta protocol expected Error object. In other words, you have to provide an error message and optionally and error code.

# **BREAKING API CHANGES**
This will break displaying errors in Microsoft/vscode-mssql. I will be making changes to properly handle the error object shortly.

* Adding contract for returning Error objects as per LanguageService "protocol"

* Fixes throughout codebase to send only error message in error cases
Cleanup of CredentialServiceTest unit test class
Adding standard error handling for event flow validator

* Adding optional data field as per protocol spec
https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md

* Adding optional validation for error objects

src/Microsoft.SqlTools.Hosting/Hosting/Contracts/Error.cs

@@ -0,0 +1,29 @@
+//
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+//
+
+
+namespace Microsoft.SqlTools.Hosting.Contracts
+{
+    /// <summary>
+    /// Defines the message contract for errors returned via SendError.
+    /// </summary>
+    public class Error
+    {
+        /// <summary>
+        /// Error code. If omitted will default to 0
+        /// </summary>
+        public int Code { get; set; }
+
+        /// <summary>
+        /// Optional information to return with the error
+        /// </summary>
+        public object Data { get; set; }
+
+        /// <summary>
+        /// Error message
+        /// </summary>
+        public string Message { get; set; }
+    }
+}

src/Microsoft.SqlTools.Hosting/Hosting/Protocol/RequestContext.cs

@@ -3,8 +3,10 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 //
 
+using System;
 using System.Threading.Tasks;
 using Microsoft.SqlTools.Hosting.Protocol.Contracts;
+using Error = Microsoft.SqlTools.Hosting.Contracts.Error;
 using Newtonsoft.Json.Linq;
 
 namespace Microsoft.SqlTools.Hosting.Protocol
@@ -37,13 +39,26 @@ namespace Microsoft.SqlTools.Hosting.Protocol
                 eventParams);
         }
 
-        public virtual async Task SendError(object errorDetails)
+        public virtual Task SendError(string errorMessage, int errorCode = 0, object data = null)
         {
-            await this.messageWriter.WriteMessage(
+            // Build the error message
+            Error error = new Error
+            {
+                Message = errorMessage,
+                Code = errorCode,
+                Data = data
+            };
+            return this.messageWriter.WriteMessage(
                 Message.ResponseError(
                     requestMessage.Id,
                     requestMessage.Method,
-                    JToken.FromObject(errorDetails)));
+                    JToken.FromObject(error)));
+        }
+
+        public virtual Task SendError(Exception e)
+        {
+            // Overload to use the parameterized error handler
+            return SendError(e.Message, e.HResult, e.ToString());
         }
     }
 }

src/Microsoft.SqlTools.ServiceLayer/LanguageServices/Contracts/Definition.cs

@@ -14,16 +14,5 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices.Contracts
             RequestType<TextDocumentPosition, Location[]> Type =
             RequestType<TextDocumentPosition, Location[]>.Create("textDocument/definition");
     }
-
-    /// <summary>
-    /// Error object for Definition
-    /// </summary>
-    public class DefinitionError
-    {
-        /// <summary>
-        /// Error message 
-        /// </summary>
-        public string message { get; set; }
-    }
 }
 

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

@@ -319,7 +319,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
                 {
                     if (definitionResult.IsErrorResult)
                     {
-                        await requestContext.SendError( new DefinitionError { message = definitionResult.Message });
+                        await requestContext.SendError(definitionResult.Message);
                     }
                     else
                     {
@@ -820,52 +820,52 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
             return result;
         }
 
-        private DefinitionResult GetDefinitionFromTokenList(TextDocumentPosition textDocumentPosition, List<Token> tokenList,
+        private DefinitionResult GetDefinitionFromTokenList(TextDocumentPosition textDocumentPosition, List<Token> tokenList,
-                ScriptParseInfo scriptParseInfo, ScriptFile scriptFile, ConnectionInfo connInfo)
+                ScriptParseInfo scriptParseInfo, ScriptFile scriptFile, ConnectionInfo connInfo)
-        {
+        {
-
+
-            DefinitionResult lastResult = null;
+            DefinitionResult lastResult = null;
-            foreach (var token in tokenList)
+            foreach (var token in tokenList)
-            {
+            {
-
+
-                // Strip "[" and "]"(if present) from the token text to enable matching with the suggestions.
+                // Strip "[" and "]"(if present) from the token text to enable matching with the suggestions.
-                // The suggestion title does not contain any sql punctuation
+                // The suggestion title does not contain any sql punctuation
-                string tokenText = TextUtilities.RemoveSquareBracketSyntax(token.Text);
+                string tokenText = TextUtilities.RemoveSquareBracketSyntax(token.Text);
-                textDocumentPosition.Position.Line = token.StartLocation.LineNumber;
+                textDocumentPosition.Position.Line = token.StartLocation.LineNumber;
-                textDocumentPosition.Position.Character = token.StartLocation.ColumnNumber;
+                textDocumentPosition.Position.Character = token.StartLocation.ColumnNumber;
-                if (Monitor.TryEnter(scriptParseInfo.BuildingMetadataLock))
+                if (Monitor.TryEnter(scriptParseInfo.BuildingMetadataLock))
-                {
+                {
-                    try
+                    try
-                    {
+                    {
-                        var result = QueueTask(textDocumentPosition, scriptParseInfo, connInfo, scriptFile, tokenText);
+                        var result = QueueTask(textDocumentPosition, scriptParseInfo, connInfo, scriptFile, tokenText);
-                        lastResult = result;
+                        lastResult = result;
-                        if (!result.IsErrorResult)
+                        if (!result.IsErrorResult)
-                        {
+                        {
-                            return result;
+                            return result;
-                        }
+                        }
-                    }
+                    }
-                    catch (Exception ex)
+                    catch (Exception ex)
-                    {
+                    {
-                        // if any exceptions are raised return error result with message
+                        // if any exceptions are raised return error result with message
-                        Logger.Write(LogLevel.Error, "Exception in GetDefinition " + ex.ToString());
+                        Logger.Write(LogLevel.Error, "Exception in GetDefinition " + ex.ToString());
-                        return new DefinitionResult
+                        return new DefinitionResult
-                        {
+                        {
-                            IsErrorResult = true,
+                            IsErrorResult = true,
-                            Message = SR.PeekDefinitionError(ex.Message),
+                            Message = SR.PeekDefinitionError(ex.Message),
-                            Locations = null
+                            Locations = null
-                        };
+                        };
-                    }
+                    }
-                    finally
+                    finally
-                    {
+                    {
-                        Monitor.Exit(scriptParseInfo.BuildingMetadataLock);
+                        Monitor.Exit(scriptParseInfo.BuildingMetadataLock);
-                    }
+                    }
-                }
+                }
-                else
+                else
-                {
+                {
-                    Logger.Write(LogLevel.Error, "Timeout waiting to query metadata from server");
+                    Logger.Write(LogLevel.Error, "Timeout waiting to query metadata from server");
-                }
+                }
-            }
+            }
-            return (lastResult != null) ? lastResult : null;
+            return (lastResult != null) ? lastResult : null;
         }
 
         /// <summary>
@@ -898,36 +898,36 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
                 return null;
             }
 
-            if (scriptParseInfo.IsConnected)
+            if (scriptParseInfo.IsConnected)
-            {
+            {
-                //try children tokens first
+                //try children tokens first
-                Stack<Token> childrenTokens = selectedToken.Item1;
+                Stack<Token> childrenTokens = selectedToken.Item1;
-                List<Token> tokenList = childrenTokens.ToList();
+                List<Token> tokenList = childrenTokens.ToList();
-                DefinitionResult childrenResult = GetDefinitionFromTokenList(textDocumentPosition, tokenList, scriptParseInfo, scriptFile, connInfo);
+                DefinitionResult childrenResult = GetDefinitionFromTokenList(textDocumentPosition, tokenList, scriptParseInfo, scriptFile, connInfo);
-
+
-                // if the children peak definition returned null then 
+                // if the children peak definition returned null then 
-                // try the parents
+                // try the parents
-                if (childrenResult == null || childrenResult.IsErrorResult)
+                if (childrenResult == null || childrenResult.IsErrorResult)
-                {
+                {
-                    Queue<Token> parentTokens = selectedToken.Item2;
+                    Queue<Token> parentTokens = selectedToken.Item2;
-                    tokenList = parentTokens.ToList();
+                    tokenList = parentTokens.ToList();
-                    DefinitionResult parentResult = GetDefinitionFromTokenList(textDocumentPosition, tokenList, scriptParseInfo, scriptFile, connInfo);
+                    DefinitionResult parentResult = GetDefinitionFromTokenList(textDocumentPosition, tokenList, scriptParseInfo, scriptFile, connInfo);
-                    return (parentResult == null) ? null : parentResult;
+                    return (parentResult == null) ? null : parentResult;
-                }
+                }
-                else
+                else
-                {
+                {
-                    return childrenResult;
+                    return childrenResult;
-                }    
+                }    
-            }
+            }
-            else
+            else
-            {
+            {
-                // User is not connected.
+                // User is not connected.
-                return new DefinitionResult
+                return new DefinitionResult
-                {
+                {
-                    IsErrorResult = true,
+                    IsErrorResult = true,
-                    Message = SR.PeekDefinitionNotConnectedError,
+                    Message = SR.PeekDefinitionNotConnectedError,
-                    Locations = null
+                    Locations = null
-                };
+                };
             }
         }
    

src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Contracts/SaveResultsRequest.cs

@@ -110,17 +110,6 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution.Contracts
         public string Messages { get; set; }
     }
 
-    /// <summary>
-    /// Error object for save result 
-    /// </summary>
-    public class SaveResultRequestError
-    {
-        /// <summary>
-        /// Error message 
-        /// </summary>
-        public string message { get; set; }
-    }
-
     /// <summary>
     /// Request type to save results as CSV
     /// </summary>

src/Microsoft.SqlTools.ServiceLayer/QueryExecution/QueryExecutionService.cs

@@ -162,7 +162,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution
                 await requestContext.SendResult(new ExecuteRequestResult());
                 return true;
             };
-            Func<string, Task> queryCreateFailureAction = requestContext.SendError;
+            Func<string, Task> queryCreateFailureAction = message => requestContext.SendError(message);
 
             // Use the internal handler to launch the query
             return InterServiceExecuteQuery(executeParams, requestContext, queryCreateSuccessAction, queryCreateFailureAction, null, null);
@@ -228,7 +228,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution
         {
             // Setup action for success and failure
             Func<Task> successAction = () => requestContext.SendResult(new QueryDisposeResult());
-            Func<string, Task> failureAction = requestContext.SendError;
+            Func<string, Task> failureAction = message => requestContext.SendError(message);
 
             // Use the inter-service dispose functionality
             await InterServiceDisposeQuery(disposeParams.OwnerUri, successAction, failureAction);
@@ -559,10 +559,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution
             Query query;
             if (!ActiveQueries.TryGetValue(saveParams.OwnerUri, out query))
             {
-                await requestContext.SendError(new SaveResultRequestError
+                await requestContext.SendError(SR.QueryServiceQueryInvalidOwnerUri);
-                {
-                    message = SR.QueryServiceQueryInvalidOwnerUri
-                });
                 return;
             }
 
@@ -574,7 +571,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution
             ResultSet.SaveAsFailureAsyncEventHandler errorHandler = async (parameters, reason) =>
             {
                 string message = SR.QueryServiceSaveAsFail(Path.GetFileName(parameters.FilePath), reason);
-                await requestContext.SendError(new SaveResultRequestError { message = message });
+                await requestContext.SendError(message);
             };
 
             try

test/Microsoft.SqlTools.ServiceLayer.UnitTests/Credentials/CredentialServiceTests.cs

@@ -22,31 +22,31 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Credentials
     /// </summary>
     public class CredentialServiceTests : IDisposable
     {
-        private static readonly StoreConfig config = new StoreConfig()
+        private static readonly StoreConfig Config = new StoreConfig
         {
             CredentialFolder = ".testsecrets", 
             CredentialFile = "sqltestsecrets.json", 
             IsRelativeToUserHomeDir = true
         };
 
-        const string credentialId = "Microsoft_SqlToolsTest_TestId";
+        private const string CredentialId = "Microsoft_SqlToolsTest_TestId";
-        const string password1 = "P@ssw0rd1";
+        private const string Password1 = "P@ssw0rd1";
-        const string password2 = "2Pass2Furious";
+        private const string Password2 = "2Pass2Furious";
 
-        const string otherCredId = credentialId + "2345";
+        private const string OtherCredId = CredentialId + "2345";
-        const string otherPassword = credentialId + "2345";
+        private const string OtherPassword = CredentialId + "2345";
 
         // Test-owned credential store used to clean up before/after tests to ensure code works as expected 
         // even if previous runs stopped midway through
-        private ICredentialStore credStore;
+        private readonly ICredentialStore credStore;
-        private CredentialService service;
+        private readonly CredentialService service;
         /// <summary>
         /// Constructor called once for every test
         /// </summary>
         public CredentialServiceTests()
         {
-            credStore = CredentialService.GetStoreForOS(config);
+            credStore = CredentialService.GetStoreForOS(Config);
-            service = new CredentialService(credStore, config);
+            service = new CredentialService(credStore, Config);
             DeleteDefaultCreds();
         }
         
@@ -57,8 +57,8 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Credentials
 
         private void DeleteDefaultCreds()
         {
-            credStore.DeletePassword(credentialId);
+            credStore.DeletePassword(CredentialId);
-            credStore.DeletePassword(otherCredId);
+            credStore.DeletePassword(OtherCredId);
 
 #if !WINDOWS_ONLY_BUILD
             if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
@@ -75,43 +75,43 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Credentials
         [Fact]
         public async Task SaveCredentialThrowsIfCredentialIdMissing()
         {
-            object errorResponse = null;
+            string errorResponse = null;
-            var contextMock = RequestContextMocks.Create<bool>(null).AddErrorHandling(obj => errorResponse = obj);
+            var contextMock = RequestContextMocks.Create<bool>(null).AddErrorHandling((msg, code, obj) => errorResponse = msg);
 
             await service.HandleSaveCredentialRequest(new Credential(null), contextMock.Object);
             TestUtils.VerifyErrorSent(contextMock);
-            Assert.True(((string)errorResponse).Contains("ArgumentException"));
+            Assert.Contains("ArgumentException", errorResponse);
         }
 
         [Fact]
         public async Task SaveCredentialThrowsIfPasswordMissing()
         {
-            object errorResponse = null;
+            string errorResponse = null;
-            var contextMock = RequestContextMocks.Create<bool>(null).AddErrorHandling(obj => errorResponse = obj);
+            var contextMock = RequestContextMocks.Create<bool>(null).AddErrorHandling((msg, code, obj) => errorResponse = msg);
             
-            await service.HandleSaveCredentialRequest(new Credential(credentialId), contextMock.Object);
+            await service.HandleSaveCredentialRequest(new Credential(CredentialId), contextMock.Object);
             TestUtils.VerifyErrorSent(contextMock);
-            Assert.True(((string)errorResponse).Contains("ArgumentException"));
+            Assert.Contains("ArgumentException", errorResponse);
         }
         
         [Fact]
         public async Task SaveCredentialWorksForSingleCredential()
         {
             await TestUtils.RunAndVerify<bool>(
-                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(credentialId, password1), requestContext),
+                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(CredentialId, Password1), requestContext),
-                verify: (actual => Assert.True(actual)));
+                verify: Assert.True);
         }
 
         [Fact]
         public async Task SaveCredentialSupportsSavingCredentialMultipleTimes()
         {
             await TestUtils.RunAndVerify<bool>(
-                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(credentialId, password1), requestContext),
+                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(CredentialId, Password1), requestContext),
-                verify: (actual => Assert.True(actual)));
+                verify: Assert.True);
 
             await TestUtils.RunAndVerify<bool>(
-                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(credentialId, password1), requestContext),
+                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(CredentialId, Password1), requestContext),
-                verify: (actual => Assert.True(actual)));
+                verify: Assert.True);
         }
 
         [Fact]
@@ -119,16 +119,16 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Credentials
         {
             // Given we have saved the credential
             await TestUtils.RunAndVerify<bool>(
-                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(credentialId, password1), requestContext),
+                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(CredentialId, Password1), requestContext),
                 verify: (actual => Assert.True(actual, "Expect Credential to be saved successfully")));
 
 
             // Expect read of the credential to return the password
             await TestUtils.RunAndVerify<Credential>(
-                test: (requestContext) => service.HandleReadCredentialRequest(new Credential(credentialId, null), requestContext),
+                test: (requestContext) => service.HandleReadCredentialRequest(new Credential(CredentialId, null), requestContext),
                 verify: (actual =>
                 {
-                    Assert.Equal(password1, actual.Password);
+                    Assert.Equal(Password1, actual.Password);
                 }));
         }
 
@@ -138,25 +138,25 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Credentials
 
             // Given we have saved multiple credentials
             await TestUtils.RunAndVerify<bool>(
-                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(credentialId, password1), requestContext),
+                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(CredentialId, Password1), requestContext),
                 verify: (actual => Assert.True(actual, "Expect Credential to be saved successfully")));
             await TestUtils.RunAndVerify<bool>(
-                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(otherCredId, otherPassword), requestContext),
+                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(OtherCredId, OtherPassword), requestContext),
-                verify: (actual => Assert.True(actual, "Expect Credential to be saved successfully")));
+                verify: actual => Assert.True(actual, "Expect Credential to be saved successfully"));
 
 
             // Expect read of the credentials to return the right password
             await TestUtils.RunAndVerify<Credential>(
-                test: (requestContext) => service.HandleReadCredentialRequest(new Credential(credentialId, null), requestContext),
+                test: (requestContext) => service.HandleReadCredentialRequest(new Credential(CredentialId, null), requestContext),
                 verify: (actual =>
                 {
-                    Assert.Equal(password1, actual.Password);
+                    Assert.Equal(Password1, actual.Password);
                 }));
             await TestUtils.RunAndVerify<Credential>(
-                test: (requestContext) => service.HandleReadCredentialRequest(new Credential(otherCredId, null), requestContext),
+                test: (requestContext) => service.HandleReadCredentialRequest(new Credential(OtherCredId, null), requestContext),
                 verify: (actual =>
                 {
-                    Assert.Equal(otherPassword, actual.Password);
+                    Assert.Equal(OtherPassword, actual.Password);
                 }));
         }
 
@@ -165,53 +165,53 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Credentials
         {
             // Given we have saved twice with a different password
             await TestUtils.RunAndVerify<bool>(
-                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(credentialId, password1), requestContext),
+                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(CredentialId, Password1), requestContext),
-                verify: (actual => Assert.True(actual)));
+                verify: Assert.True);
 
             await TestUtils.RunAndVerify<bool>(
-               test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(credentialId, password2), requestContext),
+               test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(CredentialId, Password2), requestContext),
-               verify: (actual => Assert.True(actual)));
+               verify: Assert.True);
 
             // When we read the value for this credential
             // Then we expect only the last saved password to be found
             await TestUtils.RunAndVerify<Credential>(
-                test: (requestContext) => service.HandleReadCredentialRequest(new Credential(credentialId), requestContext),
+                test: (requestContext) => service.HandleReadCredentialRequest(new Credential(CredentialId), requestContext),
                 verify: (actual =>
                 {
-                    Assert.Equal(password2, actual.Password);
+                    Assert.Equal(Password2, actual.Password);
                 }));
         }
 
         [Fact]
         public async Task ReadCredentialThrowsIfCredentialIsNull()
         {
-            object errorResponse = null;
+            string errorResponse = null;
-            var contextMock = RequestContextMocks.Create<Credential>(null).AddErrorHandling(obj => errorResponse = obj);
+            var contextMock = RequestContextMocks.Create<Credential>(null).AddErrorHandling((msg, code, obj) => errorResponse = msg);
 
             // Verify throws on null, and this is sent as an error
             await service.HandleReadCredentialRequest(null, contextMock.Object);
             TestUtils.VerifyErrorSent(contextMock);
-            Assert.True(((string)errorResponse).Contains("ArgumentNullException"));            
+            Assert.Contains("ArgumentNullException", errorResponse);
         }
         
         [Fact]
         public async Task ReadCredentialThrowsIfIdMissing()
         {
-            object errorResponse = null;
+            string errorResponse = null;
-            var contextMock = RequestContextMocks.Create<Credential>(null).AddErrorHandling(obj => errorResponse = obj);
+            var contextMock = RequestContextMocks.Create<Credential>(null).AddErrorHandling((msg, code, obj) => errorResponse = msg);
 
             // Verify throws with no ID
             await service.HandleReadCredentialRequest(new Credential(), contextMock.Object);
             TestUtils.VerifyErrorSent(contextMock);
-            Assert.True(((string)errorResponse).Contains("ArgumentException"));
+            Assert.Contains("ArgumentException", errorResponse);
         }
 
         [Fact]
         public async Task ReadCredentialReturnsNullPasswordForMissingCredential()
         {
             // Given a credential whose password doesn't exist
-            string credWithNoPassword = "Microsoft_SqlTools_CredThatDoesNotExist";
+            const string credWithNoPassword = "Microsoft_SqlTools_CredThatDoesNotExist";
-            
+
             // When reading the credential
             // Then expect the credential to be returned but password left blank
             await TestUtils.RunAndVerify<Credential>(
@@ -228,7 +228,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Credentials
         public async Task DeleteCredentialThrowsIfIdMissing()
         {
             object errorResponse = null;
-            var contextMock = RequestContextMocks.Create<bool>(null).AddErrorHandling(obj => errorResponse = obj);
+            var contextMock = RequestContextMocks.Create<bool>(null).AddErrorHandling((msg, code, obj) => errorResponse = msg);
 
             // Verify throws with no ID
             await service.HandleDeleteCredentialRequest(new Credential(), contextMock.Object);
@@ -241,18 +241,18 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Credentials
         {
             // Save should be true
             await TestUtils.RunAndVerify<bool>(
-                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(credentialId, password1), requestContext),
+                test: (requestContext) => service.HandleSaveCredentialRequest(new Credential(CredentialId, Password1), requestContext),
-                verify: (actual => Assert.True(actual)));
+                verify: Assert.True);
 
             // Then delete - should return true
             await TestUtils.RunAndVerify<bool>(
-                test: (requestContext) => service.HandleDeleteCredentialRequest(new Credential(credentialId), requestContext),
+                test: (requestContext) => service.HandleDeleteCredentialRequest(new Credential(CredentialId), requestContext),
-                verify: (actual => Assert.True(actual)));
+                verify: Assert.True);
 
             // Then delete - should return false as no longer exists
             await TestUtils.RunAndVerify<bool>(
-               test: (requestContext) => service.HandleDeleteCredentialRequest(new Credential(credentialId), requestContext),
+               test: (requestContext) => service.HandleDeleteCredentialRequest(new Credential(CredentialId), requestContext),
-               verify: (actual => Assert.False(actual)));
+               verify: Assert.False);
         }
 
     }

test/Microsoft.SqlTools.ServiceLayer.UnitTests/EditData/ServiceIntegrationTests.cs

@@ -265,7 +265,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.EditData
 
             // ... And I initialize an edit session with that
             var efv = new EventFlowValidator<EditInitializeResult>()
-                .AddErrorValidation<string>(Assert.NotNull)
+                .AddStandardErrorValidation()
                 .Complete();
             await eds.HandleInitializeRequest(initParams, efv.Object);
 
@@ -287,16 +287,4 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.EditData
             return s;
         }
     }
-
-    public static class EditServiceEventFlowValidatorExtensions
-    {
-        public static EventFlowValidator<T> AddStandardErrorValidation<T>(this EventFlowValidator<T> evf)
-        {
-            return evf.AddErrorValidation<string>(p =>
-            {
-                Assert.NotNull(p);
-                Assert.NotEmpty(p);
-            });
-        }
-    }
 }

test/Microsoft.SqlTools.ServiceLayer.UnitTests/Formatter/TSqlFormatterServiceTests.cs

@@ -154,13 +154,13 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Formatter
         public static void VerifyResult<T>(Mock<RequestContext<T>> contextMock, Action verify)
         {
             contextMock.Verify(c => c.SendResult(It.IsAny<T>()), Times.Once);
-            contextMock.Verify(c => c.SendError(It.IsAny<string>()), Times.Never);
+            contextMock.Verify(c => c.SendError(It.IsAny<string>(), It.IsAny<int>(), It.IsAny<object>()), Times.Never);
             verify();
         }
 
         private static void AssertFormattingEqual(string expected, string actual)
         {
-            if (string.Compare(expected, actual) != 0)
+            if (expected != actual)
             {
                 StringBuilder error = new StringBuilder();
                 error.AppendLine("======================");

test/Microsoft.SqlTools.ServiceLayer.UnitTests/LanguageServer/PeekDefinitionTests.cs

@@ -93,7 +93,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.LanguageServer
             requestContext = new Mock<RequestContext<Location[]>>();
             requestContext.Setup(rc => rc.SendResult(It.IsAny<Location[]>()))
                 .Returns(Task.FromResult(0));
-            requestContext.Setup(rc => rc.SendError(It.IsAny<DefinitionError>())).Returns(Task.FromResult(0));;
+            requestContext.Setup(rc => rc.SendError(It.IsAny<string>(), It.IsAny<int>(), It.IsAny<object>())).Returns(Task.FromResult(0));;
             requestContext.Setup(r => r.SendEvent(It.IsAny<EventType<TelemetryParams>>(), It.IsAny<TelemetryParams>())).Returns(Task.FromResult(0));;
             requestContext.Setup(r => r.SendEvent(It.IsAny<EventType<StatusChangeParams>>(), It.IsAny<StatusChangeParams>())).Returns(Task.FromResult(0));;
 
@@ -129,7 +129,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.LanguageServer
             await definitionTask;
             // verify that send result was not called and send error was called
             requestContext.Verify(m => m.SendResult(It.IsAny<Location[]>()), Times.Never());
-            requestContext.Verify(m => m.SendError(It.IsAny<DefinitionError>()), Times.Once());
+            requestContext.Verify(m => m.SendError(It.IsAny<string>(), It.IsAny<int>(), It.IsAny<object>()), Times.Once());
         }
 
         /// <summary>

test/Microsoft.SqlTools.ServiceLayer.UnitTests/QueryExecution/DisposeTests.cs

@@ -68,7 +68,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution
             var disposeParams = new QueryDisposeParams {OwnerUri = Constants.OwnerUri};
 
             var disposeRequest = new EventFlowValidator<QueryDisposeResult>()
-                .AddErrorValidation<string>(Assert.NotEmpty)
+                .AddStandardErrorValidation()
                 .Complete();
             await queryService.HandleDisposeRequest(disposeParams, disposeRequest.Object);
 

test/Microsoft.SqlTools.ServiceLayer.UnitTests/QueryExecution/Execution/ServiceIntegrationTests.cs

@@ -311,7 +311,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution.Execution
             var queryParams = new ExecuteDocumentSelectionParams { OwnerUri = "notConnected", QuerySelection = Common.WholeDocument };
 
             var efv = new EventFlowValidator<ExecuteRequestResult>()
-                .AddErrorValidation<string>(Assert.NotEmpty)
+                .AddStandardErrorValidation()
                 .Complete();
             await Common.AwaitExecution(queryService, queryParams, efv.Object);
 
@@ -339,7 +339,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution.Execution
             // ... And then I request another query without waiting for the first to complete
             queryService.ActiveQueries[Constants.OwnerUri].HasExecuted = false; // Simulate query hasn't finished
             var efv = new EventFlowValidator<ExecuteRequestResult>()
-                .AddErrorValidation<string>(Assert.NotEmpty)
+                .AddStandardErrorValidation()
                 .Complete();
             await Common.AwaitExecution(queryService, queryParams, efv.Object);
 
@@ -395,7 +395,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution.Execution
             var queryParams = new ExecuteDocumentSelectionParams { OwnerUri = Constants.OwnerUri, QuerySelection = null};
 
             var efv = new EventFlowValidator<ExecuteRequestResult>()
-                .AddErrorValidation<string>(Assert.NotEmpty)
+                .AddStandardErrorValidation()
                 .Complete();
             await queryService.HandleExecuteRequest(queryParams, efv.Object);
 

test/Microsoft.SqlTools.ServiceLayer.UnitTests/QueryExecution/ExecutionPlanTests.cs

@@ -175,7 +175,8 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution
             var queryService = Common.GetPrimedExecutionService(null, true, false, workspaceService);
             var executionPlanParams = new QueryExecutionPlanParams { OwnerUri = Constants.OwnerUri, ResultSetIndex = 0, BatchIndex = 0 };
             var executionPlanRequest = new EventFlowValidator<QueryExecutionPlanResult>()
-                .AddErrorValidation<string>(Assert.NotNull).Complete();
+                .AddStandardErrorValidation()
+                .Complete();
             await queryService.HandleExecutionPlanRequest(executionPlanParams, executionPlanRequest.Object);
             executionPlanRequest.Validate();
         }
@@ -205,7 +206,8 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution
             // ... And I then ask for a valid execution plan from it 
             var executionPlanParams = new QueryExecutionPlanParams { OwnerUri = Constants.OwnerUri, ResultSetIndex = 0, BatchIndex = 0 };
             var executionPlanRequest = new EventFlowValidator<QueryExecutionPlanResult>()
-                .AddErrorValidation<string>(Assert.NotNull).Complete();
+                .AddStandardErrorValidation()
+                .Complete();
             await queryService.HandleExecutionPlanRequest(executionPlanParams, executionPlanRequest.Object);
             executionPlanRequest.Validate();
         }
@@ -234,7 +236,8 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution
             // ... And I then ask for an execution plan from a result set 
             var executionPlanParams = new QueryExecutionPlanParams { OwnerUri = Constants.OwnerUri, ResultSetIndex = 0, BatchIndex = 0 };
             var executionPlanRequest = new EventFlowValidator<QueryExecutionPlanResult>()
-                .AddErrorValidation<string>(Assert.NotNull).Complete();
+                .AddStandardErrorValidation()
+                .Complete();
             await queryService.HandleExecutionPlanRequest(executionPlanParams, executionPlanRequest.Object);
             executionPlanRequest.Validate();
         }

test/Microsoft.SqlTools.ServiceLayer.UnitTests/QueryExecution/SaveResults/ServiceIntegrationTests.cs

@@ -36,7 +36,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution.SaveResults
                 OwnerUri = Constants.OwnerUri  // Won't exist because nothing has executed
             };
             var evf = new EventFlowValidator<SaveResultRequestResult>()
-                .AddStandardErrorValidator()
+                .AddStandardErrorValidation()
                 .Complete();
             await qes.HandleSaveResultsAsCsvRequest(saveParams, evf.Object);
 
@@ -75,7 +75,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution.SaveResults
             };
             qes.CsvFileFactory = GetCsvStreamFactory(storage, saveParams);
             var efv = new EventFlowValidator<SaveResultRequestResult>()
-                .AddStandardErrorValidator()
+                .AddStandardErrorValidation()
                 .Complete();
 
 
@@ -149,7 +149,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution.SaveResults
                 OwnerUri = Constants.OwnerUri  // Won't exist because nothing has executed
             };
             var efv = new EventFlowValidator<SaveResultRequestResult>()
-                .AddStandardErrorValidator()
+                .AddStandardErrorValidation()
                 .Complete();
             await qes.HandleSaveResultsAsJsonRequest(saveParams, efv.Object);
 
@@ -188,7 +188,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution.SaveResults
             };
             qes.JsonFileFactory = GetJsonStreamFactory(storage, saveParams);
             var efv = new EventFlowValidator<SaveResultRequestResult>()
-                .AddStandardErrorValidator()
+                .AddStandardErrorValidation()
                 .Complete();
             await qes.HandleSaveResultsAsJsonRequest(saveParams, efv.Object);
             await qes.ActiveQueries[saveParams.OwnerUri]
@@ -280,16 +280,6 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution.SaveResults
 
     public static class SaveResultEventFlowValidatorExtensions
     {
-        public static EventFlowValidator<SaveResultRequestResult> AddStandardErrorValidator(
-            this EventFlowValidator<SaveResultRequestResult> efv)
-        {
-            return efv.AddErrorValidation<SaveResultRequestError>(e =>
-            {
-                Assert.NotNull(e);
-                Assert.NotNull(e.message);
-            });
-        }
-
         public static EventFlowValidator<SaveResultRequestResult> AddStandardResultValidator(
             this EventFlowValidator<SaveResultRequestResult> efv)
         {

test/Microsoft.SqlTools.ServiceLayer.UnitTests/QueryExecution/SubsetTests.cs

@@ -158,7 +158,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution
             var queryService = Common.GetPrimedExecutionService(null, true, false, workspaceService);
             var subsetParams = new SubsetParams { OwnerUri = Constants.OwnerUri, RowsCount = 1, ResultSetIndex = 0, RowsStartIndex = 0 };
             var subsetRequest = new EventFlowValidator<SubsetResult>()
-                .AddErrorValidation<string>(Assert.NotEmpty)
+                .AddStandardErrorValidation()
                 .Complete();
             await queryService.HandleResultSubsetRequest(subsetParams, subsetRequest.Object);
             subsetRequest.Validate();
@@ -180,7 +180,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution
             // ... And I then ask for a valid set of results from it
             var subsetParams = new SubsetParams { OwnerUri = Constants.OwnerUri, RowsCount = 1, ResultSetIndex = 0, RowsStartIndex = 0 };
             var subsetRequest = new EventFlowValidator<SubsetResult>()
-                .AddErrorValidation<string>(Assert.NotEmpty)
+                .AddStandardErrorValidation()
                 .Complete();
             await queryService.HandleResultSubsetRequest(subsetParams, subsetRequest.Object);
             subsetRequest.Validate();
@@ -201,7 +201,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution
             // ... And I then ask for a set of results from it
             var subsetParams = new SubsetParams { OwnerUri = Constants.OwnerUri, RowsCount = 1, ResultSetIndex = 0, RowsStartIndex = 0 };
             var subsetRequest = new EventFlowValidator<SubsetResult>()
-                .AddErrorValidation<string>(Assert.NotEmpty)
+                .AddStandardErrorValidation()
                 .Complete();
             await queryService.HandleResultSubsetRequest(subsetParams, subsetRequest.Object);
             subsetRequest.Validate();

test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/EventFlowValidator.cs

@@ -6,6 +6,7 @@
 using System;
 using System.Collections.Generic;
 using System.Threading.Tasks;
+using Microsoft.SqlTools.Hosting.Contracts;
 using Microsoft.SqlTools.Hosting.Protocol;
 using Microsoft.SqlTools.Hosting.Protocol.Contracts;
 using Moq;
@@ -25,10 +26,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Utility
             requestContext = new Mock<RequestContext<TRequestContext>>(MockBehavior.Strict);
         }
 
-        public RequestContext<TRequestContext> Object
+        public RequestContext<TRequestContext> Object => requestContext.Object;
-        {
-            get { return requestContext.Object; }
-        }
 
         public EventFlowValidator<TRequestContext> AddEventValidation<TParams>(EventType<TParams> expectedEvent, Action<TParams> paramValidation)
         {
@@ -66,19 +64,60 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Utility
             return this;
         }
 
-        public EventFlowValidator<TRequestContext> AddErrorValidation<TParams>(Action<TParams> paramValidation)
+        public EventFlowValidator<TRequestContext> AddCompleteErrorValidation<TErrorObj>(Action<string, int> paramValidation,
+            Action<TErrorObj> dataValidation)
         {
+            // Put together a validator that checks for null and adds the provided validators
+            Action<Error> validator = e =>
+            {
+                Assert.NotNull(e);
+                paramValidation(e.Message, e.Code);
+
+                Assert.IsType<TErrorObj>(e.Data);
+                dataValidation((TErrorObj) e.Data);
+            };
+
+            // Add the expected error
+            expectedEvents.Add(new ExpectedEvent
+            {
+                EventType = EventTypes.Error,
+                ParamType = typeof(Error),
+                Validator = validator
+            });
+
+            return this;
+        }
+
+        public EventFlowValidator<TRequestContext> AddSimpleErrorValidation(Action<string, int> paramValidation)
+        {
+            // Put together a validator that ensures a null data
+            Action<Error> validator = e =>
+            {
+                Assert.NotNull(e);
+                Assert.Null(e.Data);
+                paramValidation(e.Message, e.Code);
+            };
+
             // Add the expected result
             expectedEvents.Add(new ExpectedEvent
             {
                 EventType = EventTypes.Error,
-                ParamType = typeof(TParams),
+                ParamType = typeof(Error),
-                Validator = paramValidation
+                Validator = validator
             });
 
             return this;
         }
 
+        public EventFlowValidator<TRequestContext> AddStandardErrorValidation()
+        {
+            // Add an error validator that just ensures a non-empty error message and null data obj
+            return AddSimpleErrorValidation((msg, code) =>
+            {
+                Assert.NotEmpty(msg);
+            });
+        }
+
         public EventFlowValidator<TRequestContext> Complete()
         {
             // Add general handler for result handling
@@ -91,11 +130,11 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Utility
                 .Returns(Task.FromResult(0));
 
             // Add general handler for error event
-            requestContext.AddErrorHandling(o =>
+            requestContext.AddErrorHandling((msg, code, obj) =>
             {
                 receivedEvents.Add(new ReceivedEvent
                 {
-                    EventObject = o,
+                    EventObject = new Error {Message = msg, Code = code},
                     EventType = EventTypes.Error
                 });
             });

test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/RequestContextMocks.cs

@@ -48,29 +48,17 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Utility
 
         public static Mock<RequestContext<TResponse>> AddErrorHandling<TResponse>(
             this Mock<RequestContext<TResponse>> mock,
-            Action<object> errorCallback)
+            Action<string, int, object> errorCallback)
         {
             // Setup the mock for SendError
-            var sendErrorFlow = mock.Setup(rc => rc.SendError(It.IsAny<object>()))
+            var sendErrorFlow = mock.Setup(rc => rc.SendError(It.IsAny<string>(), It.IsAny<int>(), It.IsAny<object>()))
                 .Returns(Task.FromResult(0));
             if (errorCallback != null)
             {
-                sendErrorFlow.Callback(errorCallback);
+                sendErrorFlow.Callback<string, int, object>(errorCallback);
             }
 
             return mock;
         }
-
-        public static Mock<RequestContext<TResponse>> SetupRequestContextMock<TResponse, TParams>(
-            Action<TResponse> resultCallback,
-            EventType<TParams> expectedEvent,
-            Action<EventType<TParams>, TParams> eventCallback,
-            Action<object> errorCallback)
-        {
-            return Create(resultCallback)
-                .AddEventHandling(expectedEvent, eventCallback)
-                .AddErrorHandling(errorCallback);            
-        }
-
     }
 }

test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/TestUtils.cs

@@ -40,20 +40,20 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Utility
         public static void VerifyErrorSent<T>(Mock<RequestContext<T>> contextMock)
         {
             contextMock.Verify(c => c.SendResult(It.IsAny<T>()), Times.Never);
-            contextMock.Verify(c => c.SendError(It.IsAny<string>()), Times.Once);
+            contextMock.Verify(c => c.SendError(It.IsAny<string>(), It.IsAny<int>(), It.IsAny<object>()), Times.Once);
         }
 
         public static void VerifyResult<T, U>(Mock<RequestContext<T>> contextMock, U expected, U actual)
         {
             contextMock.Verify(c => c.SendResult(It.IsAny<T>()), Times.Once);
             Assert.Equal(expected, actual);
-            contextMock.Verify(c => c.SendError(It.IsAny<string>()), Times.Never);
+            contextMock.Verify(c => c.SendError(It.IsAny<string>(), It.IsAny<int>(), It.IsAny<object>()), Times.Never);
         }
 
         public static void VerifyResult<T>(Mock<RequestContext<T>> contextMock, Action<T> verify, T actual)
         {
             contextMock.Verify(c => c.SendResult(It.IsAny<T>()), Times.Once);
-            contextMock.Verify(c => c.SendError(It.IsAny<string>()), Times.Never);
+            contextMock.Verify(c => c.SendError(It.IsAny<string>(), It.IsAny<int>(), It.IsAny<object>()), Times.Never);
             verify(actual);
         }