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

Default 2 seconds connection timeout value too low? #55

Closed
enekochan opened this issue Nov 11, 2017 · 3 comments
Closed

Default 2 seconds connection timeout value too low? #55

enekochan opened this issue Nov 11, 2017 · 3 comments
Labels
Milestone

Comments

@enekochan
Copy link

enekochan commented Nov 11, 2017

I've been having problems sending SMS messages because of connection timeouts. It doesn't happen every time, it's just "random" (my guess just because of the network connection quality "randomness"). Looks like I've been able to fix this using a custom HttpClient with a higher connect timeout value (10 seconds):

use MessageBird\Client;
use MessageBird\Common\HttpClient;

class MessageBird
{
    /** @var \MessageBird\Client $messageBirdClient */
    protected $messageBirdClient;

    public function __construct($key)
    {
        $httpClient = new HttpClient(Client::ENDPOINT, 10, 10); // Change default connection timeout from 2 seconds to 10
        $this->messageBirdClient = new Client($key, $httpClient);
        ...
    }
}

Although this is fine for me, it would make Chat and Voice endpoints not work as addressed in #29 because the endpoints are different and setting a custom HttpClient makes the same client be used in the 3 endpoints, making this solution not perfect for everyone.

The default value for CURLOPT_CONNECTTIMEOUT seems to be 300 seconds, much higher than our 2 seconds. Other language rest API libraries for MessageBird (I've briefly looked at Java, C# and Python libraries) use their HTTP client default values which most of the times AFAIK are set to "infinite".

The last PR that set this value was #28

Having all this in count, may be the default value should be raised. Not just me, I suspect more people may be having the same issue. In that case those 2 lines should be changed and I could provide a PR:

https://github.com/messagebird/php-rest-api/blob/master/src/MessageBird/Common/HttpClient.php#L59

https://github.com/messagebird/php-rest-api/blob/master/src/MessageBird/Client.php#L147

@enekochan
Copy link
Author

This may be related to #39 too

@Mark-H
Copy link

Mark-H commented Sep 21, 2020

This just hit me too, even though it seems the default timeout has changed since this issue was created.

It would be nice if there was an easier way to configure timeouts, perhaps as an extra option added into the $config array of the Client constructor, both for the connection and regular timeout.

@CoolGoose CoolGoose added this to the v2.0 milestone Apr 4, 2021
@CoolGoose CoolGoose modified the milestones: v3.0, vNext Dec 23, 2021
@dennisvdvliet
Copy link
Contributor

Closing this due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants