Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/SubscribePro/Http.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,19 +63,25 @@ class Http
/** @var int */
protected $requestTimeout;

/** @var int|float */
protected $connectTimeout;

/**
* Http constructor.
*
* @param $app
* @param $requestTimeout
* @param string|null $baseUrl
* @param int|float $connectTimeout Guzzle connect timeout in seconds. 0 (default) disables the
* connect timeout, preserving prior behavior.
*/
public function __construct($app, $requestTimeout, $baseUrl = null)
public function __construct($app, $requestTimeout, $baseUrl = null, $connectTimeout = 0)
{
$this->app = $app;
$this->baseUrl = $baseUrl ?: self::API_BASE_URL;
$this->handlerStack = $this->createHandlerStack();
$this->requestTimeout = $requestTimeout;
$this->connectTimeout = $connectTimeout;
}

/**
Expand All @@ -85,7 +91,7 @@ public function __construct($app, $requestTimeout, $baseUrl = null)
*/
protected function createClient()
{
return new Client([
$config = [
'base_uri' => $this->baseUrl,
'handler' => $this->handlerStack,
RequestOptions::HTTP_ERRORS => false,
Expand All @@ -94,7 +100,15 @@ protected function createClient()
RequestOptions::HEADERS => [
'User-Agent' => self::USER_AGENT,
]
]);
];

// Only set a connect timeout when one is configured (> 0); 0 keeps Guzzle's
// default of no connect timeout, preserving prior behavior for existing consumers.
if ($this->connectTimeout > 0) {
$config[RequestOptions::CONNECT_TIMEOUT] = $this->connectTimeout;
}

return new Client($config);
}

/**
Expand Down
15 changes: 14 additions & 1 deletion src/SubscribePro/Sdk.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,20 @@ class Sdk
* default value @see \SubscribePro\Http::DEFAULT_LOG_LINE_FORMAT
* - logging_message_format
* default value @see \SubscribePro\Http::DEFAULT_LOG_MESSAGE_FORMAT
* - api_request_timeout
* Total Guzzle request timeout in seconds. default value 30
* - connect_timeout
* Guzzle connect timeout in seconds, applied as RequestOptions::CONNECT_TIMEOUT.
* default value 0 (disabled — no separate connect timeout, preserving prior behavior)
* - <service_name>
* Config options for specified service
*
* Note on timeouts: a Guzzle request/connect timeout raises a
* \GuzzleHttp\Exception\ConnectException / RequestException, which is NOT
* response-based and is therefore NOT represented by \SubscribePro\Exception\HttpException
* (which requires a ResponseInterface). Consumers that must handle timeouts should
* catch the Guzzle transfer exceptions (or \RuntimeException) in addition to HttpException.
*
* @param array $config
*
* @throws \SubscribePro\Exception\InvalidArgumentException
Expand All @@ -100,6 +111,7 @@ public function __construct(array $config = [])
'logging_line_format' => null,
'logging_message_format' => null,
'api_request_timeout' => 30,
'connect_timeout' => 0,
], $config);

if (!$config['client_id']) {
Expand All @@ -113,8 +125,9 @@ public function __construct(array $config = [])
unset($config['client_id']);
unset($config['client_secret']);

$this->http = new Http($app, $config['api_request_timeout'], $config['base_url']);
$this->http = new Http($app, $config['api_request_timeout'], $config['base_url'], $config['connect_timeout']);
unset($config['base_url']);
unset($config['connect_timeout']);

if ($config['logging_enable']) {
$this->http->addDefaultLogger(
Expand Down
47 changes: 47 additions & 0 deletions tests/HttpTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Monolog\Handler\RotatingFileHandler;
use Monolog\Logger;
use Psr\Log\LogLevel;
use SubscribePro\App;
use SubscribePro\Http;

class HttpTest extends \PHPUnit_Framework_TestCase
Expand Down Expand Up @@ -108,6 +109,52 @@ public function addDefaultLoggerDataProvider()
];
}

/**
* When no connect timeout is configured (default 0), the Guzzle client must not
* receive a CONNECT_TIMEOUT option, preserving prior behavior for existing consumers.
*/
public function testCreateClientOmitsConnectTimeoutByDefault()
{
$client = $this->buildClientViaHttp(30);

$this->assertNull($client->getConfig(RequestOptions::CONNECT_TIMEOUT));
$this->assertEquals(30, $client->getConfig(RequestOptions::TIMEOUT));
}

/**
* When a connect timeout is configured (> 0), it must be passed to the Guzzle client
* as RequestOptions::CONNECT_TIMEOUT alongside the total request timeout.
*/
public function testCreateClientSetsConnectTimeoutWhenConfigured()
{
$client = $this->buildClientViaHttp(5, 2);

$this->assertEquals(2, $client->getConfig(RequestOptions::CONNECT_TIMEOUT));
$this->assertEquals(5, $client->getConfig(RequestOptions::TIMEOUT));
}

/**
* Build a real Guzzle client through Http::createClient() using reflection,
* so we assert the actual options Http passes to Guzzle.
*
* @param int|float $requestTimeout
* @param int|float|null $connectTimeout
*
* @return \GuzzleHttp\Client
*/
private function buildClientViaHttp($requestTimeout, $connectTimeout = null)
{
$app = new App('client-id', 'client-secret');
$http = null === $connectTimeout
? new Http($app, $requestTimeout)
: new Http($app, $requestTimeout, null, $connectTimeout);

$method = new \ReflectionMethod(Http::class, 'createClient');
$method->setAccessible(true);

return $method->invoke($http);
}

public function testAddLogger()
{
$logger = new Logger('Logger');
Expand Down