-
-
Notifications
You must be signed in to change notification settings - Fork 199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to Laravel HTTP client for compatibility with Pulse, Telescope, Sentry etc #75
Comments
Hi @binaryfire I am not sure if this i an easy change because of the underlying client being http client agnostic. Is the Laravel HTTP Client PSR-17 compatible? |
Hi @gehrisandro. Unfortunately no, I don't think Laravel's HTTP client is PSR-17 compatible. It's a bespoke wrapper around Guzzle. |
You might find the Guzzle Watcher Telescope plugin particularly useful, as Laravel's 'HTTP Client' leverages Guzzle internally. This plugin allows you to view Guzzle's request and response logs directly in the Telescope UI. It’s important to remember, however, that Laravel's 'HTTP Client' primarily functions as a |
@andrzejkupczyk Thanks for the suggestion but it affects more than just Telescope. Several other core and third party packages integrate with the HTTP client eg. Pulse, Sentry etc. |
Hi @binaryfire If tinkered around a bit and came up with a solution, creating a PSR-18 compatible wrapper around the HTTP facade. At least for now, the wrapper is in a separate project: https://github.com/gehrisandro/laravel-http-psr18 When using the package, you have to register the OpenAI client in your ServiceProvider: $this->app->extend(ClientContract::class, static function (): Client {
return OpenAI::factory()
->withApiKey(config('openai.api_key'))
->withOrganization(config('openai.organization'))
->withHttpClient($httpClient = HttpPsr18::make(Http::withHeader('OpenAI-Beta', 'assistants=v1')))
->withStreamHandler(fn(RequestInterface $request) => $httpClient->sendRequest($request))
->make();
}); Hope this helps and would like to hear your feedback. |
Thanks @gehrisandro! I'll let you know how things go as soon as I have some time to test. |
Thanks for the solution. Unfortunately for me, this method works only for the first request. When attempting to rerun a request, it will merge the headers and result in duplicate headers values, see example below : {
"content-length": "259",
"user-agent": "GuzzleHttp/7",
"openai-beta": "assistants=v1, assistants=v1, assistants=v1",
"accept": "application/json",
"content-type": "application/json, application/json, application/json",
"host": "api.openai.com, api.openai.com",
"authorization": "********",
"openai-organization": "*********"
} This will result in an To fix that, it's better to use a new instance of use Illuminate\Support\Facades\Http;
use OpenAI;
use OpenAI\Client;
use OpenAI\Contracts\ClientContract;
use Psr\Http\Client\ClientInterface;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;
// AppServiceProvider::register
$this->app->extend(ClientContract::class, static function (): Client {
$client = new class implements ClientInterface {
public function sendRequest(RequestInterface $request): ResponseInterface
{
return Http::withHeaders($request->getHeaders())
->asJson()
->acceptJson()
->send($request->getMethod(), (string)$request->getUri(), [
'body' => $request->getBody()->getContents(),
])
->toPsrResponse();
}
};
return OpenAI::factory()
->withApiKey(config('openai.api_key'))
->withOrganization(config('openai.organization'))
->withHttpHeader('OpenAI-Beta', 'assistants=v1')
->withHttpClient($client)
->make();
}); |
The package currently uses Guzzle directly, which means requests won't appear in core tools like Pulse and Telescope. It would be great if Laravel's HTTP client could be used instead to ensure compatibility with the wider Laravel ecosystem.
It also doesn't work with things like Sentry's Laravel HTTP client integration: getsentry/sentry-laravel#797
As a side note, it would also provide a solution to this issue since Laravel's HTTP client has built in retry functionality.
The text was updated successfully, but these errors were encountered: