Supporting PSR-7 and PSR-18 for HTTP transport layer

[ezimuel]

We would like to support PSR-7 and PSR-18 for the HTTP transport layer. We was thinking to use HTTPlug in order to use an abstraction with some adapter implementations. As default adapter we can use CURL as we are already using it in the client. The idea is to release this with elasticsearch-php 8.0.

Any feedback? thanks!

[nicolas-grekas]

Since Symfony is a major ecosystem where this package is used, it'd be great to consider Symfony HttpClient also. Symfony HttpClient implements both http-plug and PSR-18, but also symfony/http-client-contracts.

The minimum way to play best with the ecosystem IMHO could be to not require any packages past the "contracts" ones - whichever of the 3 mentioned above is selected.

This way ppl would easily decide for the implementation they want to use - or benefit from the one wired by default in their ecosystem.

[nicolas-grekas]

Side note: symfony/http-client-contracts doesn't bring the complexity of dealing with PSR-7 + it's compatible with async and transport options (eg timeout). It might be a best default for this reason, as it would free users to deal with PSR-17 too (which is another thing required when using PSR-18) and find and wire an implementation, which is just a terrible experience with the PSR things IMHO...

[ezimuel]

@nicolas-grekas thanks for your feedback! We need to consider PSR-17 HTTP Factories in the list as well.
We'll also investigate the possibility to use Symfony HTTPClient. Can you point me to some async example use cases? Right now we are supporting async using future mode. I would like to see how Symfony has implemented this feature. Thanks!

[Nyholm]

I think that PSR-18 is a great idea for all libraries that are using HTTP... unless you need async.. =)

I would recommend HTTPlug of libraries that need async support. HTTPlug is PSR-18 + async. Yes, you need a PSR-17 implementation and maybe a way to provide zero configuration support. Using HTTPlug/PSR18 will maximise inoperability.

I do also believe that Symfony's HTTP client is the best PHP HTTP client. I recommend that it should be used in every application. It does provide adapters for HTTPlug so it is the only client you actually need to install.

I fear however that if a library would require only symfony/http-client-contracts it would be vendor lock-in. Because I believe that interface is quite difficult to implement differently than Symfony's two implementations.

[nicolas-grekas]

Sure, here are some examples:
https://github.com/dbrumann/sfughh-http-client-examples/tree/http_client/src/Command

The doc is also telling about it (tl;dr, responses are "lazy", they're comparable to "futures" in the current design):
https://symfony.com/doc/current/components/http_client.html

Check also:
https://speakerdeck.com/nicolasgrekas/symfony-httpclient-what-else

Basically, this sends requests in //:

$client = new \Symfony\Component\HttpClient\CurlHttpClient();

for ($i = 0; $i < 379; ++$i) {
    $uri = "https://http2.akamai.com/demo/tile-$i.png";
    $responses[] = $client->request('GET', $uri);
}

foreach ($responses as $response) {
    // do stuff
}   

The async interface of HTTPlug is also natively supported, it uses promises:

$client = new \Symfony\Component\HttpClient\HttplugClient();

for ($i = 0; $i < 379; ++$i) {
    $uri = "https://http2.akamai.com/demo/tile-$i.png";
    $responses[] = $client->sendAsyncRequest($client->createRequest('GET', $uri));
}

foreach ($responses as $response) {
    // do stuff
}

Note that HttplugClient implements both HTTPlug's async interface + the relevant parts of PSR-17. That's why we can call ->createRequest() on it.

[Nyholm]

The only reason that you may use Symfony's HTTP client and not HTTPlug is that you think the implementation will be much more simple to read, write and maintain.

Im not super familiar with the details of this library so I cannot say for sure how much easier it will be. Try =)

[Nyholm]

Btw, we will soon mark guzzlehttp/ringphp as abandoned on packagist.

[ezimuel]

@Nyholm yes, I know this is an outdated library. That's why we are moving on. Thanks for you feedback!

[thePanz]

That's great @ezimuel ! I quickly discussed with @dbu about this (he's one of the devs behind the HttPlug library), he is open for hints/reviews/help with this 👍

[ezimuel]

Thanks @thePanz and @dbu, I'll definitely ask for reviews.

[nicolas-grekas]

Reading @Nyholm's comment about a potential vendor-locking (and talking with him on Slack) I realized that there is a similar lock-in with HTTPlug: the Promise interface is non-trivial. Symfony's HttplugClient even has a requirement on guzzlehttp/promises for the implementation, the reason being it's impossible to implement without copy/pasting everything from there :)

