From 6772d05129aceca9fcedccb5608fbe70072165bf Mon Sep 17 00:00:00 2001 From: chrismshea Date: Wed, 1 Jul 2026 17:35:06 +0200 Subject: [PATCH] feat: add opt-in connect_timeout support to SDK HTTP client Adds a new `connect_timeout` config key (default 0 = disabled) plumbed from Sdk config through Http to Guzzle RequestOptions::CONNECT_TIMEOUT. When unset/0, no CONNECT_TIMEOUT is passed, preserving prior behavior for existing consumers. The default api_request_timeout (30s) is unchanged. Also documents the timeout exception surface: a Guzzle connect/request timeout raises ConnectException/RequestException (not response-based), which is not represented by HttpException; consumers must catch the Guzzle transfer exceptions to handle timeouts. Refs subscribepro/subscribepro-php#56 --- src/SubscribePro/Http.php | 20 ++++++++++++++--- src/SubscribePro/Sdk.php | 15 ++++++++++++- tests/HttpTest.php | 47 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/src/SubscribePro/Http.php b/src/SubscribePro/Http.php index f8a4814..3b5d4a8 100644 --- a/src/SubscribePro/Http.php +++ b/src/SubscribePro/Http.php @@ -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; } /** @@ -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, @@ -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); } /** diff --git a/src/SubscribePro/Sdk.php b/src/SubscribePro/Sdk.php index 306043a..4614cde 100755 --- a/src/SubscribePro/Sdk.php +++ b/src/SubscribePro/Sdk.php @@ -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) * - * 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 @@ -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']) { @@ -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( diff --git a/tests/HttpTest.php b/tests/HttpTest.php index 56cfeb0..494aba0 100644 --- a/tests/HttpTest.php +++ b/tests/HttpTest.php @@ -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 @@ -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');