diff --git a/lib/Client.php b/lib/Client.php index 147859d..744beca 100644 --- a/lib/Client.php +++ b/lib/Client.php @@ -94,7 +94,7 @@ public function __construct( false, $options["debug"] ?? false, null, - (int) ($options['timeout'] ?? 10000) + (int) ($options['timeout'] ?? HttpClient::DEFAULT_CURL_TIMEOUT) ); $this->featureFlagsRequestTimeout = (int) ($options['feature_flag_request_timeout_ms'] ?? 3000); $this->featureFlags = []; diff --git a/lib/Consumer/ForkCurl.php b/lib/Consumer/ForkCurl.php index 5a16779..b0c2fb8 100644 --- a/lib/Consumer/ForkCurl.php +++ b/lib/Consumer/ForkCurl.php @@ -2,6 +2,7 @@ namespace PostHog\Consumer; +use PostHog\HttpClient; use PostHog\QueueConsumer; class ForkCurl extends QueueConsumer @@ -32,6 +33,25 @@ public function getConsumer() return $this->type; } + /** + * Converts the configured timeout from milliseconds to integer seconds for curl. + * @return int|null Integer seconds or null for unlimited. + */ + private function timeoutSeconds(): ?int + { + $ms = isset($this->options['timeout']) ? (int)$this->options['timeout'] : HttpClient::DEFAULT_CURL_TIMEOUT; + if ($ms <= 0) { + return null; + } + $seconds = (int)ceil($ms / 1000); + return max(1, $seconds); + } + + protected function runCommand(string $cmd, ?array &$output, ?int &$exit): void + { + exec($cmd, $output, $exit); + } + /** * Make an async request to our API. Fork a curl process, immediately send * to the API. If debug is enabled, we wait for the response. @@ -58,7 +78,7 @@ public function flushBatch($messages) // Compress request to file $tmpfname = tempnam("/tmp", "forkcurl_"); $cmd2 = "echo " . $payload . " | gzip > " . $tmpfname; - exec($cmd2, $output, $exit); + $this->runCommand($cmd2, $output, $exit); if (0 != $exit) { $this->handleError($exit, $output); @@ -89,11 +109,17 @@ public function flushBatch($messages) $libVersion = $messages[0]['library_version']; $cmd .= " -H 'User-Agent: $libName/$libVersion'"; + // Timeout + $seconds = $this->timeoutSeconds(); + if ($seconds !== null) { + $cmd .= " --max-time $seconds --connect-timeout $seconds"; + } + if (!$this->debug()) { $cmd .= " > /dev/null 2>&1 &"; } - - exec($cmd, $output, $exit); + + $this->runCommand($cmd, $output, $exit); if (0 != $exit) { $this->handleError($exit, $output); diff --git a/lib/Consumer/LibCurl.php b/lib/Consumer/LibCurl.php index 973e960..84aca3c 100644 --- a/lib/Consumer/LibCurl.php +++ b/lib/Consumer/LibCurl.php @@ -30,7 +30,8 @@ public function __construct($apiKey, $options = [], ?HttpClient $httpClient = nu $this->maximum_backoff_duration, $this->compress_request, $this->debug(), - $this->options['error_handler'] ?? null + $this->options['error_handler'] ?? null, + $this->options['timeout'] ?? HttpClient::DEFAULT_CURL_TIMEOUT ); } diff --git a/lib/HttpClient.php b/lib/HttpClient.php index 7ca81f7..28a2c23 100644 --- a/lib/HttpClient.php +++ b/lib/HttpClient.php @@ -6,6 +6,8 @@ class HttpClient { + public const DEFAULT_CURL_TIMEOUT = 10000; + /** * @var string */ @@ -47,7 +49,7 @@ public function __construct( bool $compressRequests = false, bool $debug = false, ?Closure $errorHandler = null, - int $curlTimeoutMilliseconds = 10000 + int $curlTimeoutMilliseconds = self::DEFAULT_CURL_TIMEOUT ) { $this->host = $host; $this->useSsl = $useSsl; diff --git a/test/ConsumerForkCurlTest.php b/test/ConsumerForkCurlTest.php index 0e0536e..c266e79 100644 --- a/test/ConsumerForkCurlTest.php +++ b/test/ConsumerForkCurlTest.php @@ -4,6 +4,7 @@ use PHPUnit\Framework\TestCase; use PostHog\Client; +use ReflectionClass; class ConsumerForkCurlTest extends TestCase { @@ -60,4 +61,70 @@ public function testAlias(): void ) ); } + + public function testConfigurePositiveTimeout(): void + { + $consumer = new MockedForkCurlConsumer( + 'test_api_key', + array( + 'consumer' => 'fork_curl', + 'debug' => true, + 'timeout' => 1500, + ) + ); + + $rcClient = new ReflectionClass(Client::class); + $prop = $rcClient->getProperty('consumer'); + $prop->setAccessible(true); + $prop->setValue($this->client, $consumer); + + self::assertTrue( + $this->client->capture( + array( + 'distinctId' => 'some-user', + 'event' => "PHP Fork Curl'd\" Event", + ), + ) + ); + + $this->client->flush(); + + self::assertNotEmpty($consumer->commands); + $cmd = end($consumer->commands); + self::assertStringContainsString('--max-time 2', $cmd); + self::assertStringContainsString('--connect-timeout 2', $cmd); + } + + public function testConfigureUnlimitedTimeout(): void + { + $consumer = new MockedForkCurlConsumer( + 'test_api_key', + array( + 'consumer' => 'fork_curl', + 'debug' => true, + 'timeout' => 0, + ) + ); + + $rcClient = new ReflectionClass(Client::class); + $prop = $rcClient->getProperty('consumer'); + $prop->setAccessible(true); + $prop->setValue($this->client, $consumer); + + self::assertTrue( + $this->client->capture( + array( + 'distinctId' => 'some-user', + 'event' => "PHP Fork Curl'd\" Event", + ), + ) + ); + + $this->client->flush(); + + self::assertNotEmpty($consumer->commands); + $cmd = end($consumer->commands); + self::assertStringNotContainsString('max-time', $cmd); + self::assertStringNotContainsString('connect-timeout', $cmd); + } } diff --git a/test/ConsumerLibCurlTest.php b/test/ConsumerLibCurlTest.php index 8c47678..3445c0a 100644 --- a/test/ConsumerLibCurlTest.php +++ b/test/ConsumerLibCurlTest.php @@ -4,6 +4,8 @@ use PHPUnit\Framework\TestCase; use PostHog\Client; +use PostHog\Consumer\LibCurl; +use ReflectionClass; class ConsumerLibCurlTest extends TestCase { @@ -21,6 +23,53 @@ public function setUp(): void ); } + public function testApplyConstructOptionsToHttpClient() + { + $client = new Client( + 'test_api_key', + array( + 'consumer' => 'lib_curl', + 'ssl' => false, + 'maximum_backoff_duration' => 5000, + 'compress_request' => true, + 'debug' => true, + 'timeout' => 1234, + ) + ); + + $rcClient = new ReflectionClass($client); + $consumerProp = $rcClient->getProperty('consumer'); + $consumerProp->setAccessible(true); + $consumer = $consumerProp->getValue($client); + + $this->assertInstanceOf(LibCurl::class, $consumer); + + $rcConsumer = new ReflectionClass($consumer); + $httpProp = $rcConsumer->getProperty('httpClient'); + $httpProp->setAccessible(true); + $httpClient = $httpProp->getValue($consumer); + + $rcHttp = new ReflectionClass($httpClient); + + $expectedValues = array( + 'useSsl' => false, + 'maximumBackoffDuration' => 5000, + 'compressRequests' => true, + 'debug' => true, + 'errorHandler' => null, + 'curlTimeoutMilliseconds' => 1234, + ); + + foreach ($expectedValues as $name => $expected) { + self::assertTrue($rcHttp->hasProperty($name)); + + $prop = $rcHttp->getProperty($name); + $prop->setAccessible(true); + $actual = $prop->getValue($httpClient); + self::assertSame($expected, $actual); + } + } + public function testCapture(): void { self::assertTrue( diff --git a/test/MockedForkCurlConsumer.php b/test/MockedForkCurlConsumer.php new file mode 100644 index 0000000..efd0e02 --- /dev/null +++ b/test/MockedForkCurlConsumer.php @@ -0,0 +1,18 @@ +commands[] = $cmd; + $output = []; + $exit = $this->fakeExit; + } +}