openai-php
HttpTransporter object to return error instead of throwing it.
Hope everyone is doing well. Currently the architecture of the component is such that the src/Transporters/HttpTransporter.php under its requestObject method, checks for presence of $response['error'], if there's none, it returns the response object (passes it the corresponding CreateResponse class ). I suggest to redefine the behavior of the transporter so that it will return any possible error and pass it along to the CreateResponse class. I propose the CreateResponse class to process the response attributes, including those with errors and ultimately forward the error details to the application in which it has initiated the API call. The benefits of my idea:
- In line with TDD approach, the developer can easily check for the presence of the error message, statusCode, and deal with them accordingly;
- The developer will be able to log those errors easily and use it for overall enhancement and debugging of his host application;
- The developer can provide a more user-friendly error message to the end-user based on the returned/received error;
- The developer will be able to provide translation of the errors to the end-user.
Currently, I don't see how these 4 things are possible under the currently behavior of the transporter. The following test can show better what I mean:
public function test_client_handles_error_response_correctly(): void
{
$client = OpenAI::client('sk-````');
$response = $client->completions()->create([
'prompt' => 'PHP is',
'model' => 'wrongModel', //invoke error
'max_tokens' => 20,
'temperature' => 0,
]);
// Make assertions
$this->assertNotEmpty($response->error["message"]);
$this->assertEquals(500, $response->error["status_code"]);
}
Yes please, upvoting for that! Better error handling would be much appreciated!
Just to be clear, I have already done the needed changes in the codes on my local machine and the above test passes nicely. I have achieved this with minimum changes necessary, though one may suggest bigger and more fundamental changes to the structure of the whole component, I found it not needed at presence. Plus, I stopped using this component in favor of using the native Symfony Httpclient instead of this nice component, due to its entangled use of Guzzle.
Hi @ali-m2020
Thank you for input. But I can't see what the benefit of not throwing an exception should be, because in my opinion all the 4 things you mentioned should be possible with the current implementation.
Furthermore, not throwing an exception and returning a valid response object with a new "error" attribute would require an additional check after every request to ensure the returned response has no error. With the current implementation this check is not required as the developer can be sure, that every response received is valid and no error has occurred.
If I misunderstand your request or there is a concrete error which you are not able to handle properly, please let us know.
@kamilbaranek Why did you upvote for this feature? What change would you expect?
