Ship the SDK with its own HTTP client instead of relying on HTTPlug

[cleptric]

Problem Statement

We're currently using Httplug to allow people to use their HTTP client of choice with the SDK.
This creates confusion about why we offer two composer packages sentry/sentry vs sentry/sdk.
I also have the feeling that this is rarely used and slightly over-optimized.

Solution Brainstorm

Two good examples of other SDKs/libraries shipping with an HTTP client are the AWS PHP SDK (they use Guzzle) and Stripe (they implemented their own client).

While I do not think that Guzzle is necessary in our case, what other HTTP clients we could look into using in our SDK?

[stayallive]

I would vote to go for a very simple interface like the one Stripe uses.

https://github.com/stripe/stripe-php/tree/master/lib/HttpClient

They use a very simple interface so the user can implement their own and ship with a cURL client by default. The cURL extension is by and large a default and available almost always and has no composer dependencies.

If we would go that route it's trivial to implement a Guzzle or whatever implementation that can be given to the SDK to do it's HTTP work without needing the full plumbing Htttplug provides which we workaround quite a lot already for timeouts and proxy configurations for example. Frameworks like Laravel and Symfony could also by default replace the client with the default shipped by their framework with a simple client implementation.

[ste93cry]

I would love to see Httplug go as it is a relic of the past when a PSR standard wasn't available for HTTP clients. However, I don't see the point in implementing our client interfaces: the code is already decoupled from any HTTP client, and you can already use whatever lib you want. Moreover, if we want to to provide an HTTP client out the box, we should just choose one (I would recommend symfony/http-client given that we already use other components from that framework) and certainly not reimplement one (it was like that in the past, and it didn't work well)

[cleptric]

What is Symfony's statement on how long they will support v5.0 of their HTTP client library?
Their current v6.0 major already requires PHP 8.1 and up.

[Jean85]

Symfony has its how LTS schedule, so that's not a problem: https://symfony.com/releases#symfony-releases-calendar

IMHO we should still rely on PSR-18 as a decoupling interface to allow everyone to either use the provided client or swap-it-out with another custom one. This should give us the best of both worlds, because we can rely on any common HTTP client in PHP (even the Symfony one) to implement that one.

[cleptric]

Using/building something PRS 18 compatible is definitely a good idea.
I just want to iterate that we should ship some HTTP client by default, and let people use something else if they really want to, or, use the one that ships with Symfony/Laravel for our other SDKs.

@stayallive Is the Laravel HTTP client PSR 18 compatible?

Maybe @richardm-stripe or @dcr-stripe are open about telling us about the reasoning behind Stripe opting into implementing their own HTTP client in their SDK 🙏

[richardm-stripe]

I wasn't a maintainer back when stripe-php first implemented an HTTP client 10+ years ago, but I think @stayallive has the right of it. Wrapping curl is about keeping dependencies minimal. If we were designing the library again today, we'd probably still wrap curl but make the interface more compatible with community standards e.g. PSR-7-compatible responses.

As a library maintainer there are tradeoffs to wrapping curl directly and working at such a "low level", e.g. there have been a few user issues over the years relating to strange behavior on particular versions of curl -- I'm thinking of this issue where we had to work around a bug in how HTTP/2 worked on old versions of curl. Use a third-party like Guzzle, and to some degree you can rely on the maintainers of that project to detect and implement fixes for things like that and save yourself some trouble. On the other hand, you also have less control, and would be less able to fix low-level issues yourself for your users, if you needed to.

[cleptric]

Thanks a lot for sharing some insights 😃

[Jean85]

Another input on this discussion:

• https://twitter.com/nicolasgrekas/status/1583454980769730560
• https://github.com/FriendsOfPHP/well-known-implementations

[nicolas-grekas]

@cleptric are you coming to SymfonyCon next month? Sentry has a booth so I was wondering. It'd be a nice opportunity to brainstorm about what @Jean85 just linked to ;)

[cleptric]

Yes, I'll be at the Sentry booth during SymfonyCon. Let's chat!

[nicolas-grekas]

See proposal at #1418

[Jean85]

@nicolas-grekas questions: how does it work on first install? The plugin would run during the post-install? Even if it has to be explicitly enabled now, with newer Composer versions? What happens if no compatible implementation is available?

[nicolas-grekas]

Here is a PR to move this concern to php-http/discovery: php-http/discovery#208

The plugin runs on post-update yes. Of course if it's disabled then ppl take the responsibility to install the missing implementations on their own. The plugin is compatible with both composer v1 and v2 and with PHP >= 7.1.