Skip to content
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

Http::globalRequestMiddleware not reflected in RequestSending event #51980

Closed
sky93 opened this issue Jul 1, 2024 · 7 comments
Closed

Http::globalRequestMiddleware not reflected in RequestSending event #51980

sky93 opened this issue Jul 1, 2024 · 7 comments

Comments

@sky93
Copy link

sky93 commented Jul 1, 2024

Laravel Version

11.13.0

PHP Version

8.3.6

Database Driver & Version

No response

Description

Hello. I change user agent in Http::globalRequestMiddleware like below according to this link:

Http::globalRequestMiddleware(fn ($request) => $request->withHeader(
    'User-Agent', 'Example Application/1.0'
));

When I use RequestSending event like the example in document I get another user agent:

public function LogHttpClientRequests(RequestSending $event)
{
    Log::info('Request Sending Headers:', $event->request->headers());
}

I get GuzzleHttp/7 in log file:

local.INFO: Request Sending Headers: {"User-Agent":["GuzzleHttp/7"]} 

The expected User-Agent is Example Application/1.0. This happens with any modifications in header. The request that is actually get sent has the correct User Agent but the log shows invalid output. How can I get this fixed?

Steps To Reproduce

1- Create app/Providers/HttpClientServiceProvider.php:

public function register(): void
{
}

public function boot(): void
{
    Http::globalRequestMiddleware(fn ($request) => $request->withHeader(
        'User-Agent',
        'Example Application/1.0'
    ));
}

2- Register event in app/Providers/EventServiceProvider.php:

protected $listen = [
    RequestSending::class => [
        LogHttpClientRequests::class,
    ],
    ResponseReceived::class => [
        LogHttpClientRequests::class,
    ],
    ConnectionFailed::class => [
        LogHttpClientRequests::class,
    ],
];

3- Create app/Listeners/LogHttpClientRequests.php:

public function LogHttpClientRequests(RequestSending $event): void
{
    Log::info('Request Sending Headers:', $event->request->headers());
}

4- Run Http::get('https://example.com') in tinker.

@sky93 sky93 changed the title Http::globalRequestMiddleware not reflected RequestSending event Http::globalRequestMiddleware not reflected in RequestSending event Jul 2, 2024
@driesvints
Copy link
Member

I managed to reproduce this. Not sure what's going on but would appreciate any insights here.

Copy link

github-actions bot commented Jul 2, 2024

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

@sethsandaru
Copy link
Contributor

I managed to reproduce this. Not sure what's going on but would appreciate any insights here.

https://github.com/laravel/framework/blob/11.x/src/Illuminate/Http/Client/PendingRequest.php#L1260-L1264

So we dispatch RequestSending event before invoking the global middleware(s). That would be the reason why the custom user-agent wasn't presented in the RequestSending event

@driesvints
Copy link
Member

I have a feeling this has come up before and that we labeled it as the expected behaviour. But unsure which issue that was. We might have to adjust the docs a little bit here.

@eduance
Copy link
Contributor

eduance commented Jul 29, 2024

@driesvints

This is quite funny, we add the middleware to the Guzzle stack around June 24th, 2020. We then receive a first commit on Jan 25th, 2022 (63cb622) that solves a similar issue, the middleware is not being added before the request is started. This commit moves the $stack->push($this->buildStubHandler()) underneath the middleware.

The reason for this is that you first want your middleware to be on each subsequent request. We receive an exact similar commit last year (f03e652), this moves the $stack->push($this->buildRecorderHandler()); underneath the middleware.

I'm now proud to announce, that after 3 years hereby I'll be moving the last line so whenever we send a request, we always have the middleware hooked. 😆

@eduance
Copy link
Contributor

eduance commented Aug 3, 2024

@driesvints This should be solved now.

@driesvints
Copy link
Member

Thanks @eduance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants