[11.x] Include ConnectionException in ConnectionFailed events

[alexbowers]

Currently, you receive an event (\Illuminate\Http\Client\Events\ConnectionFailed) if a HTTP Request has hit a timeout of some kind (or some other error), however, the event only has access to the request, which doesn't include any information about the nature of the failure. There is no response that has been received, but we do have an exception (\Illuminate\Http\Client\ConnectionException) that is now being passed.

[github-actions]

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

[mabdullahsari]

This was a breaking change for us...

[mabdullahsari]

We have old HTTP clients from the pre-HttpFactory era for which we've written stop-gap Guzzle middleware to dispatch Illuminate events to make them appear on Telescope. Can you please revert this? @driesvints

[mabdullahsari]

Case in point:

class HttpClientEventsMiddleware
{
    public static function make(): self
    {
        return app(self::class);
    }

    public function __construct(private readonly Dispatcher $dispatcher) {}

    public function __invoke(callable $handler)
    {
        return function ($request, array $options) use ($handler) {
            $this->dispatcher->dispatch(new RequestSending($illuminateRequest = new Request($request)));

            return $handler($request, $options)->then(
                $this->onSuccess($illuminateRequest),
                $this->onFailure($illuminateRequest)
            );
        };
    }

    protected function onSuccess(Request $request)
    {
        return function ($response) use ($request) {
            $this->dispatcher->dispatch(new ResponseReceived($request, new Response($response)));

            return $response;
        };
    }

    protected function onFailure(Request $request)
    {
        return function ($reason) use ($request) {
            $this->dispatcher->dispatch(new ConnectionFailed($request));

            return Create::rejectionFor($reason);
        };
    }
}

[alexbowers]

Ah sorry, I didn't realise this would be a feature that people are initiating themselves.

Happy for this to be reverted, and for the original PR to be focused against the next major version so that the BC can be documented properly

[mabdullahsari]

Ah sorry, I didn't realise this would be a feature that people are initiating themselves.

Happy for this to be reverted, and for the original PR to be focused against the next major version so that the BC can be documented properly

I'm sorry for this unexpected pushback, but we can't allocate resources to upgrade these old HTTP clients at this time. Thank you so much for your understanding!

[alexbowers]

Makes perfect sense. No breaking change was intended here, if I knew it would be I would have targeted the correct branch From: Muhammed Sari ***@***.***> Date: Thursday, 18 July 2024 at 15:03 To: laravel/framework ***@***.***> Cc: Alex Bowers ***@***.***>, Author ***@***.***> Subject: Re: [laravel/framework] [11.x] Include ConnectionException in ConnectionFailed events (PR #52069) Ah sorry, I didn't realise this would be a feature that people are initiating themselves. Happy for this to be reverted, and for the original PR to be focused against the next major version so that the BC can be documented properly I'm sorry for this unexpected pushback, but we can't allocate resources to upgrade these old HTTP clients at this time. Thank you so much for your understanding! — Reply to this email directly, view it on GitHub<#52069 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAGNZXTMDZQV4OEA4S6QAS3ZM7DJPAVCNFSM6AAAAABKTC4O6OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZWGYYTOMJVHA>. You are receiving this because you authored the thread.Message ID: ***@***.***>