Adopt php-http/guzzle6-adapter 2.x to get PSR-18 support without losing Guzzle's async support

Given that Drupal is using Symfony components, is there any compelling reason not to use Guzzle, considering that third-party modules should not deal directly with Guzzle classes, but with the http_client service?

Do this could really help switch to another http client implementation and might be a necessary step to do this. I think both guzzle and symfony's client have https://www.php-fig.org/psr/psr-18/ bridges available.

As a first step, can we switch to using PSR-18 interfaces? What are the BC implications of this?

@kim.pepper that seems like a great first step. We could

1. Add a PSR-18 http client (maybe based on a bridge to Guzzle)
2. Concert all core usages of the http_client service to the PSR-18 version and deprecate the http_client service.

OK. I'll make a start.

Here's an initial implementation that pulls in the composer deps, and defines services.

I attempted to write a test to demonstrate it works, but ran into issues making external requests from a kernel test.

Posting it here for some initial feedback.

Referenced issues are against 8.8 branch

I love PSR-18, and use it a lot, so I am all for this approach.

Just one thing to consider is that PSR-18 only handles synchronous HTTP calls. It will return a PSR-7 response for a PSR-7 request that it is given. It does not consider asynchronous requests and the return of promises. This is something Guzzle does, and is something most HTTP clients do, all with their own propriatary implementations at this time.

I don't have any answers for how the switchover would work, as I do not use async a lot, but it's just something to bear in mind - you may have to leave access to Guzzle around for a while until there is a more standardised way to handle async.

Link to Guzzle issue about PSR-18 https://github.com/guzzle/guzzle/issues/2186

The #1 concern raised her so far is that Guzzle offers capabilities (specifically, async request handling which enables network concurrency) that we consider important or even essential, that PSR-18 does not prescribe nor guarantees. Meaning that if we were to move to a PSR-18-only API, we'd lose access to those capabilities.

The PSR-18 announcement blog post at https://medium.com/php-fig/psr-18-the-php-standard-for-http-clients-3254... explicitly acknowledges this. The reason: there is no PSR for promises. Once a PSR for promises exists, a new PSR will be created that extends PSR-18 with async support.

Nothing seems to be happening in https://github.com/guzzle/guzzle/issues/2186 nor in https://github.com/guzzle/guzzle/pull/2122 primarily because https://github.com/php-http/guzzle6-adapter already exists, and PSR-18 itself is incapable of providing the full feature set that Guzzle has (notably, again, async support).

The https://symfony.com/blog/new-in-symfony-4-3-httpclient-component provides multiple clients: a native one, a curl-based one (which supports parallel/async requests) and a PSR-18 one. The PSR-18 one again does not have async support. So moving to that would still leave us in the same place, while being more disruptive than only adopting https://github.com/php-http/guzzle6-adapter for getting PSR-18 support.

