Audit and Expand Error handling In CloudFlareAPI class

[aweingarten]

Business Requirements

• [ ] As a use of the PHP SDK I want to get meaningful and typed exceptions thrown when a request does not succeed as expected.

  Background

The PHP SDK attempts to consolidate as much exception handling into a central location here: https://github.com/d8-contrib-modules/cloudflarephpsdk/blob/master/src/ApiEndpoints/CloudFlareAPI.php#L123

The code is designed to throw typed exceptions when a problem happens and expects the implementing code to handle these exceptions: https://github.com/d8-contrib-modules/cloudflarephpsdk/tree/master/src/Exceptions

Technical Requirements

• [ ] Review the exceptions that Guzzle can throw when misconfigured or endpoint is unreachable. Confirm that these exceptions are handled and if not fix/add exceptions.
• [ ] Review the cloudflare API here and look for the different exceptions. If any are not handled add them
• [ ] Review the existing PHPUnit tests here: https://github.com/d8-contrib-modules/cloudflarephpsdk/blob/master/tests/ApiEndpoints/ZoneApiTest.php and https://github.com/d8-contrib-modules/cloudflarephpsdk/blob/master/tests/ApiEndpoints/ZoneSettingsApiTest.php
• [ ] Add automated tests to cover these new edge cases.

[mradcliffe]

You could probably add tests that mock Guzzle, and force a specific response exception to be thrown.

I think....

$client = $this->getMockBuilder('\GuzzleHttp\Client')
   ->disableOriginalConstructor()
   ->getMock();
$client->expects($this->any())
   ->method('request')
   // Probably need to pass in some mocks into the exception object too.
   ->will($this->throwException(new \GuzzleHttp\Exception\RequestException));

Also, depending on guzzle "dev-master" might not be the best for stability.

[aweingarten]

@mradcliffe: I like the way you think. There are some primitive phpunit tests that have been added along those lines.

The constructor allows a mock object to be passed in: https://github.com/d8-contrib-modules/cloudflarephpsdk/blob/master/src/ApiEndpoints/CloudFlareAPI.php#L60

See tests here:

• https://github.com/d8-contrib-modules/cloudflarephpsdk/blob/master/tests/ApiEndpoints/ZoneApiTest.php
• https://github.com/d8-contrib-modules/cloudflarephpsdk/blob/master/tests/ApiEndpoints/ZoneSettingsApiTest.php

any chance you are interested in digging into this issue?

On the issue of tracking dev-master. You are correct. I'm going to add an issue to pin to a version for beta.

[mradcliffe]

I probably won't be able to get to it soon, but if I can, I'll fork and submit a Pull Request. I have been enjoying writing tests these days.