Fix error handling in SimpleExecuteRequest to return actual SQL error messages (#2536)

* Collect SQL error messages for simple query execution

* Return success with empty columns for queries with no result sets

* Add integration tests for error message collection and no-result-set queries

* Revert manual loc changes

* only add to sr.strings...

* run srgen

Author: caohai

src/Microsoft.SqlTools.ServiceLayer/Localization/sr.cs

@@ -287,6 +287,14 @@ public static string QueryServiceCompletedSuccessfully
             }
         }
 
+        public static string QueryServiceQueryExecutionCompletedWithErrors
+        {
+            get
+            {
+                return Keys.GetString(Keys.QueryServiceQueryExecutionCompletedWithErrors);
+            }
+        }
+
         public static string QueryServiceColumnNull
         {
             get
@@ -11111,6 +11119,9 @@ public class Keys
             public const string QueryServiceQueryFailed = "QueryServiceQueryFailed";
 
 
+            public const string QueryServiceQueryExecutionCompletedWithErrors = "QueryServiceQueryExecutionCompletedWithErrors";
+
+
             public const string QueryServiceColumnNull = "QueryServiceColumnNull";
 
 

src/Microsoft.SqlTools.ServiceLayer/Localization/sr.resx

@@ -305,6 +305,10 @@
     <comment>.
  Parameters: 0 - message (string) </comment>
   </data>
+  <data name="QueryServiceQueryExecutionCompletedWithErrors" xml:space="preserve">
+    <value>Query execution completed with errors</value>
+    <comment></comment>
+  </data>
   <data name="QueryServiceColumnNull" xml:space="preserve">
     <value>(No column name)</value>
     <comment></comment>

src/Microsoft.SqlTools.ServiceLayer/Localization/sr.strings

@@ -126,6 +126,8 @@ QueryServiceErrorFormat(int msg, int lvl, int state, int line, string newLine, s
 
 QueryServiceQueryFailed(string message) = Query failed: {0}
 
+QueryServiceQueryExecutionCompletedWithErrors = Query execution completed with errors
+
 QueryServiceColumnNull = (No column name)
 
 QueryServiceCellNull = NULL

src/Microsoft.SqlTools.ServiceLayer/Localization/sr.xlf

@@ -7043,6 +7043,11 @@ The Query Processor estimates that implementing the following index could improv
         <target state="new">If your application is connecting to an AlwaysOn availability group (AG) on different subnets, setting this to true provides faster detection of and connection to the (currently) active server.</target>
         <note></note>
       </trans-unit>
+      <trans-unit id="QueryServiceQueryExecutionCompletedWithErrors">
+        <source>Query execution completed with errors</source>
+        <target state="new">Query execution completed with errors</target>
+        <note></note>
+      </trans-unit>
     </body>
   </file>
 </xliff>

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

@@ -323,16 +323,42 @@ internal async Task HandleSimpleExecuteRequest(SimpleExecuteParams executeParams
 
                 ResultOnlyContext<SimpleExecuteResult> newContext = new ResultOnlyContext<SimpleExecuteResult>(requestContext);
 
+                // Collect error messages during execution using thread-safe ordered collection
+                var errorMessages = new System.Collections.Concurrent.ConcurrentQueue<string>();
+
                 // handle sending event back when the query completes
                 Query.QueryAsyncEventHandler queryComplete = async query =>
                 {
                     try
                     {
-                        // check to make sure any results were recieved
+                        // Check if the batch has errors (SQL errors that didn't throw exceptions)
+                        if (query.Batches.Length > 0 && query.Batches[0].HasError)
+                        {
+                            // If we collected error messages, send those
+                            if (errorMessages.Count > 0)
+                            {
+                                await requestContext.SendError(string.Join(Environment.NewLine, errorMessages));
+                            }
+                            else
+                            {
+                                // Fallback message if we somehow didn't collect messages
+                                await requestContext.SendError(SR.QueryServiceQueryExecutionCompletedWithErrors);
+                            }
+                            return;
+                        }
+
+                        // check to make sure any results were received
                         if (query.Batches.Length == 0
                             || query.Batches[0].ResultSets.Count == 0)
                         {
-                            await requestContext.SendError(SR.QueryServiceResultSetHasNoResults);
+                            // No result sets - return empty result with no columns
+                            SimpleExecuteResult emptyResult = new SimpleExecuteResult
+                            {
+                                RowCount = 0,
+                                ColumnInfo = new DbColumnWrapper[0],
+                                Rows = new DbCellValue[0][]
+                            };
+                            await requestContext.SendResult(emptyResult);
                             return;
                         }
 
@@ -388,7 +414,26 @@ internal async Task HandleSimpleExecuteRequest(SimpleExecuteParams executeParams
                     await requestContext.SendError(e);
                 };
 
-                await InterServiceExecuteQuery(executeStringParams, newConn, newContext, null, queryCreateFailureAction, queryComplete, queryFail);
+                // Collect batch messages (errors) during query execution
+                Batch.BatchAsyncMessageHandler messageHandler = async (message) =>
+                {
+                    if (message.IsError)
+                    {
+                        errorMessages.Enqueue(message.Message);
+                    }
+                };
+
+                // Execute query with message collection
+                Query createdQuery = null;
+                Func<Query, Task<bool>> queryCreateSuccess = async (q) =>
+                {
+                    createdQuery = q;
+                    // Subscribe to batch messages to collect errors
+                    q.BatchMessageSent += messageHandler;
+                    return true;
+                };
+
+                await InterServiceExecuteQuery(executeStringParams, newConn, newContext, queryCreateSuccess, queryCreateFailureAction, queryComplete, queryFail);
             });
 
             ActiveSimpleExecuteRequests.TryAdd(randomUri, workTask);

test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/QueryExecution/QueryExecutionServiceTests.cs

@@ -165,5 +165,83 @@ private async Task<ResultSetSubset> ExecuteAndVerifyQuery(string query, string o
 
             return subsetResult.ResultSubset;
         }
+
+        /// <summary>
+        /// Test that SimpleExecuteRequest collects and returns SQL error messages for queries with errors
+        /// </summary>
+        [Test]
+        public async Task SimpleExecuteRequestReturnsErrorMessages()
+        {
+            // Establish a new connection
+            ConnectionService.Instance.OwnerToConnectionMap.Clear();
+            ConnectionInfo connectionInfo = LiveConnectionHelper.InitLiveConnectionInfo().ConnectionInfo;
+
+            var requestContext = new Mock<RequestContext<SimpleExecuteResult>>();
+            ManualResetEvent errorEvent = new ManualResetEvent(false);
+            string? errorMessage = null;
+
+            requestContext.Setup(x => x.SendError(It.IsAny<string>(), It.IsAny<int>(), It.IsAny<string>()))
+                .Callback<string, int, string>((error, code, data) =>
+                {
+                    errorMessage = error;
+                    errorEvent.Set();
+                })
+                .Returns(Task.FromResult(new object()));
+
+            var executeParams = new SimpleExecuteParams
+            {
+                OwnerUri = connectionInfo.OwnerUri,
+                QueryString = "SELECT * FROM NonExistentTable123456;"
+            };
+
+            await QueryExecutionService.Instance.HandleSimpleExecuteRequest(executeParams, requestContext.Object);
+
+            // Wait for error to be sent
+            bool gotError = errorEvent.WaitOne(TimeSpan.FromSeconds(10));
+            Assert.IsTrue(gotError, "Expected error message was not received");
+            Assert.IsNotNull(errorMessage, "Error message should not be null");
+            Assert.IsTrue(errorMessage!.Contains("NonExistentTable123456") || errorMessage!.Contains("Invalid object name"),
+                $"Error message should contain table name or invalid object error. Got: {errorMessage}");
+        }
+
+        /// <summary>
+        /// Test that SimpleExecuteRequest returns success with empty columns for queries with no result sets (UPDATE, DELETE, etc.)
+        /// </summary>
+        [Test]
+        public async Task SimpleExecuteRequestSucceedsForNoResultSetQueries()
+        {
+            // Establish a new connection
+            ConnectionService.Instance.OwnerToConnectionMap.Clear();
+            ConnectionInfo connectionInfo = LiveConnectionHelper.InitLiveConnectionInfo().ConnectionInfo;
+
+            var requestContext = new Mock<RequestContext<SimpleExecuteResult>>();
+            ManualResetEvent resultEvent = new ManualResetEvent(false);
+            SimpleExecuteResult? result = null;
+
+            requestContext.Setup(x => x.SendResult(It.IsAny<SimpleExecuteResult>()))
+                .Callback<SimpleExecuteResult>(r =>
+                {
+                    result = r;
+                    resultEvent.Set();
+                })
+                .Returns(Task.FromResult(new object()));
+
+            // Use a query that doesn't return a result set - DECLARE creates a variable
+            var executeParams = new SimpleExecuteParams
+            {
+                OwnerUri = connectionInfo.OwnerUri,
+                QueryString = "DECLARE @TestVar INT; SET @TestVar = 1;"
+            };
+
+            await QueryExecutionService.Instance.HandleSimpleExecuteRequest(executeParams, requestContext.Object);
+
+            // Wait for result to be sent
+            bool gotResult = resultEvent.WaitOne(TimeSpan.FromSeconds(10));
+            Assert.IsTrue(gotResult, "Expected result was not received");
+            Assert.IsNotNull(result, "Result should not be null");
+            Assert.AreEqual(0, result!.RowCount, "Row count should be 0 for queries without result sets");
+            Assert.IsNotNull(result!.ColumnInfo, "Column info should not be null");
+            Assert.AreEqual(0, result!.ColumnInfo.Length, "Column info should be empty for queries without result sets");
+        }
     }
 }