From d19db1b4fe27cf369a043d479e4dbc0ad8c155b8 Mon Sep 17 00:00:00 2001 From: Matt Irvine Date: Tue, 3 Apr 2018 08:47:44 -0700 Subject: [PATCH] Handle errors during execution as info messages instead of exceptions (#596) --- .../QueryExecution/Batch.cs | 70 ++++++++++++++++-- .../QueryExecution/Query.cs | 1 + .../QueryExecution/Execution/BatchTests.cs | 71 ++++++++++++++++++- 3 files changed, 134 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Batch.cs b/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Batch.cs index e16f18a1..4bf9d74f 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Batch.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Batch.cs @@ -344,16 +344,16 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution { do { - // Verify that the cancellation token hasn't benn cancelled + // Verify that the cancellation token hasn't been canceled cancellationToken.ThrowIfCancellationRequested(); - // Skip this result set if there aren't any rows (ie, UPDATE/DELETE/etc queries) + // Skip this result set if there aren't any rows (i.e. UPDATE/DELETE/etc queries) if (!reader.HasRows && reader.FieldCount == 0) { continue; } - // This resultset has results (ie, SELECT/etc queries) + // This resultset has results (i.e. SELECT/etc queries) ResultSet resultSet = new ResultSet(resultSets.Count, Id, outputFileFactory); resultSet.ResultCompletion += ResultSetCompletion; @@ -520,14 +520,70 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution /// /// Delegate handler for storing messages that are returned from the server - /// NOTE: Only messages that are below a certain severity will be returned via this - /// mechanism. Anything above that level will trigger an exception. /// /// Object that fired the event /// Arguments from the event - private void ServerMessageHandler(object sender, SqlInfoMessageEventArgs args) + private async void ServerMessageHandler(object sender, SqlInfoMessageEventArgs args) { - SendMessage(args.Message, false).Wait(); + foreach (SqlError error in args.Errors) + { + await HandleSqlErrorMessage(error.Number, error.Class, error.State, error.LineNumber, error.Procedure, error.Message); + } + } + + /// + /// Handle a single SqlError's error message by processing and displaying it. The arguments come from the error being handled + /// + internal async Task HandleSqlErrorMessage(int errorNumber, byte errorClass, byte state, int lineNumber, string procedure, string message) + { + // Did the database context change (error code 5701)? + if (errorNumber == 5701) + { + return; + } + + string detailedMessage; + if (string.IsNullOrEmpty(procedure)) + { + detailedMessage = string.Format("Msg {0}, Level {1}, State {2}, Line {3}{4}{5}", + errorNumber, errorClass, state, lineNumber + Selection.StartLine, + Environment.NewLine, message); + } + else + { + detailedMessage = string.Format("Msg {0}, Level {1}, State {2}, Procedure {3}, Line {4}{5}{6}", + errorNumber, errorClass, state, procedure, lineNumber, + Environment.NewLine, message); + } + + bool isError; + if (errorClass > 10) + { + isError = true; + } + else if (errorClass > 0 && errorNumber > 0) + { + isError = false; + } + else + { + isError = false; + detailedMessage = null; + } + + if (detailedMessage != null) + { + await SendMessage(detailedMessage, isError); + } + else + { + await SendMessage(message, isError); + } + + if (isError) + { + this.HasError = true; + } } /// diff --git a/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Query.cs b/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Query.cs index 37c81374..ad9e1a5a 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Query.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/QueryExecution/Query.cs @@ -379,6 +379,7 @@ namespace Microsoft.SqlTools.ServiceLayer.QueryExecution if (sqlConn != null) { // Subscribe to database informational messages + sqlConn.GetUnderlyingConnection().FireInfoMessageEventOnUserErrors = true; sqlConn.GetUnderlyingConnection().InfoMessage += OnInfoMessage; } diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/QueryExecution/Execution/BatchTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/QueryExecution/Execution/BatchTests.cs index 3cdbf2db..4b440ef2 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/QueryExecution/Execution/BatchTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/QueryExecution/Execution/BatchTests.cs @@ -7,6 +7,7 @@ using System; using System.Collections.Generic; using System.Data; using System.Data.Common; +using System.Data.SqlClient; using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Threading; @@ -354,7 +355,75 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.QueryExecution.Execution Assert.True(messageCalls == 1); batch.StatementCompletedHandler(null, new StatementCompletedEventArgs(2)); Assert.True(messageCalls == 2); - } + } + + [Fact] + public async Task ServerMessageHandlerShowsErrorMessages() + { + // Set up the batch to track message calls + Batch batch = new Batch(Constants.StandardQuery, Common.SubsectionDocument, Common.Ordinal, MemoryFileSystem.GetFileStreamFactory()); + int errorMessageCalls = 0; + int infoMessageCalls = 0; + string actualMessage = null; + batch.BatchMessageSent += args => + { + if (args.IsError) + { + errorMessageCalls++; + } + else + { + infoMessageCalls++; + } + actualMessage = args.Message; + return Task.CompletedTask; + }; + + // If I call the server message handler with an error message + var errorMessage = "error message"; + await batch.HandleSqlErrorMessage(1, 15, 0, 1, string.Empty, errorMessage); + + // Then one error message call should be recorded + Assert.Equal(1, errorMessageCalls); + Assert.Equal(0, infoMessageCalls); + + // And the actual message should be a formatted version of the error message + Assert.True(actualMessage.Length > errorMessage.Length); + } + + [Fact] + public async Task ServerMessageHandlerShowsInfoMessages() + { + // Set up the batch to track message calls + Batch batch = new Batch(Constants.StandardQuery, Common.SubsectionDocument, Common.Ordinal, MemoryFileSystem.GetFileStreamFactory()); + int errorMessageCalls = 0; + int infoMessageCalls = 0; + string actualMessage = null; + batch.BatchMessageSent += args => + { + if (args.IsError) + { + errorMessageCalls++; + } + else + { + infoMessageCalls++; + } + actualMessage = args.Message; + return Task.CompletedTask; + }; + + // If I call the server message handler with an info message + var infoMessage = "info message"; + await batch.HandleSqlErrorMessage(0, 0, 0, 1, string.Empty, infoMessage); + + // Then one info message call should be recorded + Assert.Equal(0, errorMessageCalls); + Assert.Equal(1, infoMessageCalls); + + // And the actual message should be the exact info message + Assert.Equal(infoMessage, actualMessage); + } private static DbConnection GetConnection(ConnectionInfo info) {