-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
AmpHttpClient missing a new timeout function call #36911
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
Comments
Just to confirm, did you actually run into any issue related to this? If yes, can you please provide a full reproducer? E.g. one using httpbin.org |
I’ll work on a simple reproducer, it’s difficult to provide on the api I was using because of oauth requirements. Gist of the problem was making 1000 request calls in an array and getting rate limited and needing to sleep for about 30 seconds 800 requests in. Next code call is a new for loop with a getContents() call. Because of the sleep, by the time it got to the calls right before the sleep, more then ten seconds passed, and the new Amp inactivity timeout kicked in. Reverting from 4.3 to 4.2.2 that didn’t have that new baked in inactivity timeout of ten seconds resolved the problem. My point in this issue is that I think that Amp’s inactivity timeout is overriding the Symfony one, and because I can’t manually touch the actual Amp client, I can’t manually call their new method or pass an option through to touch it. And don’t get me wrong, I may be using a bad pattern here too. |
Thanks for the useful details. FYI, Symfony 5.2 will provide support for pausing without calling sleep, see #37136 About inactivity timeouts, the issue is that there is now a default value set to 10s. Would you mind sending a PR (branch 5.1) to disable this timeout by setting it to |
…h it on our own already (nicolas-grekas) This PR was merged into the 5.1 branch. Discussion ---------- [HttpClient] disable AMP's inactivity timeout, we deal with it on our own already | Q | A | ------------- | --- | Branch? | 5.1 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #36911 | License | MIT | Doc PR | - I encountered this today also. Commits ------- a7b18ff [HttpClient] disable AMP's inactivity timeout, we deal with it on our own already
Symfony version(s) affected: 5.1.0-RC1
Description
AmpHttpClient's request method sets these timeout values:
However, the amphp/http-client module introduced a fourth timeout in version 4.3.0 released on May 3rd, an inactivity timeout, as seen here: amphp/http-client#263
How to reproduce
Make a request and sleep for 11 seconds before calling getContent().
Possible Solution
Adding
$request->setInactivityTimeout(1000 * $options['max_duration']);
works, but I'm not sure that is the appropriate $options value to use (timeout would make sense, as would a non-existent new option).Another temporary option could be to restrict the package to version 4.2.2 when Symfony 5.1 is released?
The text was updated successfully, but these errors were encountered: