Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HttpClient] add AsyncDecoratorTrait to ease processing responses without breaking async #36779

Merged
merged 1 commit into from Jun 9, 2020

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented May 10, 2020

Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #31885, fix #32367
License MIT
Doc PR symfony/symfony-docs#13736

This PR allows processing the stream of chunks.

<?php
$client = new class() implements HttpClientInterface {
    use AsyncDecoratorTrait;

    public function request(string $method, string $url, array $options): ResponseInterface
    {
        return new AsyncResponse($this->client, $method, $url, $options, static 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;
        });
    }
};

Some ideas:

Any custom logic should fit into the $passthru filter iterator (the last constructor argument of AsyncResponse). There, one has access to an AsyncContext DTO, which allows controlling the stream, eg. to replace the current request/response, to change the passthru filter itself, etc.

The surrounding logic will catch badly behaving filters to ease spotting some mistakes (eg. never forwarding an "isLast()" chunk, or yielding extra chunks after an "isLast()" one, etc.)

For the record:

  • When the chunk passthru issues many internal requests in order to complete the external one, the info of each internal request is accessible via the previous_info entry. I considered merging all internal response_headers info under the main one since that's possible, but I'm not sure it's worth the added complexity. Please tell me if you think we should do it.
  • A future iteration/PR might add support for time-based events. Right now, implementing a pause in the stream involves calling usleep(), but this doesn't play really well with async. Implementing small pauses and summing them up to the target pause might be good enough - we'll need to give it a try to know better.

@nicolas-grekas nicolas-grekas changed the title [HttpClient] add PluggableHttpClient to allow processing the response stream [HttpClient] add AbstractStreamingHttpClient to process responses asynchronously May 14, 2020
@nicolas-grekas nicolas-grekas changed the title [HttpClient] add AbstractStreamingHttpClient to process responses asynchronously [HttpClient] add AbstractAsyncHttpClient to ease processing responses without breaking async May 14, 2020
@nicolas-grekas nicolas-grekas force-pushed the hc-pluggable branch 4 times, most recently from 5b2b058 to 6438099 Compare May 17, 2020 22:30
@nicolas-grekas nicolas-grekas changed the title [HttpClient] add AbstractAsyncHttpClient to ease processing responses without breaking async [HttpClient] add AsyncDecoratorTrait to ease processing responses without breaking async May 18, 2020
@nicolas-grekas nicolas-grekas force-pushed the hc-pluggable branch 2 times, most recently from 33e2614 to f6e0252 Compare May 18, 2020 13:02
@nicolas-grekas
Copy link
Member Author

The entry-point is now AsyncDecoratorTrait, which is a simple trait to wire AsyncResponse, where all the logic is.

I implemented test cases that illustrate some use cases:

  • retry after 404
  • retry after a transport error
  • preflight request
  • live transclusion of content, doing concurrent requests inside the main request.

This is what made me change the API proposed here.

PR ready.

@nicolas-grekas nicolas-grekas force-pushed the hc-pluggable branch 2 times, most recently from 7551c8a to 5b70e5f Compare May 18, 2020 16:17
@derrabus
Copy link
Member

Could this trait be used to solve #36967? 🤔

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented May 28, 2020

@derrabus absolutely: it would make it way easier to persist to the cache without breaking async. (bold claim, to be verified of course :) )

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented May 30, 2020

I'm quite happy with this!

BTW, doc PR at symfony/symfony-docs#13736

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Jun 7, 2020

@symfony/mergers I'd appreciate if we could merge this PR soon as that would unlock some next steps (I'm preparing a PR to allow pausing a response, needed for #36692 and for implementing support for HTTP 429 or throttling)

*/
public function pause(float $duration): void
{
if (\is_callable($pause = $this->response->getInfo('pause_handler'))) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this anticipates #37136 to support pausing requests/responses

@fabpot
Copy link
Member

fabpot commented Jun 9, 2020

Thank you @nicolas-grekas.

@fabpot fabpot merged commit d75ef18 into symfony:master Jun 9, 2020
@nicolas-grekas nicolas-grekas deleted the hc-pluggable branch June 11, 2020 13:32
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.2 Oct 5, 2020
@fabpot fabpot mentioned this pull request Oct 5, 2020
javiereguiluz added a commit to javiereguiluz/symfony-docs that referenced this pull request Apr 20, 2021
…oratorTrait (nicolas-grekas)

This PR was merged into the 5.2 branch.

Discussion
----------

[HttpClient] add doc about extending and AsyncDecoratorTrait

About symfony/symfony#36779 and symfony/symfony#37136

Commits
-------

f41ef12 [HttpClient] add doc about extending and AsyncDecoratorTrait
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[HttpClient] Ability to add middlewares [HttpClient] provide a way to alter the stream of chunks
9 participants