Merge pull request #273 from bigcommerce/272

Add response headers to the error exceptions #272
bigcommerce · Oct 7, 2021 · 7750e08 · 7750e08
2 parents 18a5ca8 + 8ad1a02
commit 7750e08
Show file tree

Hide file tree

Showing 9 changed files with 69 additions and 47 deletions.
diff --git a/.gitignore b/.gitignore
@@ -8,5 +8,6 @@ script/*
 /build
 /test/reports
 .env
+.idea
 /.php_cs
 /.php_cs.cache
diff --git a/composer.json b/composer.json
@@ -19,7 +19,8 @@
     ],
     "require": {
         "php": ">=7.0",
-        "firebase/php-jwt": "~3.0 || ~5.0"
+        "firebase/php-jwt": "~3.0 || ~5.0",
+        "ext-curl": "*"
     },
     "require-dev": {
         "codeless/jugglecode": "1.0",

diff --git a/src/Bigcommerce/Api/Connection.php b/src/Bigcommerce/Api/Connection.php
@@ -28,12 +28,12 @@ class Connection
     /**
      * @var array Hash of HTTP request headers.
      */
-    private $headers = array();
+    private $headers = [];
 
     /**
      * @var array Hash of headers from HTTP response
      */
-    private $responseHeaders = array();
+    private $responseHeaders = [];
 
     /**
      * The status line of the response.
@@ -95,8 +95,8 @@ public function __construct()
             define('STDIN', fopen('php://stdin', 'r'));
         }
         $this->curl = curl_init();
-        curl_setopt($this->curl, CURLOPT_HEADERFUNCTION, array($this, 'parseHeader'));
-        curl_setopt($this->curl, CURLOPT_WRITEFUNCTION, array($this, 'parseBody'));
+        curl_setopt($this->curl, CURLOPT_HEADERFUNCTION, [$this, 'parseHeader']);
+        curl_setopt($this->curl, CURLOPT_WRITEFUNCTION, [$this, 'parseBody']);
 
         // Set to a blank string to make cURL include all encodings it can handle (gzip, deflate, identity) in the 'Accept-Encoding' request header and respect the 'Content-Encoding' response header
         curl_setopt($this->curl, CURLOPT_ENCODING, '');
@@ -257,7 +257,7 @@ private function getContentType()
     private function initializeRequest()
     {
         $this->responseBody = '';
-        $this->responseHeaders = array();
+        $this->responseHeaders = [];
         $this->lastError = false;
         $this->addHeader('Accept', $this->getContentType());
 
@@ -277,7 +277,7 @@ private function initializeRequest()
     private function handleResponse()
     {
         if (curl_errno($this->curl)) {
-            throw new NetworkError(curl_error($this->curl), curl_errno($this->curl));
+            throw new NetworkError(curl_error($this->curl), curl_errno($this->curl), $this->responseHeaders);
         }
 
         $body = ($this->rawResponse) ? $this->getBody() : json_decode($this->getBody());
@@ -286,14 +286,14 @@ private function handleResponse()
 
         if ($status >= 400 && $status <= 499) {
             if ($this->failOnError) {
-                throw new ClientError($body, $status);
+                throw new ClientError($body, $status, $this->responseHeaders);
             } else {
                 $this->lastError = $body;
                 return false;
             }
         } elseif ($status >= 500 && $status <= 599) {
             if ($this->failOnError) {
-                throw new ServerError($body, $status);
+                throw new ServerError($body, $status, $this->responseHeaders);
             } else {
                 $this->lastError = $body;
                 return false;
@@ -342,7 +342,7 @@ private function followRedirectPath()
                 $this->get($url);
             } else {
                 $errorString = "Too many redirects when trying to follow location.";
-                throw new NetworkError($errorString, CURLE_TOO_MANY_REDIRECTS);
+                throw new NetworkError($errorString, CURLE_TOO_MANY_REDIRECTS, $this->responseHeaders);
             }
         } else {
             $this->redirectsFollowed = 0;

diff --git a/src/Bigcommerce/Api/Error.php b/src/Bigcommerce/Api/Error.php
@@ -7,12 +7,32 @@
  */
 class Error extends \Exception
 {
-    public function __construct($message, $code)
+    /**
+     * @var array
+     */
+    private $headers;
+
+    /**
+     * @param $message
+     * @param $code
+     * @param string[] $headers
+     */
+    public function __construct($message, $code, array $headers = [])
     {
         if (is_array($message)) {
             $message = $message[0]->message;
         }
 
         parent::__construct($message, $code);
+        $this->headers = $headers;
     }
+
+    /**
+     * @return string[]
+     */
+    public function getResponseHeaders(): array
+    {
+        return $this->headers;
+    }
+
 }
diff --git a/test/Unit/Api/ClientErrorTest.php b/test/Unit/Api/ClientErrorTest.php
@@ -8,7 +8,7 @@ class ClientErrorTest extends TestCase
 {
     public function testStringifyingReturnsTheMessageAndCodeAppropriately()
     {
-        $error = new ClientError('message here', 100);
+        $error = new ClientError('message here', 100, []);
         $this->assertSame('Client Error (100): message here', (string)$error);
     }
 }
