Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpClient][Contracts] introduce component and related contracts#30413
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
gnugat commentedMar 1, 2019
It seems like a shame for A pity Finally it seems odd to define a new Sounds like this component was tailored for Symfony's internal use only (to be used by the Mail component), and is being made available for anyone who would want to use it, but given the choice between this component and Guzzle I think Guzzle would still be the go to solution. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Taluu left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
For the options, why not use theOptionsResolver component ?
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| ],staticfunction ($v) {returnnull !==$v; }), | ||
| 'socket' => [ | ||
| 'bindto' =>$options['bindto'], | ||
| 'tcp_nodelay' =>true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
IMO this option should be set to false, true is very useful on small request, but on medium to large stream (like http) we want to avoid congestion by default.
nicolas-grekasMar 1, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Both fopen and curl already buffer as much as they can before writing to the network. This means the TCP Nagle algorithm is not needed and only degrades performance.
5c49831 to5b07802Comparenicolas-grekas commentedMar 1, 2019
@fancyweb thanks, comments addressed.
@Taluu because that would add a dependency and we prefer keeping the component standalone.
@gnugat not the case at all. The component is standalone and relies on decoupled interfaces. Your choice to use Guzzle. I'm going to replace it personnaly - in "Symfony apps" or "non-Symfony" ones. |
d9622bf to49fa0b4Comparenicolas-grekas commentedMar 6, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@joelwurtz, unfortunately, there is no way to implement the steps you describe: e.g. curl has no concept like that and it would be extremely hard to recreate it. fopen also is missing a lot of steps but I reimplemented the major missing ones in fact. |
Uh oh!
There was an error while loading.Please reload this page.
weaverryan left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
After playing with this for more than a day now, it's a 👍 from me! The experience is wonderful and now that we've solved a few issues (like removing thegetStatusCode() mutation), the "lazy" responses also yield an excellent experience when you just want to make one, sync request.
A few notes:
A) Responses are lazy. It will be important to document/teach that. In practice, it simply means that if you want to wrap your code in a try-catch, you should wrap all of your logic that deals with the response, not just the$client->response() call.
B) Validation on the options is awesome and the options are reasonable easy to find because the defaults are stored on a constant that is documented above everyrequest() method
C) If want to do multiple requests in parallel, that works really well. And by leveraging theuser_data option, you can basically "label" each request so that you can handle the responses appropriately.
D) Streaming a single large file also works well - I streamed a large file directly to a local file without any isuses or memory bump.
E) About decoration -@joelwurtz did an awesome job challenging this. For basic cases like logging, theon_progress option can be leveraged to log the status code from the$info that's passed to that callback. More complex decorators are possible - they may not be super easy, due to the async of the responses, but they're possible. And if you're building a decorator for your app and you don't care about async, decorators are super easy.
So, I think this is ready!
arnaud-lb commentedMar 7, 2019
Since I came here to say what I disliked (and what I liked as well), I also need to say it when those things have changed or if I changed my mind. The component actually does a great job at hiding an asynchronous internal and exposing With the removal of The fact that other requests are running when waiting for a single request (even when not explicitly multiplexing with |
4c6539e tof64f2abComparef64f2ab tofc83120Comparefabpot commentedMar 7, 2019
Thank you@nicolas-grekas. |
…d contracts (nicolas-grekas)This PR was squashed before being merged into the 4.3-dev branch (closes#30413).Discussion----------[HttpClient][Contracts] introduce component and related contracts| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#28628| License | MIT| Doc PR | -This PR introduces new `HttpClient` contracts andcomponent. It makes no compromises between DX, performance, and design.Its surface should be very simple to use, while still flexible enoughto cover most advanced use cases thanks to streaming+laziness.Common existing HTTP clients for PHP rely on PSR-7, which is complexand orthogonal to the way Symfony is designed. More reasons we needthis in core are the [package principles](https://en.wikipedia.org/wiki/Package_principles): if we want to be able to keep ourBC+deprecation promises, we have to build on more stable and moreabstract dependencies than Symfony itself. And we need an HTTP clientfor e.g. Symfony Mailer or#27738.The existing state-of-the-art puts a quite high bar in terms of features we mustsupport if we want any adoption. The code in this PR aims at implementing aneven better HTTP client for PHP than existing ones, with more (useful) featuresand a better architecture. What a pitch :)Two full implementations are provided: - `NativeHttpClient` is based on the native "http" stream wrapper. It's the most portable one but relies on a blocking `fopen()`. - `CurlHttpClient` relies on the curl extension. It supports full concurrency and HTTP/2, including server push.Here are some examples that work with both clients.For simple cases, all the methods on responses are synchronous:```php$client = new NativeHttpClient();$response = $client->get('https://google.com');$statusCode = $response->getStatusCode();$headers = $response->getHeaders();$content = $response->getContent();```By default, clients follow redirects. On `3xx`, `4xx` or `5xx`, the `getHeaders()` and `getContent()` methods throw an exception, unless their `$throw` argument is set to `false`.This is part of the "failsafe" design of the component. Another example of thisfailsafe property is that broken dechunk or gzip streams always trigger an exception,unlike most other HTTP clients who can silently ignore the situations.An array of options allows adjusting the behavior when sending requests.They are documented in `HttpClientInterface`.When several responses are 1) first requested in batch, 2) then accessedvia any of their public methods, requests are done concurrently whilewaiting for one.For more advanced use cases, when streaming is needed:Streaming the request body is possible via the "body" request option.Streaming the response content is done via client's `stream()` method:```php$client = new CurlHttpClient();$response = $client->request('GET', 'http://...');$output = fopen('output.file', 'w');foreach ($client->stream($response) as $chunk) { fwrite($output, $chunk->getContent());}```The `stream()` method also works with multiple responses:```php$client = new CurlHttpClient();$pool = [];for ($i = 0; $i < 379; ++$i) { $uri = "https://http2.akamai.com/demo/tile-$i.png"; $pool[] = $client->get($uri);}$chunks = $client->stream($pool);foreach ($chunks as $response => $chunk) { // $chunk is a ChunkInterface object if ($chunk->isLast()) { $content = $response->getContent(); }}```The `stream()` method accepts a second `$timeout` argument: responses thatare *inactive* for longer than the timeout will emit an empty chunk to signalit. Providing `0` as timeout allows monitoring responses in a non-blocking way.Implemented: - flexible contracts for HTTP clients - `fopen()` + `curl`-based clients with close feature parity - gzip compression enabled when possible - streaming multiple responses concurrently - `base_uri` option for scoped clients - progress callback with detailed info and able to cancel the request - more flexible options for precise behavior control - flexible timeout management allowing e.g. server sent events - public key pinning - auto proxy configuration via env vars - transparent IDN support - `HttpClient::create()` factory - extensive error handling, e.g. on broken dechunk/gzip streams - time stats, primary_ip and other info inspired from `curl_getinfo()` - transparent HTTP/2-push support with authority validation - `Psr18Client` for integration with libs relying on PSR-18 - free from memory leaks by avoiding circular references - fixed handling of redirects when using the `fopen`-based client - DNS cache pre-population with `resolve` optionHelp wanted (can be done after merge): - `FrameworkBundle` integration: autowireable alias + semantic configuration for default options - add `TraceableHttpClient` and integrate with the profiler - logger integration - add a mock clientMore ideas: - record/replay like CsaGuzzleBundle - use raw sockets instead of the HTTP stream wrapper - `cookie_jar` option - HTTP/HSTS cache - using the symfony CLI binary to test ssl-related options, HTTP/2-push, etc. - add "auto" mode to the "buffer" option, based on the content-type? or array of content-types to buffer - *etc.*Commits-------fc83120 [HttpClient] Add Psr18Client - aka a PSR-18 adapter8610668 [HttpClient] introduce the componentd2d63a2 [Contracts] introduce HttpClient contracts
nicolas-grekas commentedMar 8, 2019
Thank you@fabpot and thank you to everyone involved. The discussion has been sometimes passionate, but I'm overall highly impressed by the technical level and quality of the reviews. This component is already the work of the community and not mine anymore. Thank you all! |
This PR was squashed before being merged into the master branch (closes#11071).Discussion----------Documented the new HttpClient componentDocs forsymfony/symfony#30413.Commits-------0390bad Documented the new HttpClient component
Uh oh!
There was an error while loading.Please reload this page.
This PR introduces new
HttpClientcontracts andcomponent. It makes no compromises between DX, performance, and design.
Its surface should be very simple to use, while still flexible enough
to cover most advanced use cases thanks to streaming+laziness.
Common existing HTTP clients for PHP rely on PSR-7, which is complex
and orthogonal to the way Symfony is designed. More reasons we need
this in core are thepackage principles: if we want to be able to keep our
BC+deprecation promises, we have to build on more stable and more
abstract dependencies than Symfony itself. And we need an HTTP client
for e.g. Symfony Mailer or#27738.
The existing state-of-the-art puts a quite high bar in terms of features we must
support if we want any adoption. The code in this PR aims at implementing an
even better HTTP client for PHP than existing ones, with more (useful) features
and a better architecture. What a pitch :)
Two full implementations are provided:
NativeHttpClientis based on the native "http" stream wrapper.It's the most portable one but relies on a blocking
fopen().CurlHttpClientrelies on the curl extension. It supports fullconcurrency and HTTP/2, including server push.
Here are some examples that work with both clients.
For simple cases, all the methods on responses are synchronous:
By default, clients follow redirects. On
3xx,4xxor5xx, thegetHeaders()andgetContent()methods throw an exception, unless their$throwargument is set tofalse.This is part of the "failsafe" design of the component. Another example of this
failsafe property is that broken dechunk or gzip streams always trigger an exception,
unlike most other HTTP clients who can silently ignore the situations.
An array of options allows adjusting the behavior when sending requests.
They are documented in
HttpClientInterface.When several responses are 1) first requested in batch, 2) then accessed
via any of their public methods, requests are done concurrently while
waiting for one.
For more advanced use cases, when streaming is needed:
Streaming the request body is possible via the "body" request option.
Streaming the response content is done via client's
stream()method:The
stream()method also works with multiple responses:The
stream()method accepts a second$timeoutargument: responses thatareinactive for longer than the timeout will emit an empty chunk to signal
it. Providing
0as timeout allows monitoring responses in a non-blocking way.Implemented:
fopen()+curl-based clients with close feature paritybase_urioption for scoped clientsHttpClient::create()factorycurl_getinfo()Psr18Clientfor integration with libs relying on PSR-18fopen-based clientresolveoptionHelp wanted (can be done after merge):
FrameworkBundleintegration: autowireable alias + semantic configuration for default optionsTraceableHttpClientand integrate with the profilerMore ideas:
cookie_jaroption