From 0cfa823676f2cd4f7deeea6e1cd3b2d98516f301 Mon Sep 17 00:00:00 2001 From: "sina.shakouri82" Date: Thu, 10 Apr 2025 12:19:48 +0330 Subject: [PATCH 1/2] fix: add throwIfNotSuccessfulStatusCode method to validate HTTP status codes --- .../UnexpectedStatusCodeException.php | 23 +++ src/Transporters/HttpTransporter.php | 16 ++ tests/Transporters/HttpTransporter.php | 169 ++++++++++++++++++ 3 files changed, 208 insertions(+) create mode 100644 src/Exceptions/UnexpectedStatusCodeException.php diff --git a/src/Exceptions/UnexpectedStatusCodeException.php b/src/Exceptions/UnexpectedStatusCodeException.php new file mode 100644 index 00000000..5b2d9785 --- /dev/null +++ b/src/Exceptions/UnexpectedStatusCodeException.php @@ -0,0 +1,23 @@ +sendRequest(fn (): \Psr\Http\Message\ResponseInterface => $this->client->sendRequest($request)); + $this->throwIfNotSuccessfulStatusCode($response); + $contents = (string) $response->getBody(); if (str_contains($response->getHeaderLine('Content-Type'), ContentType::TEXT_PLAIN->value)) { @@ -75,6 +78,8 @@ public function requestContent(Payload $payload): string $response = $this->sendRequest(fn (): \Psr\Http\Message\ResponseInterface => $this->client->sendRequest($request)); + $this->throwIfNotSuccessfulStatusCode($response); + $contents = (string) $response->getBody(); $this->throwIfJsonError($response, $contents); @@ -91,6 +96,8 @@ public function requestStream(Payload $payload): ResponseInterface $response = $this->sendRequest(fn () => ($this->streamHandler)($request)); + $this->throwIfNotSuccessfulStatusCode($response); + $this->throwIfJsonError($response, $response); return $response; @@ -136,4 +143,13 @@ private function throwIfJsonError(ResponseInterface $response, string|ResponseIn throw new UnserializableResponse($jsonException); } } + + private function throwIfNotSuccessfulStatusCode(ResponseInterface $response): void + { + $statusCode = $response->getStatusCode(); + + if ($statusCode < 200 || $statusCode >= 300) { + throw new UnexpectedStatusCodeException($statusCode); + } + } } diff --git a/tests/Transporters/HttpTransporter.php b/tests/Transporters/HttpTransporter.php index 4fd772af..5903fbe1 100644 --- a/tests/Transporters/HttpTransporter.php +++ b/tests/Transporters/HttpTransporter.php @@ -16,6 +16,7 @@ use Psr\Http\Client\ClientInterface; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; +use \OpenAI\Exceptions\UnexpectedStatusCodeException; beforeEach(function () { $this->client = Mockery::mock(ClientInterface::class); @@ -531,3 +532,171 @@ ->and($e->getErrorType())->toBe('invalid_request_error'); }); }); + +test('request stream client error 400', function () { + $payload = Payload::create('completions', []); + + $response = new Response(400, [], json_encode([ + 'error' => [ + 'message' => 'Bad Request', + 'type' => 'client_error', + 'param' => null, + 'code' => 'bad_request', + ], + ])); + + $this->client + ->shouldReceive('sendAsyncRequest') + ->once() + ->andReturn($response); + + expect(fn () => $this->http->requestStream($payload)) + ->toThrow(function (UnexpectedStatusCodeException $e) { + expect($e->getMessage())->toBe('Unexpected status code: 400') + ->and($e->getCode())->toBe(400); + }); +}); + +test('request stream client error 401', function () { + $payload = Payload::create('completions', []); + + $response = new Response(401, [], json_encode([ + 'error' => [ + 'message' => 'Unauthorized', + 'type' => 'client_error', + 'param' => null, + 'code' => 'unauthorized', + ], + ])); + + $this->client + ->shouldReceive('sendAsyncRequest') + ->once() + ->andReturn($response); + + expect(fn () => $this->http->requestStream($payload)) + ->toThrow(function (UnexpectedStatusCodeException $e) { + expect($e->getMessage())->toBe('Unexpected status code: 401') + ->and($e->getCode())->toBe(401); + }); +}); + +test('request stream client error 403', function () { + $payload = Payload::create('completions', []); + + $response = new Response(403, [], json_encode([ + 'error' => [ + 'message' => 'Forbidden', + 'type' => 'client_error', + 'param' => null, + 'code' => 'forbidden', + ], + ])); + + $this->client + ->shouldReceive('sendAsyncRequest') + ->once() + ->andReturn($response); + + expect(fn () => $this->http->requestStream($payload)) + ->toThrow(function (UnexpectedStatusCodeException $e) { + expect($e->getMessage())->toBe('Unexpected status code: 403') + ->and($e->getCode())->toBe(403); + }); +}); + +test('request stream client error 404', function () { + $payload = Payload::create('completions', []); + + $response = new Response(404, [], json_encode([ + 'error' => [ + 'message' => 'Not Found', + 'type' => 'client_error', + 'param' => null, + 'code' => 'not_found', + ], + ])); + + $this->client + ->shouldReceive('sendAsyncRequest') + ->once() + ->andReturn($response); + + expect(fn () => $this->http->requestStream($payload)) + ->toThrow(function (UnexpectedStatusCodeException $e) { + expect($e->getMessage())->toBe('Unexpected status code: 404') + ->and($e->getCode())->toBe(404); + }); +}); + +test('request stream client error 422', function () { + $payload = Payload::create('completions', []); + + $response = new Response(422, [], json_encode([ + 'error' => [ + 'message' => 'Unprocessable Entity', + 'type' => 'client_error', + 'param' => null, + 'code' => 'unprocessable_entity', + ], + ])); + + $this->client + ->shouldReceive('sendAsyncRequest') + ->once() + ->andReturn($response); + + expect(fn () => $this->http->requestStream($payload)) + ->toThrow(function (UnexpectedStatusCodeException $e) { + expect($e->getMessage())->toBe('Unexpected status code: 422') + ->and($e->getCode())->toBe(422); + }); +}); + +test('request stream client error 429', function () { + $payload = Payload::create('completions', []); + + $response = new Response(429, [], json_encode([ + 'error' => [ + 'message' => 'Too Many Requests', + 'type' => 'client_error', + 'param' => null, + 'code' => 'too_many_requests', + ], + ])); + + $this->client + ->shouldReceive('sendAsyncRequest') + ->once() + ->andReturn($response); + + expect(fn () => $this->http->requestStream($payload)) + ->toThrow(function (UnexpectedStatusCodeException $e) { + expect($e->getMessage())->toBe('Unexpected status code: 429') + ->and($e->getCode())->toBe(429); + }); +}); + +test('request stream client error 500', function () { + $payload = Payload::create('completions', []); + + $response = new Response(500, [], json_encode([ + 'error' => [ + 'message' => 'Internal Server Error', + 'type' => 'client_error', + 'param' => null, + 'code' => 'internal_server_error', + ], + ])); + + $this->client + ->shouldReceive('sendAsyncRequest') + ->once() + ->andReturn($response); + + expect(fn () => $this->http->requestStream($payload)) + ->toThrow(function (UnexpectedStatusCodeException $e) { + expect($e->getMessage())->toBe('Unexpected status code: 500') + ->and($e->getCode())->toBe(500); + }); +}); From 2193390920368481e5edd52ad4f5f4019bd284f0 Mon Sep 17 00:00:00 2001 From: "sina.shakouri82" Date: Thu, 17 Apr 2025 20:35:47 +0330 Subject: [PATCH 2/2] Addressed review feedback in PR #553 and updated related tests --- .../UnexpectedStatusCodeException.php | 114 +++++++++++++++++- src/Transporters/HttpTransporter.php | 11 +- tests/Arch.php | 1 + tests/Transporters/HttpTransporter.php | 54 ++++----- 4 files changed, 138 insertions(+), 42 deletions(-) diff --git a/src/Exceptions/UnexpectedStatusCodeException.php b/src/Exceptions/UnexpectedStatusCodeException.php index 5b2d9785..de2ba30d 100644 --- a/src/Exceptions/UnexpectedStatusCodeException.php +++ b/src/Exceptions/UnexpectedStatusCodeException.php @@ -5,19 +5,123 @@ namespace OpenAI\Exceptions; use Exception; +use Psr\Http\Message\RequestInterface; +use Psr\Http\Message\ResponseInterface; final class UnexpectedStatusCodeException extends Exception { + private RequestInterface $request; + + private ResponseInterface $response; + /** - * Creates a new Exception instance. - * - * @param int $statusCode The HTTP status code. + * @var array{message: string|null, type: string|null, code: int|string|null} */ + private array $contents; - public function __construct(int $statusCode) + /** + * Creates a new UnexpectedStatusCodeException instance. + * + * @param int $statusCode The unexpected HTTP status code. + * @param RequestInterface $request The request that was sent. + * @param ResponseInterface $response The response that was received. + */ + public function __construct(int $statusCode, RequestInterface $request, ResponseInterface $response) { - $message = "Unexpected status code: {$statusCode}"; + $this->request = $request; + $this->response = $response; + + $body = (string) $response->getBody(); + $decoded = json_decode($body, true); + + $error = []; + if (is_array($decoded) && isset($decoded['error']) && is_array($decoded['error'])) { + $error = $decoded['error']; + } + + $this->contents = [ + 'message' => ($error['message'] ?? null) ?: "Unexpected status code: {$statusCode}", + 'type' => $error['type'] ?? null, + 'code' => $error['code'] ?? null, + ]; + + $message = $this->contents['message']; + if (is_array($message)) { + $message = implode(PHP_EOL, $message); + } parent::__construct($message, $statusCode); } + + /** + * Returns the HTTP status code of the response. + */ + public function getStatusCode(): int + { + return $this->response->getStatusCode(); + } + + /** + * Returns the error message. + */ + public function getErrorMessage(): string + { + return $this->getMessage(); + } + + /** + * Returns the error code if available. + */ + public function getErrorCode(): string|int|null + { + return $this->contents['code'] ?? null; + } + + /** + * Returns the error type if available. + */ + public function getErrorType(): ?string + { + return $this->contents['type'] ?? null; + } + + /** + * Returns the request that was made. + */ + public function getRequest(): RequestInterface + { + return $this->request; + } + + /** + * Returns the response that was received. + */ + public function getResponse(): ResponseInterface + { + return $this->response; + } + + /** + * Returns a string representation of the request. + */ + public function getRequestDetails(): string + { + return sprintf( + 'Request: %s %s', + $this->request->getMethod(), + $this->request->getUri() + ); + } + + /** + * Returns a string representation of the response. + */ + public function getResponseDetails(): string + { + return sprintf( + 'Response: %d %s', + $this->response->getStatusCode(), + $this->response->getReasonPhrase() + ); + } } diff --git a/src/Transporters/HttpTransporter.php b/src/Transporters/HttpTransporter.php index b42c219b..366027b9 100644 --- a/src/Transporters/HttpTransporter.php +++ b/src/Transporters/HttpTransporter.php @@ -20,6 +20,7 @@ use OpenAI\ValueObjects\Transporter\Response; use Psr\Http\Client\ClientExceptionInterface; use Psr\Http\Client\ClientInterface; +use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; /** @@ -49,7 +50,7 @@ public function requestObject(Payload $payload): Response $response = $this->sendRequest(fn (): \Psr\Http\Message\ResponseInterface => $this->client->sendRequest($request)); - $this->throwIfNotSuccessfulStatusCode($response); + $this->throwIfNotSuccessfulStatusCode($response, $request); $contents = (string) $response->getBody(); @@ -78,7 +79,7 @@ public function requestContent(Payload $payload): string $response = $this->sendRequest(fn (): \Psr\Http\Message\ResponseInterface => $this->client->sendRequest($request)); - $this->throwIfNotSuccessfulStatusCode($response); + $this->throwIfNotSuccessfulStatusCode($response, $request); $contents = (string) $response->getBody(); @@ -96,7 +97,7 @@ public function requestStream(Payload $payload): ResponseInterface $response = $this->sendRequest(fn () => ($this->streamHandler)($request)); - $this->throwIfNotSuccessfulStatusCode($response); + $this->throwIfNotSuccessfulStatusCode($response, $request); $this->throwIfJsonError($response, $response); @@ -144,12 +145,12 @@ private function throwIfJsonError(ResponseInterface $response, string|ResponseIn } } - private function throwIfNotSuccessfulStatusCode(ResponseInterface $response): void + private function throwIfNotSuccessfulStatusCode(ResponseInterface $response, RequestInterface $request): void { $statusCode = $response->getStatusCode(); if ($statusCode < 200 || $statusCode >= 300) { - throw new UnexpectedStatusCodeException($statusCode); + throw new UnexpectedStatusCodeException($statusCode, $request, $response); } } } diff --git a/tests/Arch.php b/tests/Arch.php index f7c8b14a..9ddeadc4 100644 --- a/tests/Arch.php +++ b/tests/Arch.php @@ -19,6 +19,7 @@ ->expect('OpenAI\Exceptions') ->toOnlyUse([ 'Psr\Http\Client', + 'Psr\Http\Message', ])->toImplement(Throwable::class); test('resources')->expect('OpenAI\Resources')->toOnlyUse([ diff --git a/tests/Transporters/HttpTransporter.php b/tests/Transporters/HttpTransporter.php index 5903fbe1..dd053dc3 100644 --- a/tests/Transporters/HttpTransporter.php +++ b/tests/Transporters/HttpTransporter.php @@ -6,6 +6,7 @@ use OpenAI\Enums\Transporter\ContentType; use OpenAI\Exceptions\ErrorException; use OpenAI\Exceptions\TransporterException; +use OpenAI\Exceptions\UnexpectedStatusCodeException; use OpenAI\Exceptions\UnserializableResponse; use OpenAI\Transporters\HttpTransporter; use OpenAI\ValueObjects\ApiKey; @@ -16,7 +17,6 @@ use Psr\Http\Client\ClientInterface; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; -use \OpenAI\Exceptions\UnexpectedStatusCodeException; beforeEach(function () { $this->client = Mockery::mock(ClientInterface::class); @@ -102,7 +102,7 @@ ->andReturn($response); expect(fn () => $this->http->requestObject($payload)) - ->toThrow(function (ErrorException $e) { + ->toThrow(function (UnexpectedStatusCodeException $e) { expect($e->getMessage())->toBe('Incorrect API key provided: foo. You can find your API key at https://platform.openai.com.') ->and($e->getErrorMessage())->toBe('Incorrect API key provided: foo. You can find your API key at https://platform.openai.com.') ->and($e->getErrorCode())->toBe('invalid_api_key') @@ -129,7 +129,7 @@ ->andReturn($response); expect(fn () => $this->http->requestObject($payload)) - ->toThrow(function (ErrorException $e) { + ->toThrow(function (UnexpectedStatusCodeException $e) { expect($e->getMessage())->toBe('That model is currently overloaded with other requests. You can ...') ->and($e->getErrorMessage())->toBe('That model is currently overloaded with other requests. You can ...') ->and($e->getErrorCode())->toBeNull() @@ -155,7 +155,7 @@ ->andReturn($response); expect(fn () => $this->http->requestObject($payload)) - ->toThrow(function (ErrorException $e) { + ->toThrow(function (UnexpectedStatusCodeException $e) { expect($e->getMessage())->toBe('The model `gpt-42` does not exist') ->and($e->getErrorMessage())->toBe('The model `gpt-42` does not exist') ->and($e->getErrorCode())->toBeNull() @@ -181,7 +181,7 @@ ->andReturn($response); expect(fn () => $this->http->requestObject($payload)) - ->toThrow(function (ErrorException $e) { + ->toThrow(function (UnexpectedStatusCodeException $e) { expect($e->getMessage())->toBe('The model `gpt-42` does not exist') ->and($e->getErrorMessage())->toBe('The model `gpt-42` does not exist') ->and($e->getErrorCode())->toBe(123) @@ -207,7 +207,7 @@ ->andReturn($response); expect(fn () => $this->http->requestObject($payload)) - ->toThrow(function (ErrorException $e) { + ->toThrow(function (UnexpectedStatusCodeException $e) { expect($e->getMessage())->toBe('You exceeded your current quota, please check') ->and($e->getErrorMessage())->toBe('You exceeded your current quota, please check') ->and($e->getErrorCode())->toBe('quota_exceeded') @@ -236,7 +236,7 @@ ->andReturn($response); expect(fn () => $this->http->requestObject($payload)) - ->toThrow(function (ErrorException $e) { + ->toThrow(function (UnexpectedStatusCodeException $e) { expect($e->getMessage())->toBe('Invalid schema for function \'get_current_weather\':'.PHP_EOL.'In context=(\'properties\', \'location\'), array schema missing items') ->and($e->getErrorMessage())->toBe('Invalid schema for function \'get_current_weather\':'.PHP_EOL.'In context=(\'properties\', \'location\'), array schema missing items') ->and($e->getErrorCode())->toBeNull() @@ -262,9 +262,8 @@ ->andReturn($response); expect(fn () => $this->http->requestObject($payload)) - ->toThrow(function (ErrorException $e) { - expect($e->getMessage())->toBe('invalid_api_key') - ->and($e->getErrorMessage())->toBe('invalid_api_key') + ->toThrow(function (UnexpectedStatusCodeException $e) { + expect($e->getMessage())->toBe('Unexpected status code: 404') ->and($e->getErrorCode())->toBe('invalid_api_key') ->and($e->getErrorType())->toBe('invalid_request_error'); }); @@ -288,9 +287,8 @@ ->andReturn($response); expect(fn () => $this->http->requestObject($payload)) - ->toThrow(function (ErrorException $e) { - expect($e->getMessage())->toBe('123') - ->and($e->getErrorMessage())->toBe('123') + ->toThrow(function (UnexpectedStatusCodeException $e) { + expect($e->getMessage())->toBe('Unexpected status code: 404') ->and($e->getErrorCode())->toBe(123) ->and($e->getErrorType())->toBe('invalid_request_error'); }); @@ -314,9 +312,8 @@ ->andReturn($response); expect(fn () => $this->http->requestObject($payload)) - ->toThrow(function (ErrorException $e) { - expect($e->getMessage())->toBe('Unknown error') - ->and($e->getErrorMessage())->toBe('Unknown error') + ->toThrow(function (UnexpectedStatusCodeException $e) { + expect($e->getMessage())->toBe('Unexpected status code: 404') ->and($e->getErrorCode())->toBeNull() ->and($e->getErrorType())->toBe('invalid_request_error'); }); @@ -473,7 +470,7 @@ ->andReturn($response); expect(fn () => $this->http->requestContent($payload)) - ->toThrow(function (ErrorException $e) { + ->toThrow(function (UnexpectedStatusCodeException $e) { expect($e->getMessage())->toBe('Incorrect API key provided: foo. You can find your API key at https://platform.openai.com.') ->and($e->getErrorMessage())->toBe('Incorrect API key provided: foo. You can find your API key at https://platform.openai.com.') ->and($e->getErrorCode())->toBe('invalid_api_key') @@ -525,7 +522,7 @@ ->andReturn($response); expect(fn () => $this->http->requestStream($payload)) - ->toThrow(function (ErrorException $e) { + ->toThrow(function (UnexpectedStatusCodeException $e) { expect($e->getMessage())->toBe('Incorrect API key provided: foo. You can find your API key at https://platform.openai.com.') ->and($e->getErrorMessage())->toBe('Incorrect API key provided: foo. You can find your API key at https://platform.openai.com.') ->and($e->getErrorCode())->toBe('invalid_api_key') @@ -541,7 +538,6 @@ 'message' => 'Bad Request', 'type' => 'client_error', 'param' => null, - 'code' => 'bad_request', ], ])); @@ -552,7 +548,7 @@ expect(fn () => $this->http->requestStream($payload)) ->toThrow(function (UnexpectedStatusCodeException $e) { - expect($e->getMessage())->toBe('Unexpected status code: 400') + expect($e->getMessage())->toBe('Bad Request') ->and($e->getCode())->toBe(400); }); }); @@ -565,7 +561,6 @@ 'message' => 'Unauthorized', 'type' => 'client_error', 'param' => null, - 'code' => 'unauthorized', ], ])); @@ -576,7 +571,7 @@ expect(fn () => $this->http->requestStream($payload)) ->toThrow(function (UnexpectedStatusCodeException $e) { - expect($e->getMessage())->toBe('Unexpected status code: 401') + expect($e->getMessage())->toBe('Unauthorized') ->and($e->getCode())->toBe(401); }); }); @@ -589,7 +584,6 @@ 'message' => 'Forbidden', 'type' => 'client_error', 'param' => null, - 'code' => 'forbidden', ], ])); @@ -600,7 +594,7 @@ expect(fn () => $this->http->requestStream($payload)) ->toThrow(function (UnexpectedStatusCodeException $e) { - expect($e->getMessage())->toBe('Unexpected status code: 403') + expect($e->getMessage())->toBe('Forbidden') ->and($e->getCode())->toBe(403); }); }); @@ -613,7 +607,6 @@ 'message' => 'Not Found', 'type' => 'client_error', 'param' => null, - 'code' => 'not_found', ], ])); @@ -624,7 +617,7 @@ expect(fn () => $this->http->requestStream($payload)) ->toThrow(function (UnexpectedStatusCodeException $e) { - expect($e->getMessage())->toBe('Unexpected status code: 404') + expect($e->getMessage())->toBe('Not Found') ->and($e->getCode())->toBe(404); }); }); @@ -637,7 +630,6 @@ 'message' => 'Unprocessable Entity', 'type' => 'client_error', 'param' => null, - 'code' => 'unprocessable_entity', ], ])); @@ -648,7 +640,7 @@ expect(fn () => $this->http->requestStream($payload)) ->toThrow(function (UnexpectedStatusCodeException $e) { - expect($e->getMessage())->toBe('Unexpected status code: 422') + expect($e->getMessage())->toBe('Unprocessable Entity') ->and($e->getCode())->toBe(422); }); }); @@ -661,7 +653,6 @@ 'message' => 'Too Many Requests', 'type' => 'client_error', 'param' => null, - 'code' => 'too_many_requests', ], ])); @@ -672,7 +663,7 @@ expect(fn () => $this->http->requestStream($payload)) ->toThrow(function (UnexpectedStatusCodeException $e) { - expect($e->getMessage())->toBe('Unexpected status code: 429') + expect($e->getMessage())->toBe('Too Many Requests') ->and($e->getCode())->toBe(429); }); }); @@ -685,7 +676,6 @@ 'message' => 'Internal Server Error', 'type' => 'client_error', 'param' => null, - 'code' => 'internal_server_error', ], ])); @@ -696,7 +686,7 @@ expect(fn () => $this->http->requestStream($payload)) ->toThrow(function (UnexpectedStatusCodeException $e) { - expect($e->getMessage())->toBe('Unexpected status code: 500') + expect($e->getMessage())->toBe('Internal Server Error') ->and($e->getCode())->toBe(500); }); });