Merge pull request #5 from cloudflare/thellimist/PI-710

Changed error message from always "Bad Request" to api.cloudflare err…
cloudflare · Jul 29, 2016 · 4f798ef · 4f798ef
2 parents ea9d01e + a7775b1
commit 4f798ef
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 4 deletions.
diff --git a/src/API/AbstractAPIClient.php b/src/API/AbstractAPIClient.php
@@ -65,20 +65,37 @@ public function callAPI(Request $request)
 
             return $response;
         } catch (RequestException $e) {
+            $errorMessage = getErrorMessage($e);
+
             $this->logAPICall($this->getAPIClientName(), array(
                 'type' => 'request',
                 'method' => $request->getMethod(),
                 'path' => $request->getUrl(),
                 'headers' => $request->getHeaders(),
                 'params' => $request->getParameters(),
                 'body' => $request->getBody(), ), true);
-            $this->logAPICall($this->getAPIClientName(), array('type' => 'response', 'code' => $e->getCode(), 'body' => $e->getMessage(), 'stacktrace' => $e->getTraceAsString()), true);
+            $this->logAPICall($this->getAPIClientName(), array('type' => 'response', 'code' => $e->getCode(), 'body' => $errorMessage, 'stacktrace' => $e->getTraceAsString()), true);
 
-            return $this->createAPIError($e->getMessage());
+            return $this->createAPIError($errorMessage);
         }
     }
 
-    public function logAPICall($api, $message, $isError)
+    /**
+     * @param RequestException $object
+     *
+     * @return string
+     */
+    public function getErrorMessage(RequestException $error)
+    {
+        return $error->getMessage();
+    }
+
+    /**
+     * @param string $apiName
+     * @param array  $message
+     * @param bool   $isError
+     */
+    public function logAPICall(string $apiName, array $message, bool $isError)
     {
         $logLevel = 'error';
         if ($isError === false) {
@@ -89,7 +106,7 @@ public function logAPICall($api, $message, $isError)
             $message = print_r($message, true);
         }
 
-        $this->logger->$logLevel('['.$api.'] '.$message);
+        $this->logger->$logLevel('['.$apiName.'] '.$message);
     }
 
     /**
@@ -103,4 +120,11 @@ abstract public function beforeSend(Request $request);
      * @return mixed
      */
     abstract public function getAPIClientName();
+
+    /**
+     * @param $message
+     *
+     * @return array
+     */
+    abstract public function createAPIError($message);
 }
diff --git a/src/API/Client.php b/src/API/Client.php
@@ -2,6 +2,8 @@
 
 namespace CF\API;
 
+use GuzzleHttp\Exception\RequestException;
+
 class Client extends AbstractAPIClient
 {
     const CLIENT_API_NAME = 'CLIENT API';
@@ -48,6 +50,23 @@ public function createAPIError($message)
         );
     }
 
+    /**
+     * @param RequestException error
+     *
+     * @return string
+     */
+    public function getErrorMessage(RequestException $error)
+    {
+        $jsonResponse = json_decode($error->getResponse()->getBody(), true);
+        $errorMessage = $error->getMessage();
+
+        if (count($jsonResponse['errors']) > 0) {
+            $errorMessage = $jsonResponse['errors'][0]['message'];
+        }
+
+        return $errorMessage;
+    }
+
     /**
      * @param $response
      *

diff --git a/src/Test/API/ClientTest.php b/src/Test/API/ClientTest.php
@@ -80,4 +80,37 @@ public function testResponseOkReturnsTrueForValidResponse()
 
         $this->assertTrue($this->mockClientAPI->responseOk($v4APIResponse));
     }
+
+    public function testGetErrorMessageSuccess()
+    {
+        $errorMessage = 'I am an error message';
+
+        $error = $this->getMockBuilder('GuzzleHttp\Exception\RequestException')
+            ->disableOriginalConstructor()
+            ->setMethods(array('getResponse', 'getBody', 'getMessage'))
+            ->getMock();
+
+        $errorJSON = json_encode(
+            array(
+                'success' => false,
+                'errors' => array(
+                    array(
+                        'message' => $errorMessage,
+                    ),
+                ),
+            )
+        );
+
+        $error->expects($this->any())
+            ->method('getMessage')
+            ->will($this->returnValue('Not this message'));
+        $error->expects($this->any())
+            ->method('getResponse')
+            ->will($this->returnSelf());
+        $error->expects($this->any())
+            ->method('getBody')
+            ->will($this->returnValue($errorJSON));
+
+        $this->assertEquals($errorMessage, $this->mockClientAPI->getErrorMessage($error));
+    }
 }