eversign / eversign-php-sdk

The official eversign PHP SDK
https://eversign.com
MIT License
17 stars 24 forks source link

Set (or allow setting of) a timeout for Guzzle calls #48

Closed jasongill closed 3 years ago

jasongill commented 3 years ago

Is your feature request related to a problem? Please describe. When Eversign had an API outage recently, the areas of our site that interact with Eversign's API in realtime ground to a halt. This is because the Guzzle calls in Eversign\ApiRequest do not have a timeout set, and the default timeout for Guzzle is unlimited. Basically, when the Eversign API didn't respond, Guzzle never gave up and threw an exception that we could handle, so pages just loaded indefinitely (or until max_execution_time just made them error out completely). Although it is clearly a rare situation, it caused a lot of user confusion as no useful error messages could be displayed.

Describe the solution you'd like Please either set a sensible timeout (15 seconds?) and connect timeout on the Guzzle calls to Eversign in Eversign\ApiRequest, or add the ability for us to set a timeout ourselves. A good example of this is how Intercom PHP handles this - when instantiating the IntercomClient, it's possible to add an array of options that are set on all Guzzle calls. You can see their solution in this pull request if you need ideas: https://github.com/intercom/intercom-php/pull/232 (but note that its a few years old and the concept may need to be adapted to more recent Guzzle versions)

Describe alternatives you've considered Beyond trying to do something sloppy like creating our own private branch of this package, or trying to forcibly overload the classes to add a timeout, there doesn't appear to be any other solution - Guzzle doesn't appear to have the ability to read any "global defaults" or otherwise allow the timeout to be set outside of the time that a new Guzzle client is created

Additional context Outage on or around April 26th caused the excessive request times without any error that could be caught as an exception: https://help.eversign.com/hc/en-us/articles/1500002838342--Service-Notifications-2021

archivelm commented 3 years ago

Hi @jasongill, thanks for reporting this,

I wrote quick MR to set timeout for 15 seconds to existing Guzzle calls. If you can check MR here https://github.com/eversign/eversign-php-sdk/pull/49/files I will release new version once approved.

At this point, I would not make it configurable if no specific need arises from community. I took a quick look, if needed at later point, we can put it in config, then propagate to Client and from Client to ApiRequest objects.

jasongill commented 3 years ago

This looks perfect. I would think that 15 seconds is sufficient and should solve the issue of the default unlimited timeout. Thanks for doing this

archivelm commented 3 years ago

Yes, I agree, merging to master. :) Latest release is here https://github.com/eversign/eversign-php-sdk/releases/tag/1.23.1 Thanks for reporting and stay safe!

jasongill commented 3 years ago

Hey @milanlatinovic , there is only one problem here that I just found when I pushed this to production - the type hinting on properties is only supported on PHP 7.4+, so now the package doesn't work at all on PHP 7.1-7.3

Can you remove the GuzzleClient type hint in ApiRequest class so that the package works again on older versions of PHP (which obviously would be nice to upgrade to 7.4 but that's a bigger project as you can imagine)

(Update: I created #50 which fixes this issue and makes the package work on PHP 7.1-7.3 again)

archivelm commented 3 years ago

Hey @jasongill, good point. My local dev is PHP 7.4 so this one went under the radar. Thanks, I approved your MR with removed GuzzleClient type, merged and made a new release. https://github.com/eversign/eversign-php-sdk/releases/tag/1.23.2

All best!

jasongill commented 3 years ago

@milanlatinovic thank you! seems to be OK now, and we are relieved to have the timeout set. Thanks for the hard work

marianperca commented 2 years ago

Hi, we are trying to create a document using

$file->setFileBase64(base64_encode($contents));

Our document is a 26 page word document and the base64 string is ~ 70KB.
Is it normal to take this long for the document to be created? If yes, can you please let us know how we can create this document?

local.ERROR: cURL error 28: Operation timed out after 15000 milliseconds with 0 bytes received (see https://curl.haxx.se/libcurl/c/libcurl-errors.html) for https://api.eversign.com/api... {"userId":2,"exception":"[object] (GuzzleHttp\\Exception\\ConnectException(code: 0): cURL error 28: Operation timed out after 15000 milliseconds with 0 bytes received (see https://curl.haxx.se/libcurl/c/libcurl-errors.html) for https://api.eversign.com/api/document.... at /var/www/html/owlcentre/vendor/guzzlehttp/guzzle/src/Handler/CurlFactory.php:210)
[stacktrace]
#0 /var/www/html/owlcentre/vendor/guzzlehttp/guzzle/src/Handler/CurlFactory.php(158): GuzzleHttp\\Handler\\CurlFactory::createRejection()
#1 /var/www/html/owlcentre/vendor/guzzlehttp/guzzle/src/Handler/CurlFactory.php(110): GuzzleHttp\\Handler\\CurlFactory::finishError()
#2 /var/www/html/owlcentre/vendor/guzzlehttp/guzzle/src/Handler/CurlHandler.php(47): GuzzleHttp\\Handler\\CurlFactory::finish()
#3 /var/www/html/owlcentre/vendor/guzzlehttp/guzzle/src/Handler/Proxy.php(28): GuzzleHttp\\Handler\\CurlHandler->__invoke()
#4 /var/www/html/owlcentre/vendor/guzzlehttp/guzzle/src/Handler/Proxy.php(48): GuzzleHttp\\Handler\\Proxy::GuzzleHttp\\Handler\\{closure}()
#5 /var/www/html/owlcentre/vendor/guzzlehttp/guzzle/src/PrepareBodyMiddleware.php(64): GuzzleHttp\\Handler\\Proxy::GuzzleHttp\\Handler\\{closure}()
#6 /var/www/html/owlcentre/vendor/guzzlehttp/guzzle/src/Middleware.php(31): GuzzleHttp\\PrepareBodyMiddleware->__invoke()
#7 /var/www/html/owlcentre/vendor/guzzlehttp/guzzle/src/RedirectMiddleware.php(71): GuzzleHttp\\Middleware::GuzzleHttp\\{closure}()
#8 /var/www/html/owlcentre/vendor/guzzlehttp/guzzle/src/Middleware.php(63): GuzzleHttp\\RedirectMiddleware->__invoke()
#9 /var/www/html/owlcentre/vendor/guzzlehttp/guzzle/src/HandlerStack.php(75): GuzzleHttp\\Middleware::GuzzleHttp\\{closure}()
#10 /var/www/html/owlcentre/vendor/guzzlehttp/guzzle/src/Client.php(331): GuzzleHttp\\HandlerStack->__invoke()
#11 /var/www/html/owlcentre/vendor/guzzlehttp/guzzle/src/Client.php(168): GuzzleHttp\\Client->transfer()
#12 /var/www/html/owlcentre/vendor/guzzlehttp/guzzle/src/Client.php(187): GuzzleHttp\\Client->requestAsync()
#13 /var/www/html/owlcentre/vendor/eversign/eversign-php-sdk/sdk/Eversign/ApiRequest.php(207): GuzzleHttp\\Client->request()
#14 /var/www/html/owlcentre/vendor/eversign/eversign-php-sdk/sdk/Eversign/Client.php(392): Eversign\\ApiRequest->startRequest()
...
archivelm commented 2 years ago

Hi @marianperca,

This thread's original issue was to introduce timeout for Guzzle calls. Issue that you are experiencing is indeed throwing a timeout exception but it is not related to this thread.

You should create a new Issue and describe your problem in more details. For example you could provide a bigger code snippet so I can see structure of whole $file object that you are preparing. Also, you should indicate a timestamp so we know when this issue occurred for the first time. Finally, it makes sense if you double-check if you had any environemnt issues that might cause your api request to fail.

In general, I saw much larger documents with 500+ pages that are processed without any problems. So, it might be: a) temporary network issue (you can retry to be sure); b) something in the way you use SDK (you need to provide more code; or c) possible bug or edge case in the SDK that was not reported by the community so far.

In any case, please make independent Issue with more details and we can continue there. Thanks!

All best, Milan

marianperca commented 2 years ago

Hi @milanlatinovic

Thanks for your swift reply. We also tried to make an API call without the SDK and it takes 17 seconds. I guess we're either doing something wrong, or the API is slow. We tried to send the file with both base64 and with a URL - same result. If the API request is expected to take this long, then I think the SDK should allow for a configurable timeout - which is somehow related to this issue. We are using Eversign on a different project (same software stack, same SDK) and there the API responds faster.

Thank you for your help! Marian

test request
archivelm commented 2 years ago

Hi @marianperca,

Commonly API would not take that long but I see what you mean. I changed Guzzle config across the SDK and increased timeout from 15 to 30 seconds. This will solve your issue at hand.

Hopefully this solves your issue. If the issue reappears, please open it in the new issue and you can mark it as a related issue to this one.

Kind regards, Milan

marianperca commented 2 years ago

Thank you very much @milanlatinovic It works fine now.