Merge pull request #22 from julienloizelet/feat/handle-404-for-get-de…

…cisions Feat/handle 404 for get decisions
crowdsecurity · Jan 5, 2023 · 9a5170b · 9a5170b
2 parents bc3354e + 4277233
commit 9a5170b
Show file tree

Hide file tree

Showing 9 changed files with 53 additions and 55 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,21 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/)
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.9.0](https://github.com/crowdsecurity/php-capi-client/releases/tag/v0.9.0) - 2023-01-05
+[_Compare with previous release_](https://github.com/crowdsecurity/php-capi-client/compare/v0.8.0...v0.9.0)
+
+
+### Changed
+
+- Do not throw error on CAPI 404 response
+- Use compressed requests for `Curl` 
+- Use message log instead of a context message field
+- Do not log error on `formatResponseBody` to avoid double reporting [(#21)](https://github.com/crowdsecurity/php-capi-client/issues/21)
+- Log retries as `info` and not as `error` [(#21)](https://github.com/crowdsecurity/php-capi-client/issues/21)
+
+---
+
+
 ## [0.8.0](https://github.com/crowdsecurity/php-capi-client/releases/tag/v0.8.0) - 2022-12-30
 [_Compare with previous release_](https://github.com/crowdsecurity/php-capi-client/compare/v0.7.0...v0.8.0)
 

diff --git a/src/AbstractClient.php b/src/AbstractClient.php
@@ -97,10 +97,7 @@ protected function request(
         $method = strtoupper($method);
         if (!in_array($method, $this->allowedMethods)) {
             $message = "Method ($method) is not allowed.";
-            $this->logger->error('', [
-                'type' => 'WATCHER_CLIENT_REQUEST',
-                'message' => $message,
-            ]);
+            $this->logger->error($message, ['type' => 'CAPI_CLIENT_REQUEST']);
             throw new ClientException($message);
         }
 
@@ -131,22 +128,22 @@ private function formatResponseBody(Response $response): array
         $statusCode = $response->getStatusCode();
 
         $body = $response->getJsonBody();
-        $decoded = ['message' => ''];
-        if (!empty($body)) {
+        $decoded = [];
+        if (!empty($body) && 'null' !== $body) {
             $decoded = json_decode($response->getJsonBody(), true);
 
             if (null === $decoded) {
-                throw new ClientException('Body response is not a valid json');
+                $message = 'Body response is not a valid json';
+                $this->logger->error($message, ['type' => 'CAPI_CLIENT_FORMAT_RESPONSE']);
+                throw new ClientException($message);
             }
         }
 
         if ($statusCode < 200 || $statusCode >= 300) {
             $message = "Unexpected response status code: $statusCode. Body was: " . str_replace("\n", '', $body);
-            $this->logger->error('', [
-                'type' => 'WATCHER_CLIENT_FORMAT_RESPONSE',
-                'message' => $message,
-            ]);
-            throw new ClientException($message, $statusCode);
+            if ($statusCode !== 404) {
+                throw new ClientException($message, $statusCode);
+            }
         }
 
         return $decoded;

diff --git a/src/Constants.php b/src/Constants.php
@@ -63,5 +63,5 @@ class Constants
     /**
      * @var string The current version of this library
      */
-    public const VERSION = 'v0.8.0';
+    public const VERSION = 'v0.9.0';
 }
diff --git a/src/Logger/FileLog.php b/src/Logger/FileLog.php
@@ -40,14 +40,14 @@ public function __construct(array $configs = [])
         if (empty($configs['disable_prod_log'])) {
             $logPath = $logDir . '/' . self::PROD_FILE;
             $fileHandler = new RotatingFileHandler($logPath, 0, Logger::INFO);
-            $fileHandler->setFormatter(new LineFormatter("%datetime%|%level%|%context%\n"));
+            $fileHandler->setFormatter(new LineFormatter("%datetime%|%level%|%message%|%context%\n"));
             $this->pushHandler($fileHandler);
         }
 
         if (!empty($configs['debug_mode'])) {
             $debugLogPath = $logDir . '/' . self::DEBUG_FILE;
             $debugFileHandler = new RotatingFileHandler($debugLogPath, 0, Logger::DEBUG);
-            $debugFileHandler->setFormatter(new LineFormatter("%datetime%|%level%|%context%\n"));
+            $debugFileHandler->setFormatter(new LineFormatter("%datetime%|%level%|%message%|%context%\n"));
             $this->pushHandler($debugFileHandler);
         }
     }

diff --git a/src/RequestHandler/Curl.php b/src/RequestHandler/Curl.php
@@ -100,6 +100,7 @@ private function createOptions(Request $request): array
             \CURLOPT_HEADER => false,
             \CURLOPT_RETURNTRANSFER => true,
             \CURLOPT_USERAGENT => $headers['User-Agent'],
+            \CURLOPT_ENCODING => ''
         ];
         // We need to keep keys indexes
         $options += $this->handleConfigs();

diff --git a/src/Watcher.php b/src/Watcher.php
@@ -348,10 +348,9 @@ private function handleLogin(): void
         $this->token = $loginResponse['token'] ?? null;
         if (!$this->token) {
             $message = 'Login response does not contain required token.';
-            $this->logger->error('', [
+            $this->logger->error($message, [
                 'type' => 'WATCHER_CLIENT_HANDLE_LOGIN',
-                'message' => $message,
-                'response' => $loginResponse,
+                'response' => $loginResponse
             ]);
             throw new ClientException($message, 401);
         }
@@ -369,10 +368,7 @@ private function handleTokenHeader(): array
     {
         if (!$this->token) {
             $message = 'Token is required.';
-            $this->logger->error('', [
-                'type' => 'WATCHER_CLIENT_HANDLE_TOKEN',
-                'message' => $message,
-            ]);
+            $this->logger->error($message, ['type' => 'WATCHER_CLIENT_HANDLE_TOKEN']);
             throw new ClientException('Token is required.', 401);
         }
 
@@ -409,7 +405,7 @@ private function manageRequest(
         string $endpoint,
         array $parameters = []
     ): array {
-        $this->logger->debug('', [
+        $this->logger->debug('Now processing a watcher request', [
             'type' => 'WATCHER_REQUEST',
             'method' => $method,
             'endpoint' => $endpoint,
@@ -436,17 +432,15 @@ private function manageRequest(
                  * In this case only, we try to log in again.
                  */
                 if (401 !== $code) {
-                    $this->logger->error('', [
+                    $this->logger->error($message, [
                         'type' => 'WATCHER_REQUEST_ERROR',
-                        'message' => $message,
-                        'code' => $code,
+                        'code' => $code
                     ]);
                     throw new ClientException($message, $code);
                 }
-                $this->logger->warning('', [
-                    'type' => 'WATCHER_REQUEST_ERROR_401',
-                    'message' => $message,
-                    'code' => $code,
+                $this->logger->info($message, [
+                    'type' => 'WATCHER_REQUEST_LOGIN_RETRY',
+                    'code' => $code
                 ]);
                 ++$loginRetry;
                 $retry = true;
@@ -455,9 +449,8 @@ private function manageRequest(
         } while ($retry && ($loginRetry <= self::LOGIN_RETRY));
         if ($loginRetry > self::LOGIN_RETRY) {
             $message = "Could not login after $loginRetry attempts. Last error was: $lastMessage";
-            $this->logger->error('', [
-                'type' => 'WATCHER_REQUEST_TOO_MANY_ATTEMPTS',
-                'message' => $message
+            $this->logger->error($message, [
+                'type' => 'WATCHER_REQUEST_TOO_MANY_ATTEMPTS'
             ]);
             throw new ClientException($message);
         }
@@ -475,18 +468,12 @@ private function normalizeTags(array $tags): array
         foreach ($tags as $tag) {
             if (!is_string($tag)) {
                 $message = 'Tag must be a string: ' . gettype($tag) . ' given.';
-                $this->logger->error('', [
-                    'type' => 'WATCHER_NORMALIZE_TAGS',
-                    'message' => $message,
-                ]);
+                $this->logger->error($message, ['type' => 'WATCHER_NORMALIZE_TAGS']);
                 throw new ClientException($message, 500);
             }
             if (empty($tag)) {
                 $message = 'Tag must not be empty';
-                $this->logger->error('', [
-                    'type' => 'WATCHER_NORMALIZE_TAGS',
-                    'message' => $message,
-                ]);
+                $this->logger->error($message, ['type' => 'WATCHER_NORMALIZE_TAGS']);
                 throw new ClientException($message, 500);
             }
         }
@@ -541,17 +528,15 @@ private function register(): void
                  * In this case only, we try to register again with new credentials.
                  */
                 if (500 !== $code) {
-                    $this->logger->error('', [
+                    $this->logger->error($message, [
                         'type' => 'WATCHER_REGISTER_ERROR',
-                        'message' => $message,
-                        'code' => $code,
+                        'code' => $code
                     ]);
                     throw new ClientException($message, $code);
                 }
-                $this->logger->warning('', [
-                    'type' => 'WATCHER_REGISTER_ERROR_500',
-                    'message' => $message,
-                    'code' => $code,
+                $this->logger->info($message, [
+                    'type' => 'WATCHER_REGISTER_RETRY',
+                    'code' => $code
                 ]);
                 ++$registerRetry;
                 $retry = true;
@@ -560,10 +545,7 @@ private function register(): void
         } while ($retry && ($registerRetry <= self::REGISTER_RETRY));
         if ($registerRetry > self::REGISTER_RETRY) {
             $message = "Could not register after $registerRetry attempts. Last error was: $lastMessage";
-            $this->logger->error('', [
-                'type' => 'WATCHER_REGISTER_TOO_MANY_ATTEMPTS',
-                'message' => $message,
-            ]);
+            $this->logger->error($message, ['type' => 'WATCHER_REGISTER_TOO_MANY_ATTEMPTS']);
             throw new ClientException($message);
         }
     }

diff --git a/tests/Unit/AbstractClientTest.php b/tests/Unit/AbstractClientTest.php
@@ -199,9 +199,9 @@ public function testPrivateOrProtectedMethods()
         );
 
         $this->assertEquals(
-            ['message' => ''],
+            [],
             $decoded,
-            'An empty response body should not return some array'
+            'An empty response body should return empty array'
         );
 
         $response = new Response('', 500);

diff --git a/tests/Unit/CurlTest.php b/tests/Unit/CurlTest.php
@@ -250,6 +250,7 @@ public function testOptions()
             \CURLOPT_URL => $url,
             \CURLOPT_CUSTOMREQUEST => $method,
             \CURLOPT_TIMEOUT => TestConstants::API_TIMEOUT,
+            \CURLOPT_ENCODING => ''
         ];
 
         $this->assertEquals(
@@ -286,6 +287,7 @@ public function testOptions()
             \CURLOPT_URL => $url . '?foo=bar&crowd=sec',
             \CURLOPT_CUSTOMREQUEST => $method,
             \CURLOPT_TIMEOUT => TestConstants::API_TIMEOUT,
+            \CURLOPT_ENCODING => ''
         ];
 
         $this->assertEquals(

diff --git a/tests/scripts/watcher/decisions-stream.php b/tests/scripts/watcher/decisions-stream.php
@@ -4,6 +4,7 @@
 
 use CrowdSec\CapiClient\Storage\FileStorage;
 use CrowdSec\CapiClient\Watcher;
+use CrowdSec\CapiClient\Logger\FileLog;
 
 $scenarios = isset($argv[1]) ? json_decode($argv[1], true) : false;
 if (is_null($scenarios)) {
@@ -24,7 +25,7 @@
     'user_agent_suffix' => 'CapiClientTest',
     'scenarios' => $scenarios,
     ];
-$client = new Watcher($configs, new FileStorage());
+$client = new Watcher($configs, new FileStorage(), null, new FileLog(['debug_mode' => true]));
 echo 'Watcher instantiated' . \PHP_EOL;
 
 echo 'Calling ' . $client->getConfig('api_url') . ' for decisions stream ...' . \PHP_EOL;