Merge pull request #79 from open-sausages/pulls/allow-empty-response

Suppress exception in case of empty curl response
instaclick · Jun 30, 2017 · 6a6063b · 6a6063b
2 parents 3e45b65 + 59991f2
commit 6a6063b
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 22 deletions.
diff --git a/lib/WebDriver/AbstractWebDriver.php b/lib/WebDriver/AbstractWebDriver.php
@@ -128,38 +128,44 @@ protected function curl($requestMethod, $command, $parameters = null, $extraOpti
             throw WebDriverException::factory(WebDriverException::CURL_EXEC, 'Webdriver http error: ' . $info['http_code'] . ', payload :' . substr($rawResult, 0, 1000));
         }
 
-        $result = json_decode($rawResult, true);
+        $result = [];
+        $value = null;
+        if (!empty($rawResult)) {
+            $result = json_decode($rawResult, true);
 
-        if ($result === null && json_last_error() != JSON_ERROR_NONE) {
-            throw WebDriverException::factory(WebDriverException::CURL_EXEC, 'Payload received from webdriver is not valid json: ' . substr($rawResult, 0, 1000));
-        }
+            if ($result === null && json_last_error() != JSON_ERROR_NONE) {
+                throw WebDriverException::factory(WebDriverException::CURL_EXEC,
+                    'Payload received from webdriver is not valid json: ' . substr($rawResult, 0, 1000));
+            }
 
-        if (!is_array($result) || !array_key_exists('status', $result)) {
-            throw WebDriverException::factory(WebDriverException::CURL_EXEC, 'Payload received from webdriver is valid but unexpected json: ' . substr($rawResult, 0, 1000));
-        }
+            if (!is_array($result) || !array_key_exists('status', $result)) {
+                throw WebDriverException::factory(WebDriverException::CURL_EXEC,
+                    'Payload received from webdriver is valid but unexpected json: ' . substr($rawResult, 0, 1000));
+                }
 
-        $value = null;
+            if (array_key_exists('value', $result)) {
+                $value = $result['value'];
+            }
 
-        if (array_key_exists('value', $result)) {
-            $value = $result['value'];
-        }
-
-        $message = null;
+            $message = null;
 
-        if (is_array($value) && array_key_exists('message', $value)) {
-            $message = $value['message'];
-        }
+            if (is_array($value) && array_key_exists('message', $value)) {
+                $message = $value['message'];
+            }
 
-        // if not success, throw exception
-        if ((int) $result['status'] !== 0) {
-            throw WebDriverException::factory($result['status'], $message);
+            // if not success, throw exception
+            if ((int) $result['status'] !== 0) {
+                throw WebDriverException::factory($result['status'], $message);
+            }
         }
 
         $sessionId = isset($result['sessionId'])
-                   ? $result['sessionId']
-                   : (isset($value['webdriver.remote.sessionid'])
+           ? $result['sessionId']
+           : (
+               isset($value['webdriver.remote.sessionid'])
                    ? $value['webdriver.remote.sessionid']
-                   : null);
+                   : null
+           );
 
         return array(
             'value'      => $value,

diff --git a/lib/WebDriver/ServiceFactory.php b/lib/WebDriver/ServiceFactory.php
@@ -103,6 +103,11 @@ public function setServiceClass($serviceName, $className)
             $className = '\\' . $className;
         }
 
+        // Flush outdated service cache
+        if (isset($this->serviceClasses[$serviceName]) && $this->serviceClasses[$serviceName] !== $className) {
+            unset($this->services[$serviceName]);
+        }
+
         $this->serviceClasses[$serviceName] = $className;
     }
 }
diff --git a/test/Test/WebDriver/TestCurlService.php b/test/Test/WebDriver/TestCurlService.php
@@ -0,0 +1,28 @@
+<?php
+
+namespace Test\WebDriver;
+
+use WebDriver\Service\CurlServiceInterface;
+
+/**
+ * HTTP 200 response for any request:
+ * - /session returns invalid json
+ * - any other request returns empty body
+ */
+class TestCurlService implements CurlServiceInterface
+{
+    public function execute($requestMethod, $url, $parameters = null, $extraOptions = array())
+    {
+        $info = array(
+            'url' => $url,
+            'request_method' => $requestMethod,
+            'http_code' => 200,
+        );
+        if (preg_match('#.*session$#', $url)) {
+            $result = 'some invalid json';
+        } else {
+            $result = '';
+        }
+        return array($result, $info);
+    }
+}
diff --git a/test/Test/WebDriver/WebDriverTest.php b/test/Test/WebDriver/WebDriverTest.php
@@ -21,6 +21,7 @@
 
 namespace Test\WebDriver;
 
+use WebDriver\ServiceFactory;
 use WebDriver\WebDriver;
 
 /**
@@ -42,6 +43,8 @@ class WebDriverTest extends \PHPUnit_Framework_TestCase
      */
     protected function setUp()
     {
+        require_once __DIR__ . '/TestCurlService.php';
+
         if ($url = getenv('ROOT_URL')) {
             $this->testDocumentRootUrl = $url;
         }
@@ -59,6 +62,7 @@ protected function setUp()
      */
     protected function tearDown()
     {
+        ServiceFactory::getInstance()->setServiceClass('service.curl', '\\WebDriver\\Service\\CurlService');
         if ($this->session) {
             $this->session->close();
         }
@@ -221,4 +225,22 @@ public function testSeleniumNoResponse()
 
         $this->assertEquals(null, $out);
     }
+
+    /**
+     * Assert that empty response does not trigger exception, but invalid JSON does
+     */
+    public function testNonJsonResponse()
+    {
+        ServiceFactory::getInstance()->setServiceClass('service.curl', '\\Test\\WebDriver\\TestCurlService');
+        $result = $this->driver->status();
+        $this->assertNull($result);
+
+        // Test /session should error
+        $this->setExpectedException(
+            'WebDriver\Exception\CurlExec',
+            'Payload received from webdriver is not valid json: some invalid json'
+        );
+        $result = $this->driver->session();
+        $this->assertNull($result);
+    }
 }