From 8e8a492189eda8b92e6964a4bcd4ac92fda2ce8e Mon Sep 17 00:00:00 2001
From: Josue Kouka <jkouka@entrouvert.com>
Date: Wed, 8 Feb 2017 16:09:51 +0100
Subject: [PATCH] improve generic timeout handling (#14913)

---
 mandayejs/mandaye/utils.py | 68 ++++++++++++++++++++++++----------------------
 tests/test_mandayejs.py    | 10 +++----
 2 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/mandayejs/mandaye/utils.py b/mandayejs/mandaye/utils.py
index 8050af9..ec87a58 100644
--- a/mandayejs/mandaye/utils.py
+++ b/mandayejs/mandaye/utils.py
@@ -29,49 +29,51 @@ from mandayejs.applications import get_app_settings
 logger = logging.getLogger(__name__)
 
 
-def exec_phantom(data, script='do_login.js'):
+def run(send_end, data, script):
+    phantom = subprocess.Popen([
+        settings.PHANTOM_JS_BINARY,
+        '--ignore-ssl-errors=yes', '--ssl-protocol=any',
+        os.path.join(settings.BASE_DIR, 'mandayejs', script)],
+        close_fds=True,
+        stdin=subprocess.PIPE,
+        stdout=subprocess.PIPE
+    )
+    stdout, stderr = phantom.communicate(json.dumps(data))
+
+    try:
+        output = re.search('<mandayejs>(.*?)</mandayejs>', stdout, re.DOTALL)
+        if not output:
+            raise ValueError
+        stdout = output.group(1)
+        result = json.loads(stdout)
+    except (ValueError,):
+        result = {"result": "json_error"}
+        logger.error("invalid json: %s" % stdout)
+
+    if result.get('stderr'):
+        logger.warning(result['stderr'])
+    if result.get('error'):
+        logger.error('Error occured: %s' % result.get('reason'))
+
+    send_end.send(result)
+    send_end.close()
 
-    def run(send_end):
-        phantom = subprocess.Popen([
-            settings.PHANTOM_JS_BINARY,
-            '--ignore-ssl-errors=yes', '--ssl-protocol=any',
-            os.path.join(settings.BASE_DIR, 'mandayejs', script)],
-            close_fds=True,
-            stdin=subprocess.PIPE,
-            stdout=subprocess.PIPE
-        )
-        stdout, stderr = phantom.communicate(json.dumps(data))
-
-        try:
-            output = re.search('<mandayejs>(.*?)</mandayejs>', stdout, re.DOTALL)
-            if not output:
-                raise ValueError
-            stdout = output.group(1)
-            result = json.loads(stdout)
-        except (ValueError,):
-            result = {"result": "json_error"}
-            logger.error("invalid json: %s" % stdout)
-
-        if result.get('stderr'):
-            logger.warning(result['stderr'])
-        if result.get('error'):
-            logger.error('Error occured: %s' % result.get('reason'))
-
-        send_end.send(result)
 
+def exec_phantom(data, script='do_login.js'):
     recv_end, send_end = multiprocessing.Pipe(False)
-    process = multiprocessing.Process(target=run, args=(send_end,))
+    process = multiprocessing.Process(target=run, args=(send_end, data, script))
     process.start()
-    process.join(settings.PHANTOM_JS_TIMEOUT)
 
-    if process.is_alive():
+    if recv_end.poll(settings.PHANTOM_JS_TIMEOUT):
+        result = recv_end.recv()
+        recv_end.close()
+    else:
         process.terminate()
+        send_end.close()
         # Don't log locators, they may contain credentials (passwords)
         context = {k: v for k, v in data.items() if k != 'locators'}
         logger.error("PhantomJS process timeout, context: %s" % context)
         result = {'result': 'timeout'}
-    else:
-        result = recv_end.recv()
 
     return result
 
diff --git a/tests/test_mandayejs.py b/tests/test_mandayejs.py
index c464340..e99ddaa 100644
--- a/tests/test_mandayejs.py
+++ b/tests/test_mandayejs.py
@@ -293,14 +293,14 @@ def test_phantom_js_errors(mocked_popen, caplog):
 
 @mock.patch('mandayejs.mandaye.utils.subprocess.Popen')
 def test_phantom_js_timeout(mocked_popen, caplog):
-    mocked_popen.return_value = MockedPopen(expected_output=json.dumps({'whatever': 'whatever'}), stall_for=True)
+    mocked_popen.return_value = MockedPopen(expected_output=(json.dumps({'whatever': 'whatever'}), None), stall_for=True)
     result = exec_phantom(LOGIN_INFO)
 
     for record in caplog.records():
-        assert record.levelname == 'ERROR'
-        assert 'https://whatever.com' in record.message
-        assert 'tenants/static/js/auth.checker.js' in record.message
-        assert 'input[type=submit], button' in record.message
+        if record.levelname == 'ERROR':
+            assert 'https://whatever.com' in record.message
+            assert 'tenants/static/js/auth.checker.js' in record.message
+            assert 'input[type=submit], button' in record.message
 
     assert result['result'] == 'timeout'
 
-- 
2.11.0