Skip to content

Commit 6fe9517

Browse files
robocoderrobocoder
committed
tweak handling of empty response
1 parent 6a6063b commit 6fe9517

File tree

5 files changed

+71
-72
lines changed

5 files changed

+71
-72
lines changed

‎lib/WebDriver/AbstractWebDriver.php‎

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -122,49 +122,46 @@ protected function curl($requestMethod, $command, $parameters = null, $extraOpti
122122

123123
list($rawResult, $info) = ServiceFactory::getInstance()->getService('service.curl')->execute($requestMethod, $url, $parameters, $extraOptions);
124124

125-
// According to https://w3c.github.io/webdriver/webdriver-spec.html all 4xx responses are to be considered an error and return plaintext,
126-
// while 5xx responses are json encoded
127-
if (substr($info['http_code'], 0, 1) === '4') {
128-
throw WebDriverException::factory(WebDriverException::CURL_EXEC, 'Webdriver http error: ' . $info['http_code'] . ', payload :' . substr($rawResult, 0, 1000));
129-
}
130-
131-
$result = [];
132-
$value = null;
133-
if (!empty($rawResult)) {
134-
$result = json_decode($rawResult, true);
125+
$httpCode = $info['http_code'];
135126

136-
if ($result === null && json_last_error() != JSON_ERROR_NONE) {
137-
throw WebDriverException::factory(WebDriverException::CURL_EXEC,
138-
'Payload received from webdriver is not valid json: ' . substr($rawResult, 0, 1000));
139-
}
127+
// According to https://w3c.github.io/webdriver/webdriver-spec.html all 4xx responses are to be considered
128+
// an error and return plaintext, while 5xx responses are json encoded
129+
if ($httpCode >= 400 && $httpCode <= 499) {
130+
throw WebDriverException::factory(
131+
WebDriverException::CURL_EXEC,
132+
'Webdriver http error: ' . $httpCode . ', payload :' . substr($rawResult, 0, 1000)
133+
);
134+
}
140135

141-
if (!is_array($result) || !array_key_exists('status', $result)) {
142-
throw WebDriverException::factory(WebDriverException::CURL_EXEC,
143-
'Payload received from webdriver is valid but unexpected json: ' . substr($rawResult, 0, 1000));
144-
}
136+
$result = json_decode($rawResult, true);
145137

146-
if (array_key_exists('value', $result)) {
147-
$value = $result['value'];
148-
}
138+
if ($result === null && json_last_error() != JSON_ERROR_NONE) {
139+
throw WebDriverException::factory(
140+
WebDriverException::CURL_EXEC,
141+
'Payload received from webdriver is not valid json: ' . substr($rawResult, 0, 1000)
142+
);
143+
}
149144

150-
$message = null;
145+
if (!is_array($result) || !array_key_exists('status', $result)) {
146+
throw WebDriverException::factory(
147+
WebDriverException::CURL_EXEC,
148+
'Payload received from webdriver is valid but unexpected json: ' . substr($rawResult, 0, 1000)
149+
);
150+
}
151151

152-
if (is_array($value) && array_key_exists('message', $value)) {
153-
$message = $value['message'];
154-
}
152+
$value = array_key_exists('value', $result) ? $result['value'] : null;
153+
$message = (is_array($value) && array_key_exists('message', $value)) ? $value['message'] : null;
155154

156-
// if not success, throw exception
157-
if ((int) $result['status'] !== 0) {
158-
throw WebDriverException::factory($result['status'], $message);
159-
}
155+
// if not success, throw exception
156+
if ((int) $result['status'] !== 0) {
157+
throw WebDriverException::factory($result['status'], $message);
160158
}
161159

162160
$sessionId = isset($result['sessionId'])
163161
? $result['sessionId']
164-
: (
165-
isset($value['webdriver.remote.sessionid'])
166-
? $value['webdriver.remote.sessionid']
167-
: null
162+
: (isset($value['webdriver.remote.sessionid'])
163+
? $value['webdriver.remote.sessionid']
164+
: null
168165
);
169166

170167
return array(

‎lib/WebDriver/Service/CurlService.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,11 @@ public function execute($requestMethod, $url, $parameters = null, $extraOptions
9393
$info = curl_getinfo($curl);
9494
$info['request_method'] = $requestMethod;
9595

96-
// This only gets triggered when CURLOPT_FAILONERROR has been set in extraOptions.
97-
// In that case, $rawResult is always empty.
98-
if (CURLE_GOT_NOTHING !== ($errno = curl_errno($curl)) && $error = curl_error($curl)) {
96+
if (array_key_exists(CURLOPT_FAILONERROR, $extraOptions) &&
97+
$extraOptions[CURLOPT_FAILONERROR] &&
98+
CURLE_GOT_NOTHING !== ($errno = curl_errno($curl)) &&
99+
$error = curl_error($curl)
100+
) {
99101
curl_close($curl);
100102

101103
throw WebDriverException::factory(

‎lib/WebDriver/ServiceFactory.php‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,17 @@ public function getService($serviceName)
9191
return $this->services[$serviceName];
9292
}
9393

94+
/**
95+
* Set service
96+
*
97+
* @param string $serviceName Name of service
98+
* @param object $service Service instance
99+
*/
100+
public function setService($serviceName, $service)
101+
{
102+
$this->services[$serviceName] = $service;
103+
}
104+
94105
/**
95106
* Override default service class
96107
*
@@ -103,11 +114,7 @@ public function setServiceClass($serviceName, $className)
103114
$className = '\\' . $className;
104115
}
105116

106-
// Flush outdated service cache
107-
if (isset($this->serviceClasses[$serviceName]) && $this->serviceClasses[$serviceName] !== $className) {
108-
unset($this->services[$serviceName]);
109-
}
110-
111117
$this->serviceClasses[$serviceName] = $className;
118+
$this->services[$serviceName] = null;
112119
}
113120
}

‎test/Test/WebDriver/TestCurlService.php‎

Lines changed: 0 additions & 28 deletions
This file was deleted.

‎test/Test/WebDriver/WebDriverTest.php‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
* @package WebDriver
1818
*
1919
* @author Anthon Pang <apang@softwaredevelopment.ca>
20+
* @author Damian Mooyman <damian@silverstripe.com>
2021
*/
2122

2223
namespace Test\WebDriver;
@@ -43,7 +44,7 @@ class WebDriverTest extends \PHPUnit_Framework_TestCase
4344
*/
4445
protected function setUp()
4546
{
46-
require_once __DIR__ . '/TestCurlService.php';
47+
ServiceFactory::getInstance()->setServiceClass('service.curl', '\\WebDriver\\Service\\CurlService');
4748

4849
if ($url = getenv('ROOT_URL')) {
4950
$this->testDocumentRootUrl = $url;
@@ -62,7 +63,6 @@ protected function setUp()
6263
*/
6364
protected function tearDown()
6465
{
65-
ServiceFactory::getInstance()->setServiceClass('service.curl', '\\WebDriver\\Service\\CurlService');
6666
if ($this->session) {
6767
$this->session->close();
6868
}
@@ -231,16 +231,37 @@ public function testSeleniumNoResponse()
231231
*/
232232
public function testNonJsonResponse()
233233
{
234-
ServiceFactory::getInstance()->setServiceClass('service.curl', '\\Test\\WebDriver\\TestCurlService');
234+
$mockCurlService = $this->createMock('WebDriver\Service\CurlService');
235+
$mockCurlService->expects($this->once())
236+
->method('execute')
237+
->will($this->returnCallback(function ($requestMethod, $url) {
238+
$info = array(
239+
'url' => $url,
240+
'request_method' => $requestMethod,
241+
'http_code' => 200,
242+
);
243+
244+
$result = preg_match('#.*session$#', $url)
245+
? $result = 'some invalid json'
246+
: $result = '';
247+
248+
return array($result, $info);
249+
}));
250+
251+
ServiceFactory::getInstance()->setService('service.curl', $mockCurlService);
252+
235253
$result = $this->driver->status();
254+
236255
$this->assertNull($result);
237256

238257
// Test /session should error
239258
$this->setExpectedException(
240259
'WebDriver\Exception\CurlExec',
241260
'Payload received from webdriver is not valid json: some invalid json'
242261
);
262+
243263
$result = $this->driver->session();
264+
244265
$this->assertNull($result);
245266
}
246267
}

0 commit comments

Comments
 (0)