Http client events do not fire when using setClient method

[ZacharyDuBois]

Laravel Version

10.48.4

PHP Version

8.3

Database Driver & Version

PostgreSQL 15 via homebrew on macOS 14.4

Description

When using the Laravel HTTP Client when setting a client (using setClient(...)), the Http event ResponseReceived does not fire (resulting in Telescope not monitoring them as a side effect). Appears to be related to https://github.com/laravel/framework/blob/10.x/src/Illuminate/Http/Client/PendingRequest.php#L1408. The dispatcher is present at this point but $this->request is null.

Also ConnectionFailed event does not fire and throws an exception when the connection fails.

Illuminate\Http\Client\Events\ConnectionFailed::__construct(): Argument #1 ($request) must be of type Illuminate\Http\Client\Request, null given, called in /vendor/laravel/framework/src/Illuminate/Http/Client/PendingRequest.php on line 1424

Steps To Reproduce

use Illuminate\Http\Client\Events\ResponseReceived;

Route::get('/test', function () {
    $client = Http::buildClient();

    Event::listen(ResponseReceived::class, function (ResponseReceived $received) {
        dd($received);
    });

    $res1 = Http::setClient($client)->get('http://127.0.0.1:7777');
    $res2 = Http::setClient($client)->get('http://127.0.0.1:7777');
    $res3 = Http::setClient($client)->get('http://127.0.0.1:7777');

    return [$res1->json('id'), $res2->json('id'), $res3->json('id')];
});

(the very simple script I was using with php -S 127.0.0.1:7777 cookie.php to test cookie persistence)

<?php
session_start();
echo json_encode(['id'=>session_id()]);

To reproduce the connection failure, set a low connectionTimeout on the requests and just point it to a non-existent port/webserver.

Then endpoint here was something I was running to attempt to get cookies to easily persist across requests (as request by Acumatica's session management). It is returning a simple JSON KV pair {"id":"ft2m2dgka630d6m8odp6sa8dnb"}.

The requests do work and I can confirm this cookie checking script is receiving all three. It is purely something isn't happening right in the internals of PendingRequest when handling the request when setClient(...) is called.

[ZacharyDuBois]

FYI - Spinning up a quick instance of Laravel 11 to see if it exists there as well.

Update:
Still an issue on Laravel 11. Just a behavior difference with the ConnectionFailed event/handling.
The ConnectionFailed event still does not throw but I did get an actual ConnectionException that time.

[crynobone]

Hey there, thanks for reporting this issue.

We'll need more info and/or code to debug this further. Can you please create a repository with the command below, commit the code that reproduces the issue as one separate commit on the main/master branch and share the repository here?

Please make sure that you have the latest version of the Laravel installer in order to run this command. Please also make sure you have both Git & the GitHub CLI tool properly set up.

laravel new bug-report --github="--public"

Do not amend and create a separate commit with your custom changes. After you've posted the repository, we'll try to reproduce the issue.

Thanks!

[ZacharyDuBois]

@crynobone No problem!

Created at: ZacharyDuBois/bug-report@a26451f

Verified using Valet as well. Difference from the project I was working on is I did not install Telescope on this one (which doesn't really impact anything as the Telescope watcher is just listening for those HTTP Client events to fire. Other difference for simplicity, I set this up with SQLite instead of Postgres but that shouldn't play into it (I'd hope).

• http://bug-report.test/successful-http-request <- Results in the ResponseReceived event never firing.
• http://bug-report.test/successful-http-request-without-shared <- Results in the ResponseReceived event firing x3 (expected behavior).
• http://bug-report.test/bad-host <- Results in the ConnectionFailed event firing and an exception on Laravel 11, not Laravel 10.
• http://bug-report.test/bad-host-without-shared <- Results in the above in both Laravel 10 and 11 (expected).

[plumthedev]

I will take a look there 👀

[plumthedev]

So, during Http::buildClient Laravel is composing new Pending Request instance and applies beforeSendingCallbacks:

$this->beforeSendingCallbacks = collect([function (Request $request, array $options, PendingRequest $pendingRequest) {
    $pendingRequest->request = $request;
    $pendingRequest->cookies = $options['cookies'];

    $pendingRequest->dispatchRequestSendingEvent();
}])

$pendingRequest is an instance of Pending Request created during Http::buildClient call as then we are binding this callback. Then you are setting the client using Http::setClient and this client has applied this callback with value of $pendingRequest as Pending Request created during the Http::buildClient.

Callback is setting request on wrong instance of object, it should on this used during HTTP methods but uses that from buildClient so that's why event is never fired.

The error stems from the assumptions of how HTTP factory and pending request work, namely that the objects created by this factory are mutable, while in my opinion they should be immutable.

Route::get('/mutable', function (Request $request) {
    $client = Http::withQueryParameters(['foo' => 'bar']);
    $client->dump();

    // first service method
    // with foo=bar
    $client->get('https://httpbin.org/status/200');

    // second service method
    // with foo=bar and bar=foo
    $client->withQueryParameters(['bar' => 'foo']);
    $client->get('https://httpbin.org/status/200');

    // third service method
    // with foo=bar and bar=foo and baz=foo
    $client->withQueryParameters(['baz' => 'foo']);
    $client->get('https://httpbin.org/status/200');
});

In example above, if you have 3 different methods of class which are using one injected by constructor pending request every call of next method will be secretly gluing next query params what can be quite unexpected.

[ZacharyDuBois]

So it sounds like either this could be an even larger issue or completely thrown out as calling buildClient and setClient as useless. There are definitely some decent use cases for wanting to reuse a Guzzle client. Honestly, might get (albeit slim) performance increase by building a Guzzle client once per request/job. Right now, it instantiates a new client each call to Http.

[plumthedev]

So, the issue is not related to setClient method but to buildClient.
Http::buildClient is defined on PendingRequest level because it is applying callbacks and dispatches events regarding to this request. Tough case.

[driesvints]

I don't think we document using setClient and buildClient atm in the way the orginal example shows so right now we don't support this being used in this way, sorry. If there's anything that can be improved we'd appreciate a PR, thanks.

[ZacharyDuBois]

Workaround for those doing this only for cookie functionality:

I have created a wrapper class like this:

<?php

namespace App\Clients;

use GuzzleHttp\Cookie\CookieJar;
use Illuminate\Http\Client\PendingRequest;
use Illuminate\Support\Facades\Http;

class CookieClient
{
    private CookieJar $jar;

    public function __construct()
    {
        $this->jar = new CookieJar(true);
    }

    public function request(): PendingRequest
    {
        $pendingRequest = Http::withOptions(['cookies' => $this->jar]);

        // Other standard presets you want between requests

        return $pendingRequest;
    }
}

Each time you are ready to make a request, you can use the normal Laravel HTTP client, the only difference is it's instantiating a new client and presetting with the same CookieJar each time.

$client = new CookieClient();

$res1 = $client->request()->get('http://127.0.0.1:7777'); // Sets cookie
$res2 = $client->request()->get('http://127.0.0.1:7777'); // Maintains session on subsequent requests.
$res3 = $client->request()->get('http://127.0.0.1:7777'); // Same as above

Hacky workaround until there is a decent overhaul of the Laravel HTTP client. The Laravel client would need to instantiate Guzzle only once for better cookie support.

Thanks for your help @plumthedev @crynobone !