Merge pull request #26 from juanjux/fix/empty_code_fatal

Fix/empty code fatal

Author: juanjux

native/python_package/python_driver/requestprocessor.py

@@ -4,7 +4,7 @@
 from pydetector import detector
 from traceback import format_exc
 from python_driver.version import __version__
-from typing import (Any, IO, NewType, Tuple, cast, List, Iterator, Dict)
+from typing import (Any, IO, NewType, Tuple, cast, List, Iterator, Dict, Optional)
 
 # typing.AnyStr is bugged on this version of MyPy, so...:
 AnyStr = Any
@@ -22,11 +22,11 @@
 Request    = NewType('Request',    Dict[str, Any])
 Response   = NewType('Response',   Dict[AnyStr, Any])
 
-class RequestCheckException(Exception):
+class EmptyCodeException(Exception):
     """
-    Exception produced while there is an error during the processing
-    of the request. It will cause an error reply to be produced on the
-    output buffer.
+    Exception produced when the input code is empty. This should
+    generate an error (because we can't parse anything) but not
+    a fatal one (e. g. empty __init__.py files)
     """
     pass
 
@@ -67,26 +67,6 @@ def _tostr_request(self, request: RawRequest) -> Request:
         """
         pass
 
-    def _check_input_request(self, request: RawRequest) -> Tuple[str, str]:
-        """
-        Check the incoming request package and validate that the 'content' and
-        'language' keys are not missing and that 'language' and 'language_version'
-        are the right ones for this driver. It will also call _preprare_dict
-        to covnvert the request bytestrings to str.
-
-        :param request: The incoming request, already deserialized.
-
-        .. raises::
-            RequestCheckException if the request failed to validate.
-        """
-        str_request = self._tostr_request(request)
-        code = asstr(str_request.get('content', ''))
-
-        if not code:
-            raise RequestCheckException('Bad input message, missing content')
-
-        return code, asstr(str_request.get('filepath', ''))
-
     @abc.abstractmethod
     def _send_response(self, response: Response) -> None:
         """
@@ -99,7 +79,8 @@ def _send_response(self, response: Response) -> None:
         """
         pass
 
-    def _return_error(self, filepath: AnyStr='', status: AnyStr='error') -> None:
+    def _return_error(self, filepath: AnyStr='', status: AnyStr='error',
+            ast: Optional[Dict[Any, Any]] = None) -> None:
         """
         Build and send to stdout and error response. Also log
         the errors to the python_driver.log.
@@ -109,11 +90,13 @@ def _return_error(self, filepath: AnyStr='', status: AnyStr='error') -> None:
         :param status: error type, 'error' or 'fatal'
         """
 
+        ret_ast = None if status == 'fatal' else ast
         logging.error('Filepath: {}, Errors: {}'.format(filepath, self.errors))
         response = Response({
             'status': status,
             'errors': self.errors,
             'driver': 'python23:%s' % __version__,
+            'ast': ret_ast,
         })
         if filepath:
             response['filepath'] = filepath
@@ -134,24 +117,35 @@ def process_request(self, request: RawRequest) -> None:
         self.errors = []
 
         try:
-            code, filepath = self._check_input_request(request)
-
-            # We want the code detection to be fast and we prefer Python3 AST so using
-            # the stop_on_ok_ast will avoid running a Python2 subprocess to check the
-            # AST with Python2 if the Python3 version (which is a better AST anyway) worked
-            resdict  = detector.detect(codestr=code, stop_on_ok_ast=True)
-            codeinfo = resdict['<code_string>']
-            version  = codeinfo['version']
-
-            if version in (3, 6) and codeinfo['py3ast']:
-                ast = codeinfo['py3ast']
-            elif version in (1, 2) and codeinfo['py2ast']:
-                ast = codeinfo['py2ast']
+            str_request = self._tostr_request(request)
+            code = asstr(str_request.get('content', ''))
+
+            if code:
+                # We want the code detection to be fast and we prefer Python3 AST so using
+                # the stop_on_ok_ast will avoid running a Python2 subprocess to check the
+                # AST with Python2 if the Python3 version (which is a better AST anyway) worked
+                resdict  = detector.detect(codestr=code, stop_on_ok_ast=True)
+                codeinfo = resdict['<code_string>']
+                version  = codeinfo['version']
+
+                if version in (3, 6) and codeinfo['py3ast']:
+                    ast = codeinfo['py3ast']
+                elif version in (1, 2) and codeinfo['py2ast']:
+                    ast = codeinfo['py2ast']
+                else:
+                    raise Exception('Could not determine Python version')
+
+                if not ast:
+                    raise Exception('Empty AST generated from non empty code')
             else:
-                raise Exception('Could not determine Python version')
-
-            if not ast:
-                raise Exception('Empty AST generated')
+                # Module with empty code (like __init__.py) return a module-only AST
+                # since this would still have semantic meaning for Python
+                ast = {"PY3AST": {
+                        "ast_type"   : "Module",
+                        "lineno"     : 1,
+                        "col_offset" : 1,
+                       }}
+                version = 3
 
             response = Response({
                 'status'           : 'ok',
@@ -163,11 +157,16 @@ def process_request(self, request: RawRequest) -> None:
                     'driver'           : 'python23:%s' % __version__,
                 }
             })
+
             if filepath:
                 response['filepath'] = filepath
 
             self._send_response(response)
 
+        except EmptyCodeException:
+            self.errors.append('Code field empty')
+            self._return_error(filepath, status='error', ast=ast)
+
         except:
             status = 'fatal' if ast is None else 'error'
             self.errors.append(format_exc())

native/python_package/test/test_python_driver.py

@@ -17,9 +17,7 @@
 sys.path.append('..')
 from python_driver import __version__, get_processor_instance
 from python_driver.requestprocessor import (
-    Request, Response, RequestProcessorJSON,
-    InBuffer, RequestCheckException
-)
+    Request, Response, RequestProcessorJSON, InBuffer, EmptyCodeException)
 
 if TEST_MSGPACK:
     from python_driver.requestprocessor import RequestProcessorMSGPack
@@ -251,16 +249,16 @@ def test_10_check_input(self) -> None:
             self._restart_data('json')
             brequest = convert_bytes(self.data, to_bytes=True)
             processor = RequestProcessorMSGPack(self.recvbuffer)
-            res = processor._check_input_request(brequest)
+            res = processor._parse_input_request(brequest)
             self.assertEqual(res[1], 'test.py')
 
         def test_20_check_input_bad(self) -> None:
             self._restart_data('msgpack')
             del self.data['content']
             brequest = convert_bytes(self.data, to_bytes=True)
             processor = RequestProcessorMSGPack(self.recvbuffer)
-            with self.assertRaises(RequestCheckException) as _:  # noqa: F841
-                processor._check_input_request(brequest)
+            with self.assertRaises(EmptyCodeException) as _:  # noqa: F841
+                processor._parse_input_request(brequest)
 
         def test_30_send_response_msgpack(self) -> None:
             self._restart_data('msgpack')
@@ -290,6 +288,7 @@ def test_50_return_error(self) -> None:
         self.assertDictEqual(res[0] , {'driver': 'python23:%s' % __version__,
                                        'errors': ['test error'],
                                        'filepath': 'test.py',
+                                       'ast': None,
                                        'status': 'fatal'})