Allow replacing the guzzle handler

[bwoebi]

This allows setting e.g. Amp\Http\Client\GuzzleAdapter\GuzzleHandlerAdapter, allowing the whole firebase-php environment to transparently run asynchronously.

Up to now this required very ugly hacking manipulating private fields directly, like:

(fn() => (fn() => (fn() => (fn() => $this->handler = new GuzzleHandlerAdapter)->call($this->config["handler"]))->call($this->client))->call($this->messagingApi))->call($messaging);

Which is not exactly ergonomic.

[jeromegamez]

Ugh, yes, this is indeed ugly 😅, thank you for bringing this up, we definitely need to change this.

The HttpClientOptions have been a thorn in my side for quite some time - my plan, in the beginning, was to have a Guzzle-independent configuration class that wouldn't need to be changed if I, one day, decided to migrate from Guzzle to e.g. HTTPlug - but with google/cloud-* and google/auth using Guzzle already, this goal was certainly too idealistic.

I'm hesitant to add another method/property to the class because I'm planning to deprecate all methods in favor of the already existing withGuzzleClientOption(), withGuzzleClientOptions(), withGuzzleMiddleware() and withGuzzleMiddlewares() methods. Those are already used within the class, and only the Guzzle Config is exposed to the factory.

So, instead of using

$options = HttpClientOptions::default()
    ->withTimeOut(3.5)
    ->withProxy('tcp://<host>:<port>');

one would use

use GuzzleHttp\RequestOptions;

$options = HttpClientOptions::default()
    ->withGuzzleClientOptions([
        RequestOptions::TIMEOUT => 3.5,
        RequestOptions::PROXY => 'tcp://<host>:<port>',
    ]);

This would make it easier to provide any config for the Guzzle Client, and not those that are exposed through the with* methods, including the handler as a special option.

This would then make it easy to add a key for then handler :

use GuzzleHttp\RequestOptions;

$options = HttpClientOptions::default()
    ->withGuzzleClientOptions([
        RequestOptions::TIMEOUT => 3.5,
        RequestOptions::PROXY => 'tcp://<host>:<port>',
    ])
    ->withGuzzleClientOption('handler', ...) // or directly in the array above
;

With this already being possible, the only remaining change would be a a check in the factory like

$handler = HandlerStack::create($config['handler'] ?? null);

What do you think?

[bwoebi]

The problem with that is the obviousness of discoverability. Like, how do I know there's an arbitrary additional option named "handler" without actually looking into the source code?
A ->withSomething() method is trivially discovered via autocompletion.
That's also why I prefer ->withTimeout/->withProxy, as it lets me get away with having to actually look at guzzle and figure out how to configure that, at least for trivial options.

I mean, what you propose definitely solves the problem, but I have some doubts on the superiority of that approach.

[jeromegamez]

I get it! Let's keep it the way it is then for the moment, and I'll think about it more when time comes - but I would hate having to add methods for every single!

To be fair - if someone is looking to replace the default handler with a custom one, they probably know Guzzle well enough 😅)

Going into the PR - I still wouldn't add a new handler property but add it to the Guzzle Config array that's used internally, like with the other methods:

# HttpClientOptions

/**
 * @param callable(RequestInterface, array): PromiseInterface $value
 */
public function withGuzzleHandler(callable $handler): self
{
    return $this->withGuzzleConfigOption('handler', $value);
}

and

# Factoy

/**
 * @param array<non-empty-string, mixed>|null $config
 * @param array<callable(callable): callable>|null $middlewares
 */
public function createApiClient(?array $config = null, ?array $middlewares = null): Client
{
    // ...

    $handler = HandlerStack::create($config['handler'] ?? null);

    // ...
}

Meaning: giving users the benefit of discoverability through auto-completion but using only the "source of truth" being the Guzzle Config array

[bwoebi]

To be honest, I have barely any idea of guzzle, but I just know that there's an adapter I can just use to make any usage of guzzle async and have my code just work :-)

Assuming adding something to the guzzle config is fine, as long as having some custom unused values in there isn't harming guzzle?
But yes, having it internally in the array is sounds fine to me too.

[jeromegamez]

Merged with db50265 - thanks again!