diff --git a/packages/http/guzzle/src/GuzzleRequestAdapter.php b/packages/http/guzzle/src/GuzzleRequestAdapter.php index 0b66640..46db329 100644 --- a/packages/http/guzzle/src/GuzzleRequestAdapter.php +++ b/packages/http/guzzle/src/GuzzleRequestAdapter.php @@ -158,6 +158,9 @@ function (ResponseInterface $result) use ($targetCallable, $requestInfo, $errorM } $this->throwFailedResponse($result, $errorMappings, $span); + if ($this->shouldReturnNull($result)) { + return null; + } $span->setStatus(StatusCode::STATUS_OK, 'response_handle_success'); if ($this->is204NoContentResponse($result)) { return null; @@ -229,6 +232,9 @@ function (ResponseInterface $result) use ($targetCallable, $requestInfo, $errorM ); } $this->throwFailedResponse($result, $errorMappings, $span); + if ($this->shouldReturnNull($result)) { + return null; + } if ($this->is204NoContentResponse($result)) { return null; } @@ -268,6 +274,9 @@ function (ResponseInterface $result) use ($primitiveType, $requestInfo, $errorMa return $response->wait(); } $this->throwFailedResponse($result, $errorMappings, $span); + if ($this->shouldReturnNull($result)) { + return null; + } $this->setResponseType($primitiveType, $span); if ($this->is204NoContentResponse($result)) { return null; @@ -342,6 +351,9 @@ function (ResponseInterface $result) use ($primitiveType, $requestInfo, $errorMa ); } $this->throwFailedResponse($result, $errorMappings, $span); + if ($this->shouldReturnNull($result)) { + return null; + } if ($this->is204NoContentResponse($result)) { return null; } @@ -510,6 +522,61 @@ private function getRootParseNode(ResponseInterface $response, SpanInterface $sp } } + /** + * Helper function to check if the response should return null. + * + * Conditions: + * - The response status code is 204 or 304 + * - The response content is empty + * - The response status code is 301 or 302 and the location header is not present + * + * @param ResponseInterface $response The HTTP response + * @return bool True if the response should return null, False otherwise + */ + private function shouldReturnNull(ResponseInterface $response): bool + { + return $response->getStatusCode() === 204 + || $response->getStatusCode() === 304 + || empty((string)$response->getBody()) + || (!$response->hasHeader('Location') && in_array($response->getStatusCode(), [301, 302])); + } + + /** + * Checks if a redirect response is missing the Location header + * + * @param ResponseInterface $response The HTTP response + * @param SpanInterface $parentSpan + * @param SpanInterface $attributeSpan + * @return bool + * @throws ApiException + */ + private function isRedirectMissingLocation( + ResponseInterface $response, + SpanInterface $parentSpan, + SpanInterface $attributeSpan + ): bool { + $statusCode = $response->getStatusCode(); + + // Check if it's a redirect status code (3xx) + if ($statusCode >= 300 && $statusCode < 400) { + if (!$response->hasHeader('Location')) { + $parentSpan->setStatus(StatusCode::STATUS_ERROR); + $attributeSpan->setStatus(StatusCode::STATUS_ERROR); + + throw new ApiException( + sprintf( + 'Redirect status code %d returned without a Location header', + $statusCode + ) + ); + } + return false; + } + + return true; + } + + /** * Authenticates and executes the request * @@ -620,6 +687,9 @@ private function throwFailedResponse(ResponseInterface $response, ?array $errorM if ($statusCode >= 200 && $statusCode < 400) { return; } + if (!$this->isRedirectMissingLocation($response, $span, $span)) { + return; + } $errorSpan = $this->tracer->spanBuilder('throwFailedResponses') ->setParent($currentContext) ->startSpan(); @@ -681,7 +751,6 @@ private function throwFailedResponse(ResponseInterface $response, ?array $errorM $spanForDeserialization->end(); } - if ($error && is_subclass_of($error, ApiException::class)) { $span->setAttribute(self::ERROR_BODY_FOUND_ATTRIBUTE_NAME, true); $error->setResponseStatusCode($response->getStatusCode()); @@ -689,6 +758,7 @@ private function throwFailedResponse(ResponseInterface $response, ?array $errorM $errorSpan->recordException($error, ['know_error' => true, 'message' => $responseBodyContents]); throw $error; } + $otherwise = new ApiException("Unsupported error type " . get_debug_type($error)); $otherwise->setResponseStatusCode($response->getStatusCode()); $otherwise->setResponseHeaders($response->getHeaders()); diff --git a/packages/http/guzzle/tests/GuzzleRequestAdapterTest.php b/packages/http/guzzle/tests/GuzzleRequestAdapterTest.php index 36d96e7..d29445c 100644 --- a/packages/http/guzzle/tests/GuzzleRequestAdapterTest.php +++ b/packages/http/guzzle/tests/GuzzleRequestAdapterTest.php @@ -113,7 +113,24 @@ public function testGetPsrRequestFromRequestInformation(): void public function testSendAsync(): void { - $requestAdapter = $this->mockRequestAdapter([new Response(200, ['Content-Type' => $this->contentType])]); + $requestAdapter = $this->mockRequestAdapter([new Response(200, ['Content-Type' => $this->contentType], '{"id": 1}')]); + + $parseNode = $this->createStub(ParseNode::class); + $parseNode->method('getObjectValue') + ->willReturn(new TestUser(1)); + + $parseNodeFactory = $this->createStub(ParseNodeFactory::class); + $parseNodeFactory->method('getRootParseNode') + ->willReturn($parseNode); + + $requestAdapter = new GuzzleRequestAdapter( + $this->authenticationProvider, + $parseNodeFactory, + $this->createMock(SerializationWriterFactory::class), + new Client(['handler' => new MockHandler([new Response(200, ['Content-Type' => $this->contentType], '{"id": 1}')])]) + ); + $requestAdapter->setBaseUrl($this->baseUrl); + $promise = $requestAdapter->sendAsync($this->requestInformation, array(TestUser::class, 'createFromDiscriminatorValue')); $this->assertInstanceOf(TestUser::class, $promise->wait()); }