Fix warning in Connection::tryDeserializeError() (#168)

* Fixed warning in Connection::tryDeserializeError(). Signed-off-by: Thomas Seidl <[email protected]> * Cleaned up code. Signed-off-by: Thomas Seidl <[email protected]> * Added CHANGELOG.md entry. Signed-off-by: Thomas Seidl <[email protected]> --------- Signed-off-by: Thomas Seidl <[email protected]>
opensearch-project · Mar 28, 2024 · 8962f02 · 8962f02
1 parent 4398973
commit 8962f02
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -28,6 +28,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Fixed host urls with trailing slash in the url ([#130](https://github.com/opensearch-project/opensearch-php/pull/140))
 - Fixed point-in-time APIs ([#142](https://github.com/opensearch-project/opensearch-php/pull/142))
 - Fixed bug in ClientBuilder where basic authentication is overridden by connection params ([#160](https://github.com/opensearch-project/opensearch-php/pull/160))
+- Fixed PHP warning in `Connection::tryDeserializeError()` for some error responses ([#167](https://github.com/opensearch-project/opensearch-php/issues/167))
 
 ### Security
 

diff --git a/src/OpenSearch/Connections/Connection.php b/src/OpenSearch/Connections/Connection.php
@@ -748,14 +748,9 @@ private function tryDeserializeError(array $response, string $errorClass): OpenS
             // 2.0 structured exceptions
             if (is_array($error['error']) && array_key_exists('reason', $error['error']) === true) {
                 // Try to use root cause first (only grabs the first root cause)
-                $root = $error['error']['root_cause'];
-                if (isset($root) && isset($root[0])) {
-                    $cause = $root[0]['reason'];
-                    $type = $root[0]['type'];
-                } else {
-                    $cause = $error['error']['reason'];
-                    $type = $error['error']['type'];
-                }
+                $info = $error['error']['root_cause'][0] ?? $error['error'];
+                $cause = $info['reason'];
+                $type = $info['type'];
                 // added json_encode to convert into a string
                 $original = new $errorClass(json_encode($response['body']), $response['status']);
 

diff --git a/tests/Connections/ConnectionTest.php b/tests/Connections/ConnectionTest.php
@@ -316,6 +316,41 @@ function () {
         $this->assertStringContainsString('master_not_discovered_exception', $result->getMessage());
     }
 
+    /**
+     * @see https://github.com/opensearch-project/opensearch-php/issues/167
+     */
+    public function testTryDeserializeErrorWith403Error()
+    {
+        $host = [
+            'host' => 'localhost'
+        ];
+
+        $connection = new Connection(
+            function () {
+            },
+            $host,
+            [],
+            new SmartSerializer(),
+            $this->logger,
+            $this->trace
+        );
+
+        $reflection = new ReflectionClass(Connection::class);
+        $tryDeserializeError = $reflection->getMethod('tryDeserializeError');
+        $tryDeserializeError->setAccessible(true);
+
+        $body = '{"status":403,"error":{"reason":"403 Forbidden","type":"Forbidden"}}';
+        $response = [
+            'transfer_stats' => [],
+            'status' => 403,
+            'body' => $body
+        ];
+
+        $result = $tryDeserializeError->invoke($connection, $response, ServerErrorResponseException::class);
+        $this->assertInstanceOf(ServerErrorResponseException::class, $result);
+        $this->assertStringContainsString('403 Forbidden', $result->getMessage());
+    }
+
     public function testHeaderClientParamIsResetAfterSent()
     {
         $host = [