Fix for malformed query leading to CLI hanging. (dbcli#140)

* Fixing queryexecution service to handle error during query execution.

* Fixing format of for statement.

* Fixing pep8 space after comma.

* Adding 2 second sleep after connection.

* Adding regression test.

* Updating baseline.

* Fixing baseline.

Author: MrMeemus

mssqlcli/jsonrpc/contracts/queryexecutestringservice.py

@@ -30,7 +30,8 @@ def get_response(self):
             if response:
                 decoded_response = self.decode_response(response)
 
-            if isinstance(decoded_response, QueryCompleteEvent):
+            if isinstance(decoded_response, QueryCompleteEvent) or \
+               isinstance(decoded_response, QueryExecuteErrorResponseEvent):
                 self.finished = True
                 self.json_rpc_client.request_finished(self.id)
 
@@ -77,7 +78,7 @@ def format(self):
 class QueryExecuteErrorResponseEvent(object):
     def __init__(self, parameters):
         inner_params = parameters[u'error']
-        self.error_message = inner_params[u'message'] if u'message' in inner_params else ''
+        self.exception_message = inner_params[u'message'] if u'message' in inner_params else ''
         self.error_code = inner_params[u'code'] if u'code' in inner_params else ''
 
 

mssqlcli/jsonrpc/contracts/tests/baselines/malformed_query_adventureworks2014.txt

@@ -0,0 +1,7 @@
+Content-Length: 40
+
+{"jsonrpc":"2.0","id":"1","result":true}Content-Length: 645
+
+{"jsonrpc":"2.0","method":"connection/complete","params":{"ownerUri":"connectionservicetest","connectionId":"6fd410b5-cd48-487a-bc9c-90569e749772","messages":null,"errorMessage":null,"errorNumber":0,"serverInfo":{"serverMajorVersion":12,"serverMinorVersion":0,"serverReleaseVersion":2269,"engineEditionId":3,"serverVersion":"12.0.2269.0","serverLevel":"RTM","serverEdition":"Enterprise Edition (64-bit)","isCloud":false,"azureVersion":0,"osVersion":"Windows NT 6.3 <X64> (Build 9600: ) (Hypervisor)\n","machineName":"BRO-HB"},"connectionSummary":{"serverName":"bro-hb","databaseName":"AdventureWorks2014","userName":"cloudsa"},"type":"Default"}}Content-Length: 205
+
+{"jsonrpc":"2.0","id":"2","error":{"code":0,"message":"SQL Execution error: A fatal error occurred.\r\n\tIncorrect syntax was encountered while select * from [HumanResources.Department was being parsed."}}

mssqlcli/jsonrpc/contracts/tests/generatequerybaseline.py

@@ -34,6 +34,8 @@ def generate_query_baseline(file_name):
         writer = json_rpc_client.JsonRpcWriter(tools_service_process.stdin)
         writer.send_request(u'connection/connect', parameters, id=1)
 
+        time.sleep(2)
+
         parameters = {u'OwnerUri': u'connectionservicetest',
                       u'Query': "select * from HumanResources.Department"}
         writer.send_request(u'query/executeString', parameters, id=2)

mssqlcli/jsonrpc/contracts/tests/test_executestring.py

@@ -32,16 +32,50 @@ def test_query_execute_response_AdventureWorks2014(self):
 
             request = queryservice.QueryExecuteStringRequest(
                 2, rpc_client, parameters)
-            self.verify_query_response(request=request)
+            self.verify_query_response(request=request,
+                                       expected_message_event=1,
+                                       expected_complete_event=1,
+                                       expected_batch_summaries=1,
+                                       expected_result_set_summaries=1,
+                                       expected_row_count=16)
             rpc_client.shutdown()
 
-    def verify_query_response(self, request):
+    def test_malformed_query_AdventureWorks2014(self):
+        """
+            Verify a failed query execute response for "select * from [HumanResources.Department"
+        """
+
+        with open(self.get_test_baseline(u'malformed_query_adventureworks2014.txt'), u'r+b', buffering=0) as response_file:
+            request_stream = io.BytesIO()
+            rpc_client = json_rpc_client.JsonRpcClient(
+                request_stream, response_file)
+            rpc_client.start()
+            # Submit a dummy request.
+            parameters = {u'OwnerUri': u'connectionservicetest',
+                          u'Query': "select * from [HumanResources.Department"}
+
+            request = queryservice.QueryExecuteStringRequest(
+                2, rpc_client, parameters)
+            self.verify_query_response(request=request,
+                                       expected_complete_event=1,
+                                       expected_error_count=1)
+            rpc_client.shutdown()
+
+    def verify_query_response(self,
+                              request,
+                              expected_message_event=0,
+                              expected_complete_event=0,
+                              expected_batch_summaries=0,
+                              expected_result_set_summaries=0,
+                              expected_row_count=0,
+                              expected_error_count=0):
         message_event = 0
         complete_event = 0
         batch_summaries = 0
         result_set_summaries = 0
-        # For the executed query expected row count is 16
         row_count = 0
+        error_count = 0
+
         request.execute()
         time.sleep(1)
 
@@ -50,18 +84,22 @@ def verify_query_response(self, request):
             if response:
                 if isinstance(response, queryservice.QueryMessageEvent):
                     message_event += 1
+                elif isinstance(response, queryservice.QueryExecuteErrorResponseEvent):
+                    error_count += 1
                 elif isinstance(response, queryservice.QueryCompleteEvent):
                     complete_event += 1
-                    batch_summaries = len(response.batch_summaries)
-                    result_set_summaries = len(
-                        response.batch_summaries[0].result_set_summaries)
-                    row_count = response.batch_summaries[0].result_set_summaries[0].row_count
+                    if hasattr(response, 'batch_summaries'):
+                        batch_summaries = len(response.batch_summaries)
+                        result_set_summaries = len(
+                            response.batch_summaries[0].result_set_summaries)
+                        row_count = response.batch_summaries[0].result_set_summaries[0].row_count
 
-        self.assertEqual(message_event, 1)
-        self.assertEqual(complete_event, 1)
-        self.assertEqual(batch_summaries, 1)
-        self.assertEqual(result_set_summaries, 1)
-        self.assertEqual(row_count, 16)
+        self.assertEqual(message_event, expected_message_event)
+        self.assertEqual(complete_event, expected_complete_event)
+        self.assertEqual(batch_summaries, expected_batch_summaries)
+        self.assertEqual(result_set_summaries, expected_result_set_summaries)
+        self.assertEqual(row_count, expected_row_count)
+        self.assertEqual(error_count, expected_error_count)
 
     def get_test_baseline(self, file_name):
         """

mssqlcli/main.py

@@ -312,7 +312,7 @@ def connect(self, database='', server='', user='', port='', passwd='',
                                                                  connection_timeout=connection_timeout,
                                                                  application_intent=application_intent,
                                                                  multi_subnet_failover=multi_subnet_failover,
-                                                                 packet_size=packet_size,**kwargs)
+                                                                 packet_size=packet_size, **kwargs)
 
             if not self.mssqlcliclient_query_execution.connect():
                 click.secho(
@@ -524,8 +524,8 @@ def _evaluate_command(self, text):
             click.secho(u'No connection to server. Exiting.')
             exit(1)
 
-        for rows, columns, status, sql, is_error in self.mssqlcliclient_query_execution.execute_multi_statement_single_batch(
-                text):
+        for rows, columns, status, sql, is_error in \
+                self.mssqlcliclient_query_execution.execute_multi_statement_single_batch(text):
             total = time() - start
             if self._should_show_limit_prompt(status, rows):
                 click.secho('The result set has more than %s rows.'
@@ -569,7 +569,7 @@ def _evaluate_command(self, text):
                 meta_changed = meta_changed or has_meta_cmd(text)
 
         return output, MetaQuery(
-            sql, all_success, total, meta_changed, db_changed, path_changed, mutated)
+            text, all_success, total, meta_changed, db_changed, path_changed, mutated)
 
     def _handle_server_closed_connection(self):
         """Used during CLI execution"""

mssqlcli/mssqlcliclient.py

@@ -132,8 +132,8 @@ def execute_multi_statement_single_batch(self, query):
                 if not sql:
                     yield None, None, None, sql, False
                     continue
-                for rows, columns, status, sql, is_error in self.execute_single_batch_query(sql):
-                    yield rows, columns, status, sql, is_error
+                for rows, columns, status, statement, is_error in self.execute_single_batch_query(sql):
+                    yield rows, columns, status, statement, is_error
 
     def execute_single_batch_query(self, query):
         if not self.is_connected:
@@ -150,18 +150,14 @@ def execute_single_batch_query(self, query):
         while not query_request.completed():
             query_response = query_request.get_response()
             if query_response:
-                if isinstance(query_response, queryservice.QueryExecuteErrorResponseEvent):
-                    yield self.tabular_results_generator(column_info=None, result_rows=None,
-                                                          query=query, message=query_response.error_message,
-                                                          is_error=True)
-                elif isinstance(query_response, queryservice.QueryMessageEvent):
+                if isinstance(query_response, queryservice.QueryMessageEvent):
                     query_messages.append(query_response)
             else:
                 sleep(time_wait_if_no_response)
 
         if query_response.exception_message:
             logger.error(u'Query response had an exception')
-            yield self.tabular_results_generator(
+            return self.tabular_results_generator(
                 column_info=None,
                 result_rows=None,
                 query=query,