[Jean85]

As default adapter we can use CURL as we are already using it in the client.

I would advise you against that choice, since that HTTPlug adapter has many flaws. We encountered those in Sentry and switched to Guzzle as a suggested transport.

[dbu]

@Jean85 what are the flaws you encountered? we know that there are a couple of issues with the curl-client promises but one can use guzzle with the httplug adapter as well. can you be a bit more specific where you had the issues?

does elasticsearch do synchronous calls, or also asynchronous? if you do synchronous, i would recommend to go with psr-18, then users can chose which client they want. httplug is in a way the predecessor to psr-18. for async calls however, there is no PSR, because there is no PSR for promises, and we decided it would be wrong for a HTTP client PSR to define a standard for promises.

[Jean85]

The main issue that we encountered was getsentry/sentry-php#878 caused by php-http/curl-client#55

[dbu]

thanks for the clarification. httplug is an abstraction over client. curl-client is just one implementation of that abstraction that seems useful to some people (afaik mostly those doing synchronous requests). there is a httplug guzzle adapter, and the upside of using the httplug interfaces (or psr-18, if you are only doing synchronous requests - the newest versions of guzzle implement psr-18 natively) would be that elastica is not tied to guzzle (or even a specific major version of guzzle).

[dbu]

oh, sorry @Jean85 i think i misunderstood what you are saying! so you advise against recommending the php-http/curl-client, but do not adivce against httplug? if that is what you are saying, i even agree with you :-D

[ezimuel]

@Jean85 I was looking into php-http/curl-client#55 and it seems to be related with this https://bugs.php.net/bug.php?id=61141 that is not a bug, it's reported in the curl documentation here:

When libcurl returns -1 in max_fd, it is because libcurl currently does something that isn't possible for your application to monitor with a socket and unfortunately you can then not know exactly when the current action is completed using select(). You then need to wait a while before you proceed and call curl_multi_perform anyway. How long to wait? Unless curl_multi_timeout gives you a lower number, we suggest 100 milliseconds or so, but you may want to test it out in your own particular conditions to find a suitable value

@adoy reported a solution here:

while ($active && $mrc == CURLM_OK) {
    if (curl_multi_select($mh) == -1) usleep(100);
    do { $mrc = curl_multi_exec($mh, $active); }
    while ($mrc == CURLM_CALL_MULTI_PERFORM);
}

A fix for php-http/curl-client has been proposed by @mekras here. I think we just need to fix it properly.

@Jean85, @dbu do you have other complains against using CURL?

[dbu]

my general feeling is that guzzle is the more mature and maintained client. the php-http/curl-client does not have very active maintainers currently. the fix you mention has been open for almost a year, with nobody picking up on the feedback and wrapping it up.

the other thing are the problems around promises: php-http/curl-client#59 and php-http/curl-client#60

that said, when i raised the question if we want to keep the client around, people said yes they want it. however, nobody stepped up to be an active maintainer of it.

[Jean85]

@dbu properly summarized my concerns 👍

[shadowhand]

A few thoughts:

1. Is async support really necessary? If it isn't, then why not wait until a PSR arrives to address it?
2. I would much rather see dependencies on PSRs than a requirement for HTTPlug or anything from the Symfony ecosystem. I don't use Symfony, but I do use PSR 7/15/17 already.
3. There are PSR-17 discovery packages already, so discovery is not a problem that has been uniquely solved by HTTPlug.

[dbu]

A few thoughts:

1. I guess you meant "If we need async calls, lets wait for a PSR for that"? If that is the case, i would recommend against that, afaik there is nothing even in the pipeline for it. Even if people would start that now, it won't be there before 2021 at the earliest.
2. HTTPlug is not a Symfony component. it depends on the psr and on the php-http/promise package. There is however a Symfony integration for it. But I agree that if we don't need async, depending only on the PSR would be preferrable.
3. Same as 2. Also, i am no fan of discovery, it can lead to unexpected behaviour.

So in summary: If this library does async calls, i would recommend to use HTTPlug. If there are only synchronous calls, i'd recommend to use the series of PSRs for HTTP.

[sebastiaanluca]

For those like me finding this issue when running into the bug below and are (maybe) using Laravel 9 or Symfony 6 components, you can manually set the HTTP client so any custom options work. Otherwise it will find an existing client class that might not work (as in this case).

Exception:

The HTTP client Symfony\Component\HttpClient\Psr18Client is not supported for custom options

Fix:

$clientBuilder->setHttpClient(new \GuzzleHttp\Client);