HttpClient decoration with nested AsyncResponse instances problem #49246

ghost · 2023-02-05T13:32:29Z

ghost
Feb 5, 2023

Hello,
In my SF 5.4 app I have a scoped clients in http_client.yaml

See http_client.yaml

framework:
    http_client:
        scoped_clients:
            first.client:
                base_uri: 'http://api1.com'
            second.client:
                base_uri: 'http://api2.com'

Then I have a decorator that adds a dynamic header

See decorator class

class WithMyHeaderClientDecorator implements HttpClientInterface {
    use AsyncDecoratorTrait;

    public function __construct(HttpClientInterface $client) {
        $this->client = $client->withOptions([
            'headers' => [
                'my-header' => 'my header value'
            ]
        ]);
    }

    public function request(string $method, string $url, array $options = []): ResponseInterface
    {
        return new AsyncResponse($this->client, $method, $url, $options);
    }
}

I wrote a compiler pass to decorate my http clients

See MyCompilerPass.php

    public function process(ContainerBuilder $container): void
    {
        $definitions = $container->findTaggedServiceIds('http_client.client');
        foreach ($definitions as $id => $tags) {
            $container->register($id.'.with_my_header', WithMyHeaderClient::class)
                ->setDecoratedService($id);
        }
    }

The problem is that now I get an empty content when calling my clients, e.g. $firstClient->request('GET', '/users')->getContent(); // returns "". I did some debugging and the reason is that WithMyHeaderClientDecorator decorates the base client (http_client service) and all the scoped clients (that are also a decorators for the base client). This leads to the nested AsyncResponse returned from request() method. The getContent() call on the nested instance returns an expected content but the same call on the outer instance returns empty string.
Is it a bug in Symfony or my misunderstanding how/when to use AsyncResponse class (or/and AsyncDecoratorTrait?).
In my specific case I could just exclude http_client id from decoration in compiler pass and it will work but it's just a coincidence, because my case is simple. I could also (and probably should) just call the inner client's request() method and forward the returned value from my decorator. But let's consider the problem with nested AsyncResponse instances. Let's remember that a client can be decorated multiple times (by me or by the vendor code) so I never know whether a client I decorate (directly or indirectly as another decorator in the decoration stack) with WithMyHeaderClientDecorator will return AsyncResponse instance from request(): ResponseInterface method (decoration considers only interface contract).

Reproducer

See reproducer

<?php

use Symfony\Component\HttpClient\AsyncDecoratorTrait;
use Symfony\Component\HttpClient\DecoratorTrait;
use Symfony\Component\HttpClient\HttpClient;
use Symfony\Component\HttpClient\Response\AsyncResponse;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;

require __DIR__.'/vendor/autoload.php';

class WithMyHeaderClientDecorator implements HttpClientInterface {
    use AsyncDecoratorTrait;

    public function __construct(HttpClientInterface $client) {
        $this->client = $client->withOptions([
            'headers' => [
                'my-header' => 'my header value'
            ]
        ]);
    }

    public function request(string $method, string $url, array $options = []): ResponseInterface
    {
        return new AsyncResponse($this->client, $method, $url, $options);
    }
}

class MyClient implements HttpClientInterface 
{
  use DecoratorTrait;  

  public function __construct($client)
  {
    $this->client = $client;
  }
}

// works
$client = HttpClient::create();
// works
// $client = new WithMyHeaderClientDecorator($client);
// does not work, $content is an empty string ""
// $client = new WithMyHeaderClientDecorator(new WithMyHeaderClientDecorator($client));
// also does not work
$client = new WithMyHeaderClientDecorator(new MyClient(new WithMyHeaderClientDecorator($client)));


$response = $client->request('GET', 'https://jsonplaceholder.typicode.com/users');
$content = $response->getContent();
print_r($content);

MatTheCat · 2023-02-06T08:53:26Z

MatTheCat
Feb 6, 2023

Hello,

as your decorator does not care about the response you should return the decorated service’s:

public function request(string $method, string $url, array $options = []): ResponseInterface
{
    return $this->client->request($method, $url, $options);
}

(You do not need to use AsyncDecoratorTrait.)

5 replies

ghost Feb 6, 2023

I know that and I mentioned that

I could also (and probably should) just call the inner client's request() method and forward the returned value from my decorator

my discussion is about nested AsyncResponse problem.

MatTheCat Feb 6, 2023

The documentation provides a minimal example returning a new AsyncResponse:

class MyExtendedHttpClient implements HttpClientInterface
{
    use AsyncDecoratorTrait;

    public function request(string $method, string $url, array $options = []): ResponseInterface
    {
        // process and/or change the $method, $url and/or $options as needed

        $passthru = function (ChunkInterface $chunk, AsyncContext $context) {
            // do what you want with chunks, e.g. split them
            // in smaller chunks, group them, skip some, etc.

            yield $chunk;
        };

        return new AsyncResponse($this->client, $method, $url, $options, $passthru);
    }
}

Note the $passthru argument passed to AsyncResponse.

carcabot Feb 18, 2023

@MatTheCat how can be this used to change options dynamically based on shouldRetry function? Since options is not passed on it and everything is done inside request()

For example , I want to change the proxy value when shouldRetry is true.

Thanks for your time.

MatTheCat Feb 19, 2023

Look at RetryableHttpClient’s implementation: AsyncContext exposes a replaceRequest method; this is probably what you need.

carcabot Feb 23, 2023

@MatTheCat I have tried to extend RetryableHttpClient but without success because the retry strategy is marked as private inside.

What I want to do is to change for example: user-agent in headers when 429 ocurrs.

Here's an example for GuzzleHttp - https://github.com/caseyamcl/guzzle_retry_middleware

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HttpClient decoration with nested AsyncResponse instances problem #49246

{{title}}

{{editor}}'s edit

{{editor}}'s edit

Replies: 1 comment 5 replies

{{title}}

{{title}}

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{title}}

Select a reply

HttpClient decoration with nested AsyncResponse instances problem #49246

ghost Feb 5, 2023

Replies: 1 comment · 5 replies

MatTheCat Feb 6, 2023

ghost Feb 6, 2023

MatTheCat Feb 6, 2023

carcabot Feb 18, 2023

MatTheCat Feb 19, 2023

carcabot Feb 23, 2023

ghost
Feb 5, 2023

Replies: 1 comment 5 replies

MatTheCat
Feb 6, 2023