fix: handle drained response streams in api decoding (#121)

* test: cover drained response bodies without retrying

* fix(api-call): read full response bodies before decoding

- switch api responses to string-cast body reads so drained streams from middleware still decode correctly
- update retry tests to mock string-castable response streams

Author: tharropoulos

src/ApiCall.php

@@ -256,8 +256,7 @@ private function makeRequest(string $method, string $endPoint, bool $asJson, arr
                     $this->setNodeHealthCheck($node, true);
                 }
 
-                $responseContents = $response->getBody()
-                    ->getContents();
+                $responseContents = (string) $response->getBody();
 
                 if (!(200 <= $statusCode && $statusCode < 300)) {
                     try {

tests/Feature/ApiCallRetryTest.php

@@ -18,6 +18,19 @@
 
 class ApiCallRetryTest extends TestCase
 {
+    private function createJsonResponseMock(string $json, int $statusCode = 200): ResponseInterface
+    {
+        $stream = $this->createMock(StreamInterface::class);
+        $stream->method('__toString')->willReturn($json);
+        $stream->method('getContents')->willReturn($json);
+
+        $response = $this->createMock(ResponseInterface::class);
+        $response->method('getStatusCode')->willReturn($statusCode);
+        $response->method('getBody')->willReturn($stream);
+
+        return $response;
+    }
+
     public function testRetriesOnHttpExceptionWithNon408Status(): void
     {
         $callCount = 0;
@@ -33,12 +46,7 @@ public function testRetriesOnHttpExceptionWithNon408Status(): void
                     $response->method('getStatusCode')->willReturn(500);
                     throw new HttpException('Server error', $this->createMock(RequestInterface::class), $response);
                 } else {
-                    $response = $this->createMock(ResponseInterface::class);
-                    $response->method('getStatusCode')->willReturn(200);
-                    $stream = $this->createMock(StreamInterface::class);
-                    $stream->method('getContents')->willReturn('{"success": true}');
-                    $response->method('getBody')->willReturn($stream);
-                    return $response;
+                    return $this->createJsonResponseMock('{"success": true}');
                 }
             });
 
@@ -109,12 +117,7 @@ public function testRetriesOnTypesenseClientError(): void
                 if ($callCount < $expectedCalls) {
                     throw new RequestMalformed('Bad request');
                 } else {
-                    $response = $this->createMock(ResponseInterface::class);
-                    $response->method('getStatusCode')->willReturn(200);
-                    $stream = $this->createMock(StreamInterface::class);
-                    $stream->method('getContents')->willReturn('{"success": true}');
-                    $response->method('getBody')->willReturn($stream);
-                    return $response;
+                    return $this->createJsonResponseMock('{"success": true}');
                 }
             });
 
@@ -152,12 +155,7 @@ public function testRetriesOnHttpClientException(): void
                 if ($callCount < $expectedCalls) {
                     throw new TransferException('Connection error');
                 } else {
-                    $response = $this->createMock(ResponseInterface::class);
-                    $response->method('getStatusCode')->willReturn(200);
-                    $stream = $this->createMock(StreamInterface::class);
-                    $stream->method('getContents')->willReturn('{"success": true}');
-                    $response->method('getBody')->willReturn($stream);
-                    return $response;
+                    return $this->createJsonResponseMock('{"success": true}');
                 }
             });
 
@@ -198,12 +196,7 @@ public function testSkips408TimeoutErrorsAndContinuesRetrying(): void
                     $response->method('getStatusCode')->willReturn(500);
                     throw new HttpException('Server error', $this->createMock(RequestInterface::class), $response);
                 } else {
-                    $response = $this->createMock(ResponseInterface::class);
-                    $response->method('getStatusCode')->willReturn(200);
-                    $stream = $this->createMock(StreamInterface::class);
-                    $stream->method('getContents')->willReturn('{"success": true}');
-                    $response->method('getBody')->willReturn($stream);
-                    return $response;
+                    return $this->createJsonResponseMock('{"success": true}');
                 }
             });
 
@@ -384,12 +377,7 @@ public function test408ErrorsAreSkippedAndRetryingContinues(): void
                     $response->method('getStatusCode')->willReturn(408);
                     throw new HttpException('Request timeout', $this->createMock(RequestInterface::class), $response);
                 } else {
-                    $response = $this->createMock(ResponseInterface::class);
-                    $response->method('getStatusCode')->willReturn(200);
-                    $stream = $this->createMock(StreamInterface::class);
-                    $stream->method('getContents')->willReturn('{"success": true}');
-                    $response->method('getBody')->willReturn($stream);
-                    return $response;
+                    return $this->createJsonResponseMock('{"success": true}');
                 }
             });
 
@@ -475,6 +463,7 @@ public function testThrowsTypesenseClientErrorWhenSuccessResponseContainsInvalid
                 $response = $this->createMock(ResponseInterface::class);
                 $response->method('getStatusCode')->willReturn(200);
                 $stream = $this->createMock(StreamInterface::class);
+                $stream->method('__toString')->willReturn('{invalid json');
                 $stream->method('getContents')->willReturn('{invalid json');
                 $response->method('getBody')->willReturn($stream);
                 return $response;
@@ -500,4 +489,40 @@ public function testThrowsTypesenseClientErrorWhenSuccessResponseContainsInvalid
             $this->assertInstanceOf(JsonException::class, $exception->getPrevious());
         }
     }
+
+    public function testDrainedResponseBodyStillDecodesWithoutRetrying(): void
+    {
+        $callCount = 0;
+
+        $stream = $this->createMock(StreamInterface::class);
+        $stream->method('__toString')->willReturn('{"ok": true}');
+        $stream->method('getContents')->willReturn('');
+
+        $response = $this->createMock(ResponseInterface::class);
+        $response->method('getStatusCode')->willReturn(200);
+        $response->method('getBody')->willReturn($stream);
+
+        $httpClient = $this->createMock(ClientInterface::class);
+        $httpClient->method('sendRequest')
+            ->willReturnCallback(function() use (&$callCount, $response) {
+                $callCount++;
+
+                return $response;
+            });
+
+        $config = new Configuration([
+            'api_key' => 'test-key',
+            'nodes' => [
+                ['host' => 'node1', 'port' => 8108, 'protocol' => 'http']
+            ],
+            'client' => $httpClient
+        ]);
+
+        $apiCall = new ApiCall($config);
+
+        $result = $apiCall->get('/health', []);
+
+        $this->assertSame(['ok' => true], $result);
+        $this->assertSame(1, $callCount);
+    }
 }