So, given the inability to keep our feature set if we move to PSR-18 (and as comments show there's clear needs for those features), I propose to add a dependency on php-http/guzzle6-adapter instead. That seems to be the only workable solution.

I'm glancing through all of this quickly, so correct me if I'm wrong here. As I understand it, Guzzle 6 + adapter will provide a single client that is PSR-18 compatible, and is also Httplug async client compatible.

Running with symfony http client, we would get the standard async compatible Symfony client, along with a PSR-18 wrapper for that which basically squashes the async capabilities and makes all requests blocking. So we could essentially provide both a PSR-18 client and an async client.

Honestly, given the uncertainty about FIG's future handling of promises and async, and the fact that we will probably need to change in 2 years again anyway, I'm not sure which solution would actually be better for Drupal 9.

I like removing guzzle as a dependency and trying to keep as much as possible in symfony. Symfony's http-client claims to be compatible with Httplug with a wrapper too, but that seems to only apply to the non-async client. The guzzle6 adaptor would switch the client interface from the guzzle async interface to the httplug async interface anyway, so looks like we are switching async interfaces either way.

That being said, guzzle+adaptor does provide both within the same client.

I'm thinking that long term, when FIG comes up with the promises and async PSRs, they will likely make it reasonably easy to add the async interface into existing PSR clients, so this makes me think that keeping the async client separate from the psr-18 client right now might make things easier down the road, as opposed to having the async stuff in the PSR-18 client and then having to change it all to meet some new future FIG spec. So I think right now I am leaning toward Symfony's http-client, and providing separate async and psr-18 clients, but I am certainly not entrenched in that position.

Deleted. Not worth it.

Can we get an issue summary update here outlining the benefit/reasons for adapting PSR-18?
There are plenty of other PSRs that Drupal doesn't opt-in to, so we do have a choice here. If there is a future PSR coming that adds async support, perhaps its best to wait until that is available and then adapt once instead of once now at the cost of Guzzle features and once again later when the subsequent PSR is released.

Also, with regards to maintenance etc of Guzzle, it is a key tenant of the AWS PHP SDK. So I don't think its going to be going away anytime soon.

1. #19:
   

   I like removing guzzle as a dependency and trying to keep as much as possible in symfony.

2. My concern with this is that we are changing everything for every Drupal module using a HTTP client with zero functional gain: we force everyone to change their code without gain to the end user.
3. #21Thanks for framing that so clearly. That's what I've been leaning towards in my head, but I didn't know about Drupal explicitly choosing to not opt in to certain PSRs. I've never been involved with Drupal adopting PSRs, so I appreciate your guidance here! 🙏
4. WRT Guzzle EOL, I also opened https://github.com/guzzle/guzzle/issues/2358.

I'll update the issue summary.

It sounds like Guzzle's PSR-18 support is just held back on decisions around which branches of Guzzle will require PHP 7, so they will have it eventually: https://github.com/guzzle/guzzle/issues/2358#issuecomment-530487682

That said, there's no published timeline for it at present, so if we need PSR-18 in the meanwhile, the adapter is probably the way to go.

Yay, @xjm and @alexpott agreed with closing #3047294: Switch to symfony/http-client as a duplicate of this, and with my analysis in #18.

Issue update following shortly.

@mikelutz pointed out that even if we decide to adopt php-http/guzzle6-adapter, we still need to decide between the PHP 5-compatible version 1.1.1 and the PHP 7.1-requiring version 2.0.1.

Which is similar to the situation at #3043471: Replace the DiactorosFactory message factory in symfony/psr-http-message-bridge with a PSR-17 compliant message factory, but different: there we have no choice but to wait for PHP 7.1, here we could choose to:

1. adopt version 1.1.1 of php-http/guzzle6-adapter now and update it to version 2.0.1 later.
2. or wait until Drupal requires PHP >=7.1 and only then adopt php-http/guzzle6-adapter

Hah, @mikelutz and I missed something while we were talking about #26! 😅

Actually, the PHP 5-compatible version 1.1.1 does not provide PSR-18 support, it only is adapting Guzzle to HttPlug (which is the main inspiration of PSR-18). "psr/http-client-implementation": "1.0" is not in https://github.com/php-http/guzzle6-adapter/blob/v1.1.1/composer.json#L26, but it is in https://github.com/php-http/guzzle6-adapter/blob/v2.0.1/composer.json#L28.

_{(Note: @mikelutz pointed out that HttPlug does have the same interface as PSR-18 without the return types. But that's still not PSR-18 itself, so still doesn't get us where we need to be.)}

So, we need the PHP 7.1-requiring version 2.x. Which means this issue is blocked on PHP 7.1 being required by Drupal core. (Just like #3043471: Replace the DiactorosFactory message factory in symfony/psr-http-message-bridge with a PSR-17 compliant message factory.)

Yeah, the Guzzle contributor pointed out something I'd missed previously which is that the PSR itself has the PHP version requirement. Which is... special. I'm not sure this needs doing in 8.9/9.0 even if we start requiring 7.1 for 8.9 (which I'm still not totally sure about).

Meantime async in progress of adoption into SF http-client https://github.com/symfony/symfony/pull/35115

Core in D9 requires PHP 7.3, so this is unblocked.

Option #3 in summary is not true as symfony/http-client is async initially.
Moreover since 5.1 it has portable http/2 support https://symfony.com/blog/new-in-symfony-5-1-portable-http-2-implementation

Also Drupal testing depends on GoutteDriver which is deprecated for SF contracts and there's no way to use guzzle for it, see #3176655-8: Deprecate GoutteDriver use in core and use Behat\Mink\Driver\BrowserKitDriver directly instead

Refs
- https://www.reddit.com/r/PHP/comments/hub30w/guzzle_vs_symfony_http_client/
- https://symfonycasts.com/screencast/london2019/http-client-what-else

The only missing psr/http-client-implementation in https://github.com/symfony/http-client/blob/5.x/composer.json#L17
Comparing to https://github.com/php-http/guzzle6-adapter/blob/v2.0.1/composer.json#L28

alexpott: fwiw once there is a stable release of all the code that is in https://github.com/guzzle/psr7 main branch (ie. https://github.com/guzzle/psr7/issues/327) then we can swap out core’s diactoros based implementation for guzzle’s and then we can deprecate diactoros without adding any new dependencies.

I have made a PR for guzzle/psr7 so it can hopefully be released.

We're looking at this issue again for D10 because we would like to be able to deprecate Diactoros as a dependency.

Reposting from a Slack discussion. Per http://docs.php-http.org/en/latest/clients/guzzle7-adapter.html :

Guzzle 7 supports PSR-18 out of the box. This adapter makes sense if you want to use HTTPlug async interface or to use Guzzle 7 with a library that did not upgrade to PSR-18 yet and depends on php-http/client-implementation.

In #3104353: Upgrade to Guzzle 7 we're close to making core's existing use of Guzzle compatible with Guzzle 7. So for me, the question is: Do we still need this issue, or can we provide a continuous upgrade path from Diactoros in D9 to Guzzle in D10 without the adapter?

Since the PSRs are at least theoretically supposed to be the public APIs, we could theoretically build a branch for the deprecation of Diactoros on top of Guzzle 7 and see if that gets us to what we would need for removing Diactoros in D10.

We would not be able to backport that to D9, but realistically speaking it might only be a six-month delay anyway, and would also avoid us having to do things twice.

Thoughts?

or can we provide a continuous upgrade path from Diactoros in D9 to Guzzle in D10 without the adapter?

I think that's worth exploring

OK, looked into this some more.

While Guzzle 7 is fully compatible with PSR-18, it doesn't yet support PSR-17 in a stable release. It's apparently only in the master branch of their psr7 package: https://github.com/guzzle/psr7/issues/327

So theoretically we need Guzzle to tag a new minor or major of guzzlehttp/psr7 that includes that, at least.

Just for completeness sake. Me (and XJM) got the guys at Guzzle to get things going. They released guzzle/psr7 2.0.0-RC-1 on 29-04 so we should be able to include 2.0 soon.

https://github.com/guzzle/psr7/releases/tag/2.0.0

The 2.0 release for guzzle/psr7 is now live :D

It needs work for new patch

Reparenting to #3251854: [META] Requirements for tagging Drupal 10.0.0-alpha1.

+++ b/composer.json
@@ -5,6 +5,9 @@
+        "http-interop/http-factory-guzzle": "^1.0",
+        "psr/http-client": "^1.0",
+        "ricardofiorani/guzzle-psr18-adapter": "^1.0",

There's more packages nowadays https://packagist.org/providers/psr/http-client-implementation

We are using Guzzle 7 which nativley supports Psr18. Is this issue closed (outdated)?

@kim.pepper according to https://docs.php-http.org/en/latest/clients/guzzle7-adapter.html I guess it no longer needed

PS: In related psr-17 been added #3255243: Replace Diactoros' PSR-17 implementation with Guzzle's

Closing. Feel free to re-open if you feel the assumption here is incorrect.

As of #3255243: Replace Diactoros' PSR-17 implementation with Guzzle's Diactoros does not exist in the Drupal 10 codebase. The parent of this issue #3251854: [META] Requirements for tagging Drupal 10.0.0-alpha1 indicates this would be important to remove Diactoros, but it is already removed. I'll request a double check from committers :)

I don't think we need this issue any more, because it's baked into guzzle now.

However, do we need a new issue to use PSR-17 where we can rather than the guzzle client directly?

That would depend on running guzzlehttp/psr7 version 2.x in core. Then guzzle van provide psr-17 if i remember correctly. Don't remember is d10 will dispose of guzzlehttp/psr7 1.x

@bbrala: that is already the case in 10.x as of #3220220: Update guzzlehttp/psr7 to 2.1.0.

Sorry, didnt pay anough attention. :) Thanks @Gábor Hojtsy

@catch that seems like an good idea, seems like quite the change though.