diff --git a/test/Unit/Api/ClientTest.php b/test/Unit/Api/ClientTest.php
@@ -283,21 +283,18 @@ public function testGetRequestsRemainingRequestsTimeWhenNoValueAvailable()
 
     public function collections()
     {
-        return array(
+        return [
             //      path           function             classname
-            array('products', 'getProducts', 'Product'),
-            array('brands', 'getBrands', 'Brand'),
-            array('orders', 'getOrders', 'Order'),
-            array('customers', 'getCustomers', 'Customer'),
-            array('coupons', 'getCoupons', 'Coupon'),
-            array('order_statuses', 'getOrderStatuses', 'OrderStatus'),
-            array('categories', 'getCategories', 'Category'),
-            array('options', 'getOptions', 'Option'),
-            array('optionsets', 'getOptionSets', 'OptionSet'),
-            array('products/skus', 'getSkus', 'Sku'),
-            array('requestlogs', 'getRequestLogs', 'RequestLog'),
-            array('pages', 'getPages', 'Page'),
-        );
+            ['products', 'getProducts', 'Product'],
+            ['brands', 'getBrands', 'Brand'],
+            ['orders', 'getOrders', 'Order'],
+            ['customers', 'getCustomers', 'Customer'],
+            ['coupons', 'getCoupons', 'Coupon'],
+            ['categories', 'getCategories', 'Category'],
+            ['options', 'getOptions', 'Option'],
+            ['optionsets', 'getOptionSets', 'OptionSet'],
+            ['products/skus', 'getSkus', 'Sku'],
+        ];
     }
 
     /**
@@ -322,11 +319,6 @@ public function testGettingASpecificResourceReturnsACollectionOfThatResource($pa
      */
     public function testGettingTheCountOfACollectionReturnsThatCollectionsCount($path, $fnName, $class)
     {
-        if (in_array($path, array('order_statuses', 'requestlogs', 'pages'))) {
-            //$this->markTestSkipped(sprintf('The API does not currently support getting the count of %s', $path));
-            return;
-        }
-
         $this->connection->expects($this->once())
             ->method('get')
             ->with($this->basePath . '/' . $path . '/count', false)
@@ -339,19 +331,19 @@ public function testGettingTheCountOfACollectionReturnsThatCollectionsCount($pat
 
     public function resources()
     {
-        return array(
+        return [
             //    path            function        classname
-            array('products',     '%sProduct',    'Product'),
-            array('brands',       '%sBrand',      'Brand'),
-            array('orders',       '%sOrder',      'Order'),
-            array('customers',    '%sCustomer',   'Customer'),
-            array('categories',   '%sCategory',   'Category'),
-            array('options',      '%sOption',     'Option'),
-            array('optionsets',   '%sOptionSet',  'OptionSet'),
-            array('coupons',      '%sCoupon',     'Coupon'),
-            array('currencies',   '%sCurrency',   'Currency'),
-            array('pages',        '%sPage',       'Page'),
-        );
+            ['products',     '%sProduct',    'Product'],
+            ['brands',       '%sBrand',      'Brand'],
+            ['orders',       '%sOrder',      'Order'],
+            ['customers',    '%sCustomer',   'Customer'],
+            ['categories',   '%sCategory',   'Category'],
+            ['options',      '%sOption',     'Option'],
+            ['optionsets',   '%sOptionSet',  'OptionSet'],
+            ['coupons',      '%sCoupon',     'Coupon'],
+            ['currencies',   '%sCurrency',   'Currency'],
+            ['pages',        '%sPage',       'Page'],
+        ];
     }
 
     /**

diff --git a/test/Unit/Api/ErrorTest.php b/test/Unit/Api/ErrorTest.php
@@ -9,13 +9,13 @@ class ErrorTest extends TestCase
     public function testConstructorHandlesArrayOfMessageObjects()
     {
         $messageObj = (object)array('message' => 'message here');
-        $error = new Error(array($messageObj), 0);
+        $error = new Error(array($messageObj), 0, []);
         $this->assertSame('message here', $error->getMessage());
     }
 
     public function testConstructorPassesMessageAndCodeThrough()
     {
-        $error = new Error('message here', 100);
+        $error = new Error('message here', 100, []);
         $this->assertSame('message here', $error->getMessage());
         $this->assertSame(100, $error->getCode());
     }

diff --git a/test/Unit/Api/NetworkErrorTest.php b/test/Unit/Api/NetworkErrorTest.php
@@ -8,7 +8,7 @@ class NetworkErrorTest extends TestCase
 {
     public function testBehavesExactlyLikeException()
     {
-        $error = new NetworkError('message', 100);
+        $error = new NetworkError('message', 100, []);
         $this->assertSame('message', $error->getMessage());
         $this->assertSame(100, $error->getCode());
     }

diff --git a/test/Unit/Api/ServerErrorTest.php b/test/Unit/Api/ServerErrorTest.php
@@ -8,8 +8,16 @@ class ServerErrorTest extends TestCase
 {
     public function testBehavesExactlyLikeException()
     {
-        $error = new ServerError('message', 100);
+        $error = new ServerError('message', 100, []);
         $this->assertSame('message', $error->getMessage());
         $this->assertSame(100, $error->getCode());
     }
+
+    public function testServerErrorCarriesResponseHeaders()
+    {
+        $error = new ServerError('message', 100, ['x-bc-header' => 'abc-123']);
+        $this->assertSame('message', $error->getMessage());
+        $this->assertSame(100, $error->getCode());
+        $this->assertSame(['x-bc-header' => 'abc-123'], $error->getResponseHeaders());
+    }
 }