From d999637808b4906d6674bb94768083f63ce572e8 Mon Sep 17 00:00:00 2001
From: Alex Silverman <asilverman@sailthru.com>
Date: Mon, 14 Aug 2017 18:36:35 -0400
Subject: [PATCH] Refactoring httpRequest exceptions, adding without curl test

---
 sailthru/Sailthru_Client.php            | 28 +++++++++----------------
 sailthru/Sailthru_Client_Exception.php  |  5 ++---
 tests/Sailthru_Client_ExceptionTest.php | 27 ++++++++++++++++++------
 3 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/sailthru/Sailthru_Client.php b/sailthru/Sailthru_Client.php
index 25e8568..05ea552 100644
--- a/sailthru/Sailthru_Client.php
+++ b/sailthru/Sailthru_Client.php
@@ -1370,19 +1370,11 @@ protected function httpRequestCurl($action, array $data, $method = 'POST', $opti
         if (false === $response) {
             // There's a curl error! throw an exception which gives us some details
             throw new Sailthru_Client_Exception(
-                "Curl error: $errorMessage",
+                "Curl error: {$errorMessage}",
                 Sailthru_Client_Exception::CODE_HTTP_ERROR
             );
         }
 
-        if (!$response) {
-            // We have an empty (well, falsey) response for the server, but not a curl error
-            throw new Sailthru_Client_Exception(
-                "Bad response received from $url",
-                Sailthru_Client_Exception::CODE_RESPONSE_EMPTY
-            );
-        }
-
         // parse headers and body
         $parts = explode("\r\n\r\nHTTP/", $response);
         $parts = (count($parts) > 1 ? 'HTTP/' : '') . array_pop($parts); // deal with HTTP/1.1 100 Continue before other headers
@@ -1423,15 +1415,15 @@ protected function httpRequestWithoutCurl($action, $data, $method = 'POST', $opt
         $fp = @fopen($url, 'rb', false, $ctx);
         if (!$fp) {
             throw new Sailthru_Client_Exception(
-                "Unable to open stream: $url",
-                Sailthru_Client_Exception::CODE_GENERAL
+                "Stream error: unable to open stream: {$url}",
+                Sailthru_Client_Exception::CODE_HTTP_ERROR
             );
         }
         $response = @stream_get_contents($fp);
         if ($response === false) {
             throw new Sailthru_Client_Exception(
-                "No response received from stream: $url",
-                Sailthru_Client_Exception::CODE_RESPONSE_EMPTY
+                "Stream error: Failed to read from stream: {$url}",
+                Sailthru_Client_Exception::CODE_HTTP_ERROR
             );
         }
         return $response;
@@ -1444,17 +1436,17 @@ protected function httpRequestWithoutCurl($action, $data, $method = 'POST', $opt
      * @param array $data
      * @param string $method
      * @param array $options
-     * @return string
+     * @return array
      * @throws Sailthru_Client_Exception
      */
     protected function httpRequest($action, $data, $method = 'POST', $options = [ ]) {
         $response = $this->{$this->http_request_type}($action, $data, $method, $options);
         $json = json_decode($response, true);
         if ($json === NULL) {
-            throw new Sailthru_Client_Exception(
-                "Response: {$response} is not a valid JSON",
-                Sailthru_Client_Exception::CODE_RESPONSE_INVALID
-            );
+            $exception_msg = empty($response)
+                ? "Empty response"
+                : "Response: {$response} is not a valid JSON";
+            throw new Sailthru_Client_Exception($exception_msg, Sailthru_Client_Exception::CODE_RESPONSE_INVALID);
         }
         if (!empty($json['error'])) {
             throw new Sailthru_Client_Exception($json['errormsg'], $json['error']);
diff --git a/sailthru/Sailthru_Client_Exception.php b/sailthru/Sailthru_Client_Exception.php
index f1ca56e..eee0a86 100644
--- a/sailthru/Sailthru_Client_Exception.php
+++ b/sailthru/Sailthru_Client_Exception.php
@@ -14,7 +14,6 @@ class Sailthru_Client_Exception extends Exception {
      * Standardized exception codes.
      */
     const CODE_GENERAL = 1000;
-    const CODE_RESPONSE_EMPTY = 1001;
-    const CODE_RESPONSE_INVALID = 1002;
-    const CODE_HTTP_ERROR = 1003;
+    const CODE_RESPONSE_INVALID = 1001;
+    const CODE_HTTP_ERROR = 1002;
 }
diff --git a/tests/Sailthru_Client_ExceptionTest.php b/tests/Sailthru_Client_ExceptionTest.php
index c449676..3f0be46 100644
--- a/tests/Sailthru_Client_ExceptionTest.php
+++ b/tests/Sailthru_Client_ExceptionTest.php
@@ -1,23 +1,38 @@
 <?php
 
 class Sailthru_Client_ExceptionTest extends PHPUnit_Framework_TestCase {
+
+    private $api_key = "invalid_key";
+    private $api_secret = "invalid_secret";
+    private $bad_api_uri = "http://foo.invalid"; // .invalid is reserved as an invalid TLD, see https://en.wikipedia.org/wiki/.invalid
+
     public function testSailthru_Client_Exception_IsThrownWithCurlError() {
         $expectedExceptionMessage = 'Curl error: ';
-
         $this->setExpectedException(
             'Sailthru_Client_Exception',
             $expectedExceptionMessage,
-            1003
+            1002
         );
 
-        $api_key = "invalid_key";
-        $api_secret = "invalid_secret";
-        $api_url = "http://foo.invalid"; // .invalid is reserved as an invalid TLD, see https://en.wikipedia.org/wiki/.invalid
+        $sailthruClient = new Sailthru_Client($this->api_key, $this->api_secret, $this->bad_api_uri);
+        $sailthruClient->getUser("praj@sailthru.com");
+    }
+
+    public function testSailthru_Client_Exception_IsThrownWithoutCurlError() {
+        $sailthruClient = new Sailthru_Client($this->api_key, $this->api_secret, $this->bad_api_uri);
+        $mockHttpType = new ReflectionProperty("Sailthru_Client", "http_request_type");
+        $mockHttpType->setAccessible(true);
+        $mockHttpType->setValue($sailthruClient, "httpRequestWithoutCurl");
 
-        $sailthruClient = new Sailthru_Client($api_key, $api_secret, $api_url);
+        $this->setExpectedException(
+            'Sailthru_Client_Exception',
+            'Stream error: ',
+            1002
+        );
         $sailthruClient->getUser("praj@sailthru.com");
     }
 
+
     public function testSailthru_Client_Exception_IsThrownWithEmptyResponse() {
         $this->markTestSkipped('This cannot be tested without either mocking curl or having a public server which returns us an empty response.');