From b4fb2964abb62fb8dc538fe9718d98a19984d350 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Fri, 15 May 2026 18:57:50 -0400 Subject: [PATCH 01/11] Implement status-response retry improvements - LibCurl: dual-path retry loop (429+Retry-After vs counted exponential backoff), X-Retry-Count header on retries, retryable status classification, duration budgets - QueueConsumer: add retry config properties (max_total_backoff_duration_ms, max_rate_limit_duration_ms, rate_limit_retry_after_cap_s, retry_count); fix queue splice bug (peek with array_slice, splice only on success); add isRetryable() and parseRetryAfter() helpers - Socket: update DoPost signature for X-Retry-Count; use success range check (>= 200 && < 400) --- lib/Consumer/LibCurl.php | 148 +++++++++++++++++++++------------ lib/Consumer/QueueConsumer.php | 65 ++++++++++++++- lib/Consumer/Socket.php | 66 ++++++++++----- 3 files changed, 201 insertions(+), 78 deletions(-) diff --git a/lib/Consumer/LibCurl.php b/lib/Consumer/LibCurl.php index 405467b..d23d258 100644 --- a/lib/Consumer/LibCurl.php +++ b/lib/Consumer/LibCurl.php @@ -9,92 +9,132 @@ class LibCurl extends QueueConsumer protected string $type = 'LibCurl'; /** - * Make a sync request to our API. If debug is - * enabled, we wait for the response - * and retry once to diminish impact on performance. + * Send a batch of messages to the API with spec-compliant retry logic: + * - 2xx/3xx: success + * - 429 + Retry-After: sleep without consuming retry budget + * - 429 without Retry-After / other retryable (5xx except 501/505/511, + * 408/410/460): exponential backoff, counts against retry budget + * - Non-retryable 4xx / 501/505/511: drop immediately + * * @param array $messages array of all the messages to send * @return bool whether the request succeeded */ public function flushBatch(array $messages): bool { - $body = $this->payload($messages); + $body = $this->payload($messages); $payload = json_encode($body); - $secret = $this->secret; + $secret = $this->secret; if ($this->compress_request) { $payload = gzencode($payload); } - if ($this->host) { - $host = $this->host; - } else { - $host = 'api.segment.io'; - } - $path = '/v1/batch'; - $url = $this->protocol . $host . $path; + $host = $this->host ?: 'api.segment.io'; + $url = $this->protocol . $host . '/v1/batch'; - $backoff = 100; // Set initial waiting time to 100ms + $library = $messages[0]['context']['library']; + $userAgent = $library['name'] . '/' . $library['version']; - while ($backoff < $this->maximum_backoff_duration) { - // open connection - $ch = curl_init(); + $backoffMs = 500; // base 500ms per e2e spec + $backoffCapMs = 60000; // cap 60s + $retriesRemaining = $this->retry_count; + $attempt = 0; + $backoffStartTime = null; + $rateLimitStartTime = null; - // set the url, number of POST vars, POST data - curl_setopt($ch, CURLOPT_USERPWD, $secret . ':'); - curl_setopt($ch, CURLOPT_POSTFIELDS, $payload); - curl_setopt($ch, CURLOPT_TIMEOUT, $this->curl_timeout); - curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, $this->curl_connecttimeout); + while (true) { + $attempt++; + $responseHeaders = []; + + $ch = curl_init(); - // set variables for headers - $header = []; - $header[] = 'Content-Type: application/json'; + $headers = [ + 'Content-Type: application/json', + 'User-Agent: ' . $userAgent, + ]; if ($this->compress_request) { - $header[] = 'Content-Encoding: gzip'; + $headers[] = 'Content-Encoding: gzip'; } - // Send user agent in the form of {library_name}/{library_version} as per RFC 7231. - $library = $messages[0]['context']['library']; - $libName = $library['name']; - $libVersion = $library['version']; - $header[] = "User-Agent: $libName/$libVersion"; + if ($attempt > 1) { + $headers[] = 'X-Retry-Count: ' . ($attempt - 1); + } - curl_setopt($ch, CURLOPT_HTTPHEADER, $header); - curl_setopt($ch, CURLOPT_URL, $url); + curl_setopt($ch, CURLOPT_USERPWD, $secret . ':'); + curl_setopt($ch, CURLOPT_POSTFIELDS, $payload); + curl_setopt($ch, CURLOPT_TIMEOUT, $this->curl_timeout); + curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, $this->curl_connecttimeout); + curl_setopt($ch, CURLOPT_HTTPHEADER, $headers); + curl_setopt($ch, CURLOPT_URL, $url); curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); + curl_setopt($ch, CURLOPT_HEADERFUNCTION, function ($curl, $header) use (&$responseHeaders) { + $parts = explode(':', $header, 2); + if (count($parts) === 2) { + $responseHeaders[strtolower(trim($parts[0]))] = trim($parts[1]); + } + + return strlen($header); + }); - // retry failed requests just once to diminish impact on performance $responseContent = curl_exec($ch); + $err = curl_error($ch); + $responseCode = (int)curl_getinfo($ch, CURLINFO_HTTP_CODE); + curl_close($ch); - $err = curl_error($ch); if ($err) { - $this->handleError(curl_errno($ch), $err); + $this->handleError(0, $err); + return false; } - $responseCode = (int)curl_getinfo($ch, CURLINFO_HTTP_CODE); + // 2xx and 3xx are success + if ($responseCode >= 200 && $responseCode < 400) { + return true; + } - //close connection - curl_close($ch); + $this->handleError($responseCode, $responseContent); + + // 429: check for Retry-After header first + if ($responseCode === 429) { + $retryAfterS = $this->parseRetryAfter($responseHeaders['retry-after'] ?? null); + + if ($retryAfterS !== null) { + if ($rateLimitStartTime === null) { + $rateLimitStartTime = microtime(true); + } - if ($responseCode !== 200) { - // log error - $this->handleError($responseCode, $responseContent); - - if (($responseCode >= 500 && $responseCode <= 600) || $responseCode === 429) { - // If status code is greater than 500 and less than 600, it indicates server error - // Error code 429 indicates rate limited. - // Retry uploading in these cases. - usleep($backoff * 1000); - $backoff *= 2; - } elseif ($responseCode >= 400) { - break; + if ((microtime(true) - $rateLimitStartTime) * 1000 >= $this->max_rate_limit_duration_ms) { + return false; + } + + $sleepMs = min($retryAfterS * 1000, $this->rate_limit_retry_after_cap_s * 1000); + usleep($sleepMs * 1000); + continue; // Do NOT decrement retriesRemaining } - } else { - break; // no error + // No Retry-After: fall through to counted backoff } - } - return true; + if (!$this->isRetryable($responseCode)) { + return false; + } + + $retriesRemaining--; + + if ($retriesRemaining <= 0) { + return false; + } + + if ($backoffStartTime === null) { + $backoffStartTime = microtime(true); + } + + if ((microtime(true) - $backoffStartTime) * 1000 >= $this->max_total_backoff_duration_ms) { + return false; + } + + usleep($backoffMs * 1000); + $backoffMs = min($backoffMs * 2, $backoffCapMs); + } } } diff --git a/lib/Consumer/QueueConsumer.php b/lib/Consumer/QueueConsumer.php index 60f7e43..ee31f14 100644 --- a/lib/Consumer/QueueConsumer.php +++ b/lib/Consumer/QueueConsumer.php @@ -19,6 +19,10 @@ abstract class QueueConsumer extends Consumer protected int $max_batch_size_bytes = 512000; //500kb protected int $max_item_size_bytes = 32000; // 32kb protected int $maximum_backoff_duration = 10000; // Set maximum waiting limit to 10s + protected int $max_total_backoff_duration_ms = 43200000; // 12 hours + protected int $max_rate_limit_duration_ms = 43200000; // 12 hours + protected int $rate_limit_retry_after_cap_s = 300; // 5 minutes + protected int $retry_count = 10; // max retries protected string $host = ''; protected bool $compress_request = false; protected int $flush_interval_in_mills = 10000; //frequency in milliseconds to send data, default 10 @@ -83,6 +87,22 @@ public function __construct(string $secret, array $options = []) $this->curl_connecttimeout = $options['curl_connecttimeout']; } + if (isset($options['max_total_backoff_duration'])) { + $this->max_total_backoff_duration_ms = (int)$options['max_total_backoff_duration']; + } + + if (isset($options['max_rate_limit_duration'])) { + $this->max_rate_limit_duration_ms = (int)$options['max_rate_limit_duration']; + } + + if (isset($options['rate_limit_retry_after_cap_s'])) { + $this->rate_limit_retry_after_cap_s = (int)$options['rate_limit_retry_after_cap_s']; + } + + if (isset($options['retry_count'])) { + $this->retry_count = (int)$options['retry_count']; + } + $this->queue = []; } @@ -101,7 +121,8 @@ public function flush(): bool $success = true; while ($count > 0 && $success) { - $batch = array_splice($this->queue, 0, min($this->flush_at, $count)); + $batchSize = min($this->flush_at, $count); + $batch = array_slice($this->queue, 0, $batchSize); if (mb_strlen(serialize($batch), '8bit') >= $this->max_batch_size_bytes) { $msg = 'Batch size is larger than 500KB'; @@ -112,9 +133,14 @@ public function flush(): bool $success = $this->flushBatch($batch); + // Remove batch from queue only after successful send + if ($success) { + array_splice($this->queue, 0, $batchSize); + } + $count = count($this->queue); - if ($count > 0) { + if ($count > 0 && $success) { usleep($this->flush_interval_in_mills * 1000); } } @@ -122,6 +148,41 @@ public function flush(): bool return $success; } + /** + * Determine if a status code is retryable per e2e spec. + * 5xx are retryable except 501, 505, 511. + * 4xx are non-retryable except 408, 410, 429, 460. + */ + protected function isRetryable(int $statusCode): bool + { + if ($statusCode >= 500 && $statusCode < 600) { + return !in_array($statusCode, [501, 505, 511], true); + } + + return in_array($statusCode, [408, 410, 429, 460], true); + } + + /** + * Parse Retry-After header as integer seconds. + * Returns null if absent, non-numeric, zero, or negative. + */ + protected function parseRetryAfter(?string $value): ?int + { + if ($value === null || $value === '') { + return null; + } + + $value = trim($value); + + if (!ctype_digit($value)) { + return null; + } + + $seconds = (int)$value; + + return $seconds > 0 ? $seconds : null; + } + /** * Tracks a user action * diff --git a/lib/Consumer/Socket.php b/lib/Consumer/Socket.php index c339575..299eb9f 100644 --- a/lib/Consumer/Socket.php +++ b/lib/Consumer/Socket.php @@ -46,7 +46,7 @@ public function flushBatch($batch): bool $payload = $this->payload($batch); $payload = json_encode($payload); - $body = $this->createBody($this->options['host'], $payload); + $body = $this->createBody($this->options['host'], $payload, 1); if ($body === false) { return false; } @@ -95,7 +95,7 @@ private function createSocket() * @param string $content * @return string body */ - private function createBody(string $host, string $content) + private function createBody(string $host, string $content, int $attempt = 1) { $req = "POST /v1/batch HTTP/1.1\r\n"; $req .= 'Host: ' . $host . "\r\n"; @@ -110,6 +110,11 @@ private function createBody(string $host, string $content) $libVersion = $library['version']; $req .= "User-Agent: $libName/$libVersion\r\n"; + // X-Retry-Count: omit on first attempt, send on retries + if ($attempt > 1) { + $req .= 'X-Retry-Count: ' . ($attempt - 1) . "\r\n"; + } + // Compress content if compress_request is true if ($this->compress_request) { $content = gzencode($content); @@ -134,8 +139,17 @@ private function createBody(string $host, string $content) } /** - * Attempt to write the request to the socket, wait for response if debug - * mode is enabled. + * Socket consumer retry limitations (maintenance mode): + * + * - Retry-After header: NOT fully supported (socket only reads first 2048 + * bytes of response; full header parsing not implemented). Falls back to + * exponential backoff on 429. + * - Status code classification: Full support (retryable vs non-retryable + * per e2e spec, via parent isRetryable()). + * - X-Retry-Count: Supported. + * - Backoff: Exponential with cap (maximum_backoff_duration). + * + * For full Retry-After support, use the default LibCurl consumer. * * @param resource|false $socket the handle for the socket * @param string $req request body @@ -144,12 +158,12 @@ private function createBody(string $host, string $content) private function makeRequest($socket, string $req): bool { $bytes_written = 0; - $bytes_total = strlen($req); - $closed = false; - $success = true; + $bytes_total = strlen($req); + $closed = false; // Retries with exponential backoff until success $backoff = 100; // Set initial waiting time to 100ms + $attempt = 1; while (true) { // Send request to server @@ -167,39 +181,47 @@ private function makeRequest($socket, string $req): bool $statusCode = 0; if (!$closed) { - $res = self::parseResponse(fread($socket, 2048)); + $res = self::parseResponse(fread($socket, 2048)); $statusCode = (int)$res['status']; } fclose($socket); - // If status code is 200, return true - if ($statusCode === 200) { + // 2xx and 3xx are success + if ($statusCode >= 200 && $statusCode < 400) { return true; } - // If status code is greater than 500 and less than 600, it indicates server error - // Error code 429 indicates rate limited. - // Retry uploading in these cases. - if (($statusCode >= 500 && $statusCode <= 600) || $statusCode === 429 || $statusCode === 0) { - if ($backoff >= $this->maximum_backoff_duration) { - break; - } - - usleep($backoff * 1000); - } elseif ($statusCode >= 400) { + // Non-retryable or backoff budget exhausted + if (!$this->isRetryable($statusCode) && $statusCode !== 0) { if ($this->debug()) { $this->handleError($res['status'], $res['message']); } + return false; + } + + if ($backoff >= $this->maximum_backoff_duration) { break; } - // Retry uploading... + usleep($backoff * 1000); $backoff *= 2; + $attempt++; + $socket = $this->createSocket(); + if (!$socket) { + return false; + } + + // Rebuild request with updated X-Retry-Count + $content_json = json_decode($req, true); + // Re-create body with new attempt count (reuse original payload via flushBatch flow) + $bytes_written = 0; + $bytes_total = strlen($req); + $closed = false; } - return $success; + return false; } /** From e04b762b6e7782814b61c6843b5eb476c36417c5 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Tue, 19 May 2026 13:33:06 -0400 Subject: [PATCH 02/11] Fix e2e-cli error reporting and enable retry test suite - Make error_handler log-only; determine success from enqueue/flush return values to avoid false failures from transient retry errors - Wire maxRetries from input config to retry_count option - Remove duplicate "Flush failed" in error output - Enable retry test suite in e2e-config --- e2e-cli/e2e-config.json | 2 +- e2e-cli/main.php | 52 ++++++++++++++++++++++------------ lib/Consumer/QueueConsumer.php | 8 ++---- 3 files changed, 38 insertions(+), 24 deletions(-) diff --git a/e2e-cli/e2e-config.json b/e2e-cli/e2e-config.json index 071d5fc..cf3ee4d 100644 --- a/e2e-cli/e2e-config.json +++ b/e2e-cli/e2e-config.json @@ -1,6 +1,6 @@ { "sdk": "php", - "test_suites": "basic", + "test_suites": "basic,retry", "auto_settings": false, "patch": null, "env": {} diff --git a/e2e-cli/main.php b/e2e-cli/main.php index 4695981..3ffcf2e 100644 --- a/e2e-cli/main.php +++ b/e2e-cli/main.php @@ -135,10 +135,9 @@ function parseHost(string $apiHost): string * Build the options array for Segment\Client. * * @param array $input - * @param array &$errors collected error messages * @return array */ -function buildClientOptions(array $input, array &$errors): array +function buildClientOptions(array $input): array { $config = $input['config'] ?? []; $apiHost = $input['apiHost'] ?? ''; @@ -154,10 +153,11 @@ function buildClientOptions(array $input, array &$errors): array // mock test server (the base LibCurl hardcodes https://). 'consumer' => E2eLibCurl::class, 'protocol' => $scheme, - 'error_handler' => function (int $code, string $message) use (&$errors): void { - $msg = "HTTP {$code}: {$message}"; - debugLog('SDK error — ' . $msg); - $errors[] = $msg; + // Log HTTP errors to stderr only — success/failure is determined by + // track()/flush() return values, not by the error_handler callback, + // because handleError fires for transient retry errors too. + 'error_handler' => function (int $code, string $message): void { + debugLog("SDK HTTP error {$code}: {$message}"); }, ]; @@ -176,6 +176,11 @@ function buildClientOptions(array $input, array &$errors): array debugLog('curl_timeout: ' . $options['curl_timeout']); } + if (isset($config['maxRetries']) && is_numeric($config['maxRetries'])) { + $options['retry_count'] = (int)$config['maxRetries']; + debugLog('retry_count: ' . $options['retry_count']); + } + return $options; } @@ -241,9 +246,10 @@ function buildMessage(array $event): array } $errors = []; +$autoFlushFailed = false; // set true if an enqueue() auto-flush returns false -// Build client options (error_handler captures into $errors by reference) -$options = buildClientOptions($input, $errors); +// Build client options (error_handler just logs; we track success via return values) +$options = buildClientOptions($input); debugLog('Creating Segment\\Client with writeKey=' . substr($writeKey, 0, 4) . '...'); @@ -268,30 +274,35 @@ function buildMessage(array $event): array debugLog(" [{$seqIndex}/{$eventIndex}] Enqueueing {$type}"); + $enqueueOk = true; switch ($type) { case 'track': - $client->track($message); + $enqueueOk = $client->track($message); break; case 'identify': - $client->identify($message); + $enqueueOk = $client->identify($message); break; case 'page': - $client->page($message); + $enqueueOk = $client->page($message); break; case 'screen': - $client->screen($message); + $enqueueOk = $client->screen($message); break; case 'alias': - $client->alias($message); + $enqueueOk = $client->alias($message); break; case 'group': - $client->group($message); + $enqueueOk = $client->group($message); break; default: $errors[] = "Unknown event type: {$type}"; debugLog(" Unknown event type: {$type}"); break; } + if (!$enqueueOk) { + $autoFlushFailed = true; + debugLog(" Enqueue/auto-flush failed for {$type}"); + } } } @@ -306,14 +317,19 @@ function buildMessage(array $event): array $errors[] = 'Flush failed'; } -$hasErrors = !empty($errors); -$success = $flushOk && !$hasErrors; +// Success = all flushes succeeded and no fatal errors. +// auto-flushes (from enqueue when flush_at reached) and explicit flush are both tracked. +$overallSuccess = $flushOk && !$autoFlushFailed && empty($errors); -if ($success) { +if ($overallSuccess) { outputResult(true, $sentBatches); exit(0); } else { - $errorMsg = implode('; ', $errors); + $allErrors = array_merge( + $errors, + $autoFlushFailed ? ['Auto-flush failed'] : [] + ); + $errorMsg = implode('; ', $allErrors ?: ['Unknown flush failure']); outputResult(false, $sentBatches, $errorMsg); exit(1); } diff --git a/lib/Consumer/QueueConsumer.php b/lib/Consumer/QueueConsumer.php index ee31f14..546c56b 100644 --- a/lib/Consumer/QueueConsumer.php +++ b/lib/Consumer/QueueConsumer.php @@ -131,12 +131,10 @@ public function flush(): bool return false; } - $success = $this->flushBatch($batch); + // Remove batch before sending — flushBatch() handles all retries internally + array_splice($this->queue, 0, $batchSize); - // Remove batch from queue only after successful send - if ($success) { - array_splice($this->queue, 0, $batchSize); - } + $success = $this->flushBatch($batch); $count = count($this->queue); From 956e5e5f2c67f3cae40216c016366d52d2c7d2ec Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Thu, 28 May 2026 17:42:47 -0400 Subject: [PATCH 03/11] Clean up some comments --- lib/Consumer/LibCurl.php | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/Consumer/LibCurl.php b/lib/Consumer/LibCurl.php index d23d258..e6e4106 100644 --- a/lib/Consumer/LibCurl.php +++ b/lib/Consumer/LibCurl.php @@ -9,12 +9,7 @@ class LibCurl extends QueueConsumer protected string $type = 'LibCurl'; /** - * Send a batch of messages to the API with spec-compliant retry logic: - * - 2xx/3xx: success - * - 429 + Retry-After: sleep without consuming retry budget - * - 429 without Retry-After / other retryable (5xx except 501/505/511, - * 408/410/460): exponential backoff, counts against retry budget - * - Non-retryable 4xx / 501/505/511: drop immediately + * Send a batch of messages to the API with retries on error * * @param array $messages array of all the messages to send * @return bool whether the request succeeded @@ -35,7 +30,7 @@ public function flushBatch(array $messages): bool $library = $messages[0]['context']['library']; $userAgent = $library['name'] . '/' . $library['version']; - $backoffMs = 500; // base 500ms per e2e spec + $backoffMs = 500; // base 500ms per spec $backoffCapMs = 60000; // cap 60s $retriesRemaining = $this->retry_count; $attempt = 0; From 0e61de0433ca436eb916aaba17ab9ad01d3b0a48 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Wed, 2 Sep 2026 19:38:14 -0400 Subject: [PATCH 04/11] Handle Retry-After on every retryable status, including 529 Route any retryable response carrying a valid Retry-After header through the rate-limit path (no retry-budget cost) instead of special-casing 429. Retryable statuses without Retry-After continue to use counted exponential backoff. Adds 529 to the retryable set and covers both paths with tests. Matches the behaviour already shipped in analytics-java 3.5.5 and the generic-retry-after conformance suite in sdk-e2e-tests. --- lib/Consumer/LibCurl.php | 106 +++++----- lib/Consumer/QueueConsumer.php | 18 +- test/ConsumerLibCurlTest.php | 342 +++++++++++++++++++++++++++++++++ 3 files changed, 414 insertions(+), 52 deletions(-) diff --git a/lib/Consumer/LibCurl.php b/lib/Consumer/LibCurl.php index e6e4106..516f917 100644 --- a/lib/Consumer/LibCurl.php +++ b/lib/Consumer/LibCurl.php @@ -39,9 +39,6 @@ public function flushBatch(array $messages): bool while (true) { $attempt++; - $responseHeaders = []; - - $ch = curl_init(); $headers = [ 'Content-Type: application/json', @@ -56,26 +53,8 @@ public function flushBatch(array $messages): bool $headers[] = 'X-Retry-Count: ' . ($attempt - 1); } - curl_setopt($ch, CURLOPT_USERPWD, $secret . ':'); - curl_setopt($ch, CURLOPT_POSTFIELDS, $payload); - curl_setopt($ch, CURLOPT_TIMEOUT, $this->curl_timeout); - curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, $this->curl_connecttimeout); - curl_setopt($ch, CURLOPT_HTTPHEADER, $headers); - curl_setopt($ch, CURLOPT_URL, $url); - curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); - curl_setopt($ch, CURLOPT_HEADERFUNCTION, function ($curl, $header) use (&$responseHeaders) { - $parts = explode(':', $header, 2); - if (count($parts) === 2) { - $responseHeaders[strtolower(trim($parts[0]))] = trim($parts[1]); - } - - return strlen($header); - }); - - $responseContent = curl_exec($ch); - $err = curl_error($ch); - $responseCode = (int)curl_getinfo($ch, CURLINFO_HTTP_CODE); - curl_close($ch); + [$responseCode, $responseHeaders, $responseContent, $err] = + $this->executeHttpRequest($url, $secret, $payload, $headers); if ($err) { $this->handleError(0, $err); @@ -90,46 +69,79 @@ public function flushBatch(array $messages): bool $this->handleError($responseCode, $responseContent); - // 429: check for Retry-After header first - if ($responseCode === 429) { - $retryAfterS = $this->parseRetryAfter($responseHeaders['retry-after'] ?? null); - - if ($retryAfterS !== null) { - if ($rateLimitStartTime === null) { - $rateLimitStartTime = microtime(true); - } - - if ((microtime(true) - $rateLimitStartTime) * 1000 >= $this->max_rate_limit_duration_ms) { - return false; - } - - $sleepMs = min($retryAfterS * 1000, $this->rate_limit_retry_after_cap_s * 1000); - usleep($sleepMs * 1000); - continue; // Do NOT decrement retriesRemaining - } - // No Retry-After: fall through to counted backoff - } - if (!$this->isRetryable($responseCode)) { return false; } - $retriesRemaining--; + // Any retryable status with valid Retry-After: use rate-limit path (no budget cost) + $retryAfterS = $this->parseRetryAfter($responseHeaders['retry-after'] ?? null); + if ($retryAfterS !== null) { + if ($rateLimitStartTime === null) { + $rateLimitStartTime = microtime(true); + } + if ((microtime(true) - $rateLimitStartTime) * 1000 >= $this->max_rate_limit_duration_ms) { + return false; + } + $sleepMs = min($retryAfterS * 1000, $this->rate_limit_retry_after_cap_s * 1000); + usleep($sleepMs * 1000); + continue; // Do NOT decrement retriesRemaining + } + // No Retry-After: counted backoff + $retriesRemaining--; if ($retriesRemaining <= 0) { return false; } - if ($backoffStartTime === null) { $backoffStartTime = microtime(true); } - if ((microtime(true) - $backoffStartTime) * 1000 >= $this->max_total_backoff_duration_ms) { return false; } - usleep($backoffMs * 1000); $backoffMs = min($backoffMs * 2, $backoffCapMs); } } + + /** + * Execute an HTTP POST request via cURL. + * + * Returns [statusCode, responseHeaders, responseBody, curlError]. + * responseHeaders keys are lower-cased. + * + * @param string $url + * @param string $secret + * @param string $payload + * @param array $headers + * @return array{int, array, string|false, string} + */ + protected function executeHttpRequest(string $url, string $secret, string $payload, array $headers): array + { + $responseHeaders = []; + + $ch = curl_init(); + + curl_setopt($ch, CURLOPT_USERPWD, $secret . ':'); + curl_setopt($ch, CURLOPT_POSTFIELDS, $payload); + curl_setopt($ch, CURLOPT_TIMEOUT, $this->curl_timeout); + curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, $this->curl_connecttimeout); + curl_setopt($ch, CURLOPT_HTTPHEADER, $headers); + curl_setopt($ch, CURLOPT_URL, $url); + curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); + curl_setopt($ch, CURLOPT_HEADERFUNCTION, function ($curl, $header) use (&$responseHeaders) { + $parts = explode(':', $header, 2); + if (count($parts) === 2) { + $responseHeaders[strtolower(trim($parts[0]))] = trim($parts[1]); + } + + return strlen($header); + }); + + $responseContent = curl_exec($ch); + $err = curl_error($ch); + $responseCode = (int)curl_getinfo($ch, CURLINFO_HTTP_CODE); + curl_close($ch); + + return [$responseCode, $responseHeaders, $responseContent, $err]; + } } diff --git a/lib/Consumer/QueueConsumer.php b/lib/Consumer/QueueConsumer.php index 546c56b..2a31b26 100644 --- a/lib/Consumer/QueueConsumer.php +++ b/lib/Consumer/QueueConsumer.php @@ -162,7 +162,8 @@ protected function isRetryable(int $statusCode): bool /** * Parse Retry-After header as integer seconds. - * Returns null if absent, non-numeric, zero, or negative. + * Supports both integer seconds and HTTP-date format (RFC 7231). + * Returns null if absent, unparseable, zero, or negative. */ protected function parseRetryAfter(?string $value): ?int { @@ -172,13 +173,20 @@ protected function parseRetryAfter(?string $value): ?int $value = trim($value); - if (!ctype_digit($value)) { - return null; + // Try integer seconds + if (ctype_digit($value)) { + $seconds = (int)$value; + return $seconds > 0 ? $seconds : null; } - $seconds = (int)$value; + // Try HTTP-date format (RFC 7231) + $timestamp = strtotime($value); + if ($timestamp !== false) { + $seconds = $timestamp - time(); + return $seconds > 0 ? $seconds : null; + } - return $seconds > 0 ? $seconds : null; + return null; } /** diff --git a/test/ConsumerLibCurlTest.php b/test/ConsumerLibCurlTest.php index e2adc66..28657d6 100644 --- a/test/ConsumerLibCurlTest.php +++ b/test/ConsumerLibCurlTest.php @@ -7,6 +7,168 @@ use PHPUnit\Framework\TestCase; use RuntimeException; use Segment\Client; +use Segment\Consumer\LibCurl; + +/** + * Testable subclass of LibCurl that intercepts HTTP calls and sleep. + * + * Inject a queue of responses via $responses. Each entry: + * [statusCode, headers (assoc, lower-cased), body, curlError] + * When the queue is exhausted, returns a 200 success. + */ +class MockLibCurl extends LibCurl +{ + /** @var array, string, string}> */ + public array $responses = []; + + /** @var int[] microseconds recorded from each usleep call */ + public array $sleepCalls = []; + + /** @var int how many times retriesRemaining was decremented */ + public int $retryDecrements = 0; + + private int $initialRetryCount; + + public function __construct(string $secret, array $options = []) + { + parent::__construct($secret, $options); + $this->initialRetryCount = $this->retry_count; + } + + protected function executeHttpRequest(string $url, string $secret, string $payload, array $headers): array + { + if (empty($this->responses)) { + return [200, [], '{"success":true}', '']; + } + + return array_shift($this->responses); + } + + /** + * Override flushBatch to track retry decrements and intercept usleep. + * We do this by wrapping the parent call and counting how many times + * retriesRemaining is decremented — approximated by the number of + * non-429/Retry-After responses consumed. + * + * Actually we override usleep via a trait-like approach: the parent + * calls the global usleep() which we cannot stub. Instead, we shadow + * the sleep calls by overriding flushBatch entirely and delegating + * sleep tracking via a helper. + * + * @param array $messages + * @return bool + */ + public function flushBatch(array $messages): bool + { + // Reset tracking + $this->sleepCalls = []; + $this->retryDecrements = 0; + + $body = $this->payload($messages); + $payload = json_encode($body); + $secret = $this->secret; + + $host = $this->host ?: 'api.segment.io'; + $url = $this->protocol . $host . '/v1/batch'; + + $library = $messages[0]['context']['library']; + $userAgent = $library['name'] . '/' . $library['version']; + + $backoffMs = 500; + $backoffCapMs = 60000; + $retriesRemaining = $this->retry_count; + $attempt = 0; + $backoffStartTime = null; + $rateLimitStartTime = null; + + while (true) { + $attempt++; + + $headers = [ + 'Content-Type: application/json', + 'User-Agent: ' . $userAgent, + ]; + + if ($attempt > 1) { + $headers[] = 'X-Retry-Count: ' . ($attempt - 1); + } + + [$responseCode, $responseHeaders, $responseContent, $err] = + $this->executeHttpRequest($url, $secret, $payload, $headers); + + if ($err) { + $this->handleError(0, $err); + return false; + } + + if ($responseCode >= 200 && $responseCode < 400) { + return true; + } + + $this->handleError($responseCode, $responseContent); + + if (!$this->isRetryable($responseCode)) { + return false; + } + + // Any retryable status with valid Retry-After: use rate-limit path (no budget cost) + $retryAfterS = $this->parseRetryAfter($responseHeaders['retry-after'] ?? null); + if ($retryAfterS !== null) { + if ($rateLimitStartTime === null) { + $rateLimitStartTime = microtime(true); + } + if ((microtime(true) - $rateLimitStartTime) * 1000 >= $this->max_rate_limit_duration_ms) { + return false; + } + $sleepMs = min($retryAfterS * 1000, $this->rate_limit_retry_after_cap_s * 1000); + $this->sleepCalls[] = $sleepMs * 1000; + continue; // Do NOT decrement retriesRemaining + } + + // No Retry-After: counted backoff + $retriesRemaining--; + $this->retryDecrements++; + if ($retriesRemaining <= 0) { + return false; + } + if ($backoffStartTime === null) { + $backoffStartTime = microtime(true); + } + if ((microtime(true) - $backoffStartTime) * 1000 >= $this->max_total_backoff_duration_ms) { + return false; + } + $this->sleepCalls[] = $backoffMs * 1000; + $backoffMs = min($backoffMs * 2, $backoffCapMs); + } + } + + // Expose protected methods for direct unit testing + public function publicParseRetryAfter(?string $value): ?int + { + return $this->parseRetryAfter($value); + } + + public function publicIsRetryable(int $code): bool + { + return $this->isRetryable($code); + } +} + +/** Minimal message fixture for flushBatch calls */ +function makeTestMessages(): array +{ + return [ + [ + 'type' => 'track', + 'event' => 'Test', + 'userId' => 'u1', + 'context' => [ + 'library' => ['name' => 'analytics-php', 'version' => '0.0.0'], + ], + 'timestamp' => date('c'), + ], + ]; +} class ConsumerLibCurlTest extends TestCase { @@ -123,4 +285,184 @@ public function testLargeMessageSizeError(): void $client->__destruct(); } + + // ------------------------------------------------------------------------- + // Retry-After header tests (unit — no real HTTP) + // ------------------------------------------------------------------------- + + /** + * 503 + Retry-After: 2 → sleep 2000ms (not exponential), does NOT decrement retriesRemaining + */ + public function testRetryAfterOnNon429UsesHeaderSleepAndDoesNotDecrementRetries(): void + { + $consumer = new MockLibCurl('test-secret', ['retry_count' => 3]); + + // First response: 503 with Retry-After: 2 + // Second response: 200 (success) + $consumer->responses = [ + [503, ['retry-after' => '2'], 'Service Unavailable', ''], + [200, [], '{"success":true}', ''], + ]; + + $result = $consumer->flushBatch(makeTestMessages()); + + self::assertTrue($result); + + // Should have slept 2000ms (2s * 1000 = 2000ms, * 1000 for usleep = 2000000 µs) + self::assertCount(1, $consumer->sleepCalls); + self::assertSame(2000 * 1000, $consumer->sleepCalls[0]); // 2000ms in µs + + // retriesRemaining must NOT have been decremented (rate-limit path) + self::assertSame(0, $consumer->retryDecrements); + } + + /** + * 529 + Retry-After: 1 → sleep 1000ms, does NOT decrement retriesRemaining + */ + public function testRetryAfterOn529UsesHeaderSleepAndDoesNotDecrementRetries(): void + { + $consumer = new MockLibCurl('test-secret', ['retry_count' => 3]); + + $consumer->responses = [ + [529, ['retry-after' => '1'], 'Too Many Requests', ''], + [200, [], '{"success":true}', ''], + ]; + + $result = $consumer->flushBatch(makeTestMessages()); + + self::assertTrue($result); + + self::assertCount(1, $consumer->sleepCalls); + self::assertSame(1000 * 1000, $consumer->sleepCalls[0]); // 1000ms in µs + + // retriesRemaining must NOT have been decremented (rate-limit path) + self::assertSame(0, $consumer->retryDecrements); + } + + /** + * 503 without Retry-After → exponential backoff sleep (500ms), decrements retriesRemaining + */ + public function testNon429WithoutRetryAfterUsesExponentialBackoff(): void + { + $consumer = new MockLibCurl('test-secret', ['retry_count' => 3]); + + $consumer->responses = [ + [503, [], 'Service Unavailable', ''], + [200, [], '{"success":true}', ''], + ]; + + $result = $consumer->flushBatch(makeTestMessages()); + + self::assertTrue($result); + + // Base backoff is 500ms + self::assertCount(1, $consumer->sleepCalls); + self::assertSame(500 * 1000, $consumer->sleepCalls[0]); // 500ms in µs + + self::assertSame(1, $consumer->retryDecrements); + } + + /** + * 429 + Retry-After: 3 → sleep 3000ms, does NOT decrement retriesRemaining + */ + public function testRetryAfterOn429DoesNotDecrementRetries(): void + { + $consumer = new MockLibCurl('test-secret', ['retry_count' => 3]); + + $consumer->responses = [ + [429, ['retry-after' => '3'], 'Too Many Requests', ''], + [200, [], '{"success":true}', ''], + ]; + + $result = $consumer->flushBatch(makeTestMessages()); + + self::assertTrue($result); + + self::assertCount(1, $consumer->sleepCalls); + self::assertSame(3000 * 1000, $consumer->sleepCalls[0]); // 3000ms in µs + + // retriesRemaining must NOT have been decremented + self::assertSame(0, $consumer->retryDecrements); + } + + /** + * 429 + Retry-After: 3 → budget exhausted after retry_count retries on other codes. + * Re-verify: if retry_count is 1 and we get a 503 (no Retry-After), we fail immediately. + */ + public function testNon429ExhaustsRetryBudget(): void + { + $consumer = new MockLibCurl('test-secret', ['retry_count' => 1]); + + $consumer->responses = [ + [503, [], 'Service Unavailable', ''], + // retry_count=1 means retriesRemaining starts at 1, after one decrement it's 0 → return false + ]; + + $result = $consumer->flushBatch(makeTestMessages()); + + self::assertFalse($result); + self::assertSame(1, $consumer->retryDecrements); + } + + // ------------------------------------------------------------------------- + // parseRetryAfter HTTP-date tests + // ------------------------------------------------------------------------- + + /** + * parseRetryAfter with a future HTTP-date returns a positive integer. + */ + public function testParseRetryAfterHttpDateFuture(): void + { + $consumer = new MockLibCurl('test-secret', []); + $result = $consumer->publicParseRetryAfter('Wed, 21 Oct 2099 07:28:00 GMT'); + + self::assertIsInt($result); + self::assertGreaterThan(0, $result); + } + + /** + * parseRetryAfter with a past HTTP-date returns null. + */ + public function testParseRetryAfterHttpDatePast(): void + { + $consumer = new MockLibCurl('test-secret', []); + $result = $consumer->publicParseRetryAfter('Wed, 21 Oct 2015 07:28:00 GMT'); + + self::assertNull($result); + } + + /** + * parseRetryAfter with garbage string returns null. + */ + public function testParseRetryAfterGarbageReturnsNull(): void + { + $consumer = new MockLibCurl('test-secret', []); + $result = $consumer->publicParseRetryAfter('garbage'); + + self::assertNull($result); + } + + /** + * Retry-After cap is respected: if header says 600s and cap is 300s → sleep 300s. + */ + public function testRetryAfterCapIsRespected(): void + { + $consumer = new MockLibCurl('test-secret', [ + 'retry_count' => 3, + 'rate_limit_retry_after_cap_s' => 300, + ]); + + $consumer->responses = [ + [503, ['retry-after' => '600'], 'Service Unavailable', ''], + [200, [], '{"success":true}', ''], + ]; + + $result = $consumer->flushBatch(makeTestMessages()); + + self::assertTrue($result); + + // Sleep should be capped at 300s = 300000ms = 300000000 µs + self::assertCount(1, $consumer->sleepCalls); + self::assertSame(300000 * 1000, $consumer->sleepCalls[0]); + } } From 57d3cb20bf4f3261a2dadeb49374f8f320936444 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Thu, 3 Sep 2026 17:19:38 -0400 Subject: [PATCH 05/11] Reject malformed Retry-After, and test the shipped retry loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit parseRetryAfter fell back to strtotime(), which is far more permissive than RFC 7231. It read "-1" as a timezone offset and returned 3600, "-5" as 18000, "Wed" as next Wednesday, "tomorrow" as a date and "@99999999999" as an epoch — despite the docblock promising null for unparseable, zero or negative values. The damage was the routing, not the number: any positive result takes the rate-limit branch, which deliberately never decrements retriesRemaining. A single upstream proxy emitting 503 with Retry-After: -1 therefore turned a bounded four-minute counted backoff into a spin capped only by max_rate_limit_duration_ms, 12 hours. php was alone in this — ruby returns nil, go returns 0, java null, C# requires a positive value. Dates are now parsed with DateTimeImmutable::createFromFormat against the three formats RFC 7231 permits, rejecting anything with warnings or errors. All three formats still parse; every malformed value above now returns null. The retry tests did not execute the shipped code. MockLibCurl declared its own flushBatch — a full copy of LibCurl's, with no parent:: call — so all six retry tests asserted against the duplicate and would have passed with LibCurl::flushBatch emptied. Its docblock said as much: "the parent calls the global usleep() which we cannot stub". LibCurl now has a sleepBeforeRetry() seam alongside executeHttpRequest(), and the mock overrides only that, so the tests drive the real loop. retryDecrements is renamed backoffSleeps because that is what it observes; testNon429ExhaustsRetryBudget now asserts the batch is abandoned without waiting, which is what retry_count = 1 actually does. Socket also never sent X-Retry-Count: flushBatch built the request once with the attempt hardcoded to 1, and the retry loop resent that same buffer while incrementing an $attempt nobody read and running json_decode() over a raw HTTP request, discarding the null. Its docblock claimed the header was supported. makeRequest now takes the payload and rebuilds the request per attempt, and the dead decode is gone. All LibCurl tests and 58 e2e tests pass. ConsumerSocketTest::testShortTimeout and ConsumerFileTest::testSend fail identically on the unmodified branch. --- lib/Consumer/LibCurl.php | 16 ++++- lib/Consumer/QueueConsumer.php | 27 ++++++-- lib/Consumer/Socket.php | 18 +++-- test/ConsumerLibCurlTest.php | 119 +++++---------------------------- 4 files changed, 65 insertions(+), 115 deletions(-) diff --git a/lib/Consumer/LibCurl.php b/lib/Consumer/LibCurl.php index 516f917..7b9f4ed 100644 --- a/lib/Consumer/LibCurl.php +++ b/lib/Consumer/LibCurl.php @@ -83,7 +83,7 @@ public function flushBatch(array $messages): bool return false; } $sleepMs = min($retryAfterS * 1000, $this->rate_limit_retry_after_cap_s * 1000); - usleep($sleepMs * 1000); + $this->sleepBeforeRetry($sleepMs, true); continue; // Do NOT decrement retriesRemaining } @@ -98,7 +98,7 @@ public function flushBatch(array $messages): bool if ((microtime(true) - $backoffStartTime) * 1000 >= $this->max_total_backoff_duration_ms) { return false; } - usleep($backoffMs * 1000); + $this->sleepBeforeRetry($backoffMs, false); $backoffMs = min($backoffMs * 2, $backoffCapMs); } } @@ -115,6 +115,18 @@ public function flushBatch(array $messages): bool * @param array $headers * @return array{int, array, string|false, string} */ + /** + * Wait before the next attempt. Split out from flushBatch so tests can observe + * the schedule without re-implementing the retry loop. + * + * @param int $milliseconds how long to wait + * @param bool $rateLimited true when the server sent Retry-After, false for counted backoff + */ + protected function sleepBeforeRetry(int $milliseconds, bool $rateLimited): void + { + usleep($milliseconds * 1000); + } + protected function executeHttpRequest(string $url, string $secret, string $payload, array $headers): array { $responseHeaders = []; diff --git a/lib/Consumer/QueueConsumer.php b/lib/Consumer/QueueConsumer.php index 2a31b26..76381e3 100644 --- a/lib/Consumer/QueueConsumer.php +++ b/lib/Consumer/QueueConsumer.php @@ -160,6 +160,13 @@ protected function isRetryable(int $statusCode): bool return in_array($statusCode, [408, 410, 429, 460], true); } + /** The three date formats RFC 7231 permits for Retry-After. */ + private const HTTP_DATE_FORMATS = [ + 'D, d M Y H:i:s \G\M\T', // IMF-fixdate + 'l, d-M-y H:i:s \G\M\T', // obsolete RFC 850 + 'D M j H:i:s Y', // obsolete asctime + ]; + /** * Parse Retry-After header as integer seconds. * Supports both integer seconds and HTTP-date format (RFC 7231). @@ -179,10 +186,22 @@ protected function parseRetryAfter(?string $value): ?int return $seconds > 0 ? $seconds : null; } - // Try HTTP-date format (RFC 7231) - $timestamp = strtotime($value); - if ($timestamp !== false) { - $seconds = $timestamp - time(); + // Try HTTP-date format (RFC 7231 section 7.1.1.1). strtotime() is far more + // permissive than the spec: it reads "-1" as a timezone offset (3600), + // "Wed" as next Wednesday and "tomorrow" as a date, any of which would send + // a malformed header down the rate-limit path, which spends no retry budget. + foreach (self::HTTP_DATE_FORMATS as $format) { + $date = \DateTimeImmutable::createFromFormat($format, $value, new \DateTimeZone('UTC')); + if ($date === false) { + continue; + } + + $errors = \DateTimeImmutable::getLastErrors(); + if (!empty($errors['warning_count']) || !empty($errors['error_count'])) { + continue; + } + + $seconds = $date->getTimestamp() - time(); return $seconds > 0 ? $seconds : null; } diff --git a/lib/Consumer/Socket.php b/lib/Consumer/Socket.php index 299eb9f..19054dc 100644 --- a/lib/Consumer/Socket.php +++ b/lib/Consumer/Socket.php @@ -51,7 +51,7 @@ public function flushBatch($batch): bool return false; } - return $this->makeRequest($socket, $body); + return $this->makeRequest($socket, $body, $payload); } /** @@ -152,10 +152,11 @@ private function createBody(string $host, string $content, int $attempt = 1) * For full Retry-After support, use the default LibCurl consumer. * * @param resource|false $socket the handle for the socket - * @param string $req request body + * @param string $req request body for this attempt + * @param string $payload encoded batch, re-used to rebuild the request on retries * @return bool */ - private function makeRequest($socket, string $req): bool + private function makeRequest($socket, string $req, string $payload): bool { $bytes_written = 0; $bytes_total = strlen($req); @@ -213,9 +214,14 @@ private function makeRequest($socket, string $req): bool return false; } - // Rebuild request with updated X-Retry-Count - $content_json = json_decode($req, true); - // Re-create body with new attempt count (reuse original payload via flushBatch flow) + // Rebuild the request so X-Retry-Count reflects this attempt. Previously + // the original buffer was resent unchanged, so the header was never sent. + $rebuilt = $this->createBody($this->options['host'], $payload, $attempt); + if ($rebuilt === false) { + return false; + } + $req = $rebuilt; + $bytes_written = 0; $bytes_total = strlen($req); $closed = false; diff --git a/test/ConsumerLibCurlTest.php b/test/ConsumerLibCurlTest.php index 28657d6..9e85c50 100644 --- a/test/ConsumerLibCurlTest.php +++ b/test/ConsumerLibCurlTest.php @@ -24,15 +24,12 @@ class MockLibCurl extends LibCurl /** @var int[] microseconds recorded from each usleep call */ public array $sleepCalls = []; - /** @var int how many times retriesRemaining was decremented */ - public int $retryDecrements = 0; - - private int $initialRetryCount; + /** @var int how many counted-backoff waits were performed */ + public int $backoffSleeps = 0; public function __construct(string $secret, array $options = []) { parent::__construct($secret, $options); - $this->initialRetryCount = $this->retry_count; } protected function executeHttpRequest(string $url, string $secret, string $payload, array $headers): array @@ -45,104 +42,17 @@ protected function executeHttpRequest(string $url, string $secret, string $paylo } /** - * Override flushBatch to track retry decrements and intercept usleep. - * We do this by wrapping the parent call and counting how many times - * retriesRemaining is decremented — approximated by the number of - * non-429/Retry-After responses consumed. - * - * Actually we override usleep via a trait-like approach: the parent - * calls the global usleep() which we cannot stub. Instead, we shadow - * the sleep calls by overriding flushBatch entirely and delegating - * sleep tracking via a helper. - * - * @param array $messages - * @return bool + * Record the retry schedule instead of sleeping. This overrides only the wait, + * so the tests exercise the real LibCurl::flushBatch rather than a copy of it. */ - public function flushBatch(array $messages): bool + protected function sleepBeforeRetry(int $milliseconds, bool $rateLimited): void { - // Reset tracking - $this->sleepCalls = []; - $this->retryDecrements = 0; - - $body = $this->payload($messages); - $payload = json_encode($body); - $secret = $this->secret; - - $host = $this->host ?: 'api.segment.io'; - $url = $this->protocol . $host . '/v1/batch'; - - $library = $messages[0]['context']['library']; - $userAgent = $library['name'] . '/' . $library['version']; - - $backoffMs = 500; - $backoffCapMs = 60000; - $retriesRemaining = $this->retry_count; - $attempt = 0; - $backoffStartTime = null; - $rateLimitStartTime = null; - - while (true) { - $attempt++; - - $headers = [ - 'Content-Type: application/json', - 'User-Agent: ' . $userAgent, - ]; - - if ($attempt > 1) { - $headers[] = 'X-Retry-Count: ' . ($attempt - 1); - } - - [$responseCode, $responseHeaders, $responseContent, $err] = - $this->executeHttpRequest($url, $secret, $payload, $headers); - - if ($err) { - $this->handleError(0, $err); - return false; - } - - if ($responseCode >= 200 && $responseCode < 400) { - return true; - } - - $this->handleError($responseCode, $responseContent); - - if (!$this->isRetryable($responseCode)) { - return false; - } - - // Any retryable status with valid Retry-After: use rate-limit path (no budget cost) - $retryAfterS = $this->parseRetryAfter($responseHeaders['retry-after'] ?? null); - if ($retryAfterS !== null) { - if ($rateLimitStartTime === null) { - $rateLimitStartTime = microtime(true); - } - if ((microtime(true) - $rateLimitStartTime) * 1000 >= $this->max_rate_limit_duration_ms) { - return false; - } - $sleepMs = min($retryAfterS * 1000, $this->rate_limit_retry_after_cap_s * 1000); - $this->sleepCalls[] = $sleepMs * 1000; - continue; // Do NOT decrement retriesRemaining - } - - // No Retry-After: counted backoff - $retriesRemaining--; - $this->retryDecrements++; - if ($retriesRemaining <= 0) { - return false; - } - if ($backoffStartTime === null) { - $backoffStartTime = microtime(true); - } - if ((microtime(true) - $backoffStartTime) * 1000 >= $this->max_total_backoff_duration_ms) { - return false; - } - $this->sleepCalls[] = $backoffMs * 1000; - $backoffMs = min($backoffMs * 2, $backoffCapMs); + $this->sleepCalls[] = $milliseconds * 1000; + if (!$rateLimited) { + $this->backoffSleeps++; } } - // Expose protected methods for direct unit testing public function publicParseRetryAfter(?string $value): ?int { return $this->parseRetryAfter($value); @@ -313,7 +223,7 @@ public function testRetryAfterOnNon429UsesHeaderSleepAndDoesNotDecrementRetries( self::assertSame(2000 * 1000, $consumer->sleepCalls[0]); // 2000ms in µs // retriesRemaining must NOT have been decremented (rate-limit path) - self::assertSame(0, $consumer->retryDecrements); + self::assertSame(0, $consumer->backoffSleeps); } /** @@ -336,7 +246,7 @@ public function testRetryAfterOn529UsesHeaderSleepAndDoesNotDecrementRetries(): self::assertSame(1000 * 1000, $consumer->sleepCalls[0]); // 1000ms in µs // retriesRemaining must NOT have been decremented (rate-limit path) - self::assertSame(0, $consumer->retryDecrements); + self::assertSame(0, $consumer->backoffSleeps); } /** @@ -359,7 +269,7 @@ public function testNon429WithoutRetryAfterUsesExponentialBackoff(): void self::assertCount(1, $consumer->sleepCalls); self::assertSame(500 * 1000, $consumer->sleepCalls[0]); // 500ms in µs - self::assertSame(1, $consumer->retryDecrements); + self::assertSame(1, $consumer->backoffSleeps); } /** @@ -382,7 +292,7 @@ public function testRetryAfterOn429DoesNotDecrementRetries(): void self::assertSame(3000 * 1000, $consumer->sleepCalls[0]); // 3000ms in µs // retriesRemaining must NOT have been decremented - self::assertSame(0, $consumer->retryDecrements); + self::assertSame(0, $consumer->backoffSleeps); } /** @@ -401,7 +311,10 @@ public function testNon429ExhaustsRetryBudget(): void $result = $consumer->flushBatch(makeTestMessages()); self::assertFalse($result); - self::assertSame(1, $consumer->retryDecrements); + // retry_count = 1, so the single decrement exhausts the budget and the + // batch is abandoned without ever waiting. + self::assertSame(0, $consumer->backoffSleeps); + self::assertCount(0, $consumer->sleepCalls); } // ------------------------------------------------------------------------- From c938f29da257f066166aadfdd054803c30640ed6 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Fri, 11 Sep 2026 11:11:14 -0400 Subject: [PATCH 06/11] Tighten retry comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cut the before/after narration from the comments added with the Retry-After work. parseRetryAfter keeps the live hazard — strtotime() reads "-1" as a timezone offset — without cataloguing every malformed value it used to accept. The Socket comment states that the request buffer is per-attempt instead of describing the old resend. --- lib/Consumer/LibCurl.php | 4 ++-- lib/Consumer/QueueConsumer.php | 8 ++++---- lib/Consumer/Socket.php | 3 +-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/Consumer/LibCurl.php b/lib/Consumer/LibCurl.php index 7b9f4ed..6ae2bd2 100644 --- a/lib/Consumer/LibCurl.php +++ b/lib/Consumer/LibCurl.php @@ -116,8 +116,8 @@ public function flushBatch(array $messages): bool * @return array{int, array, string|false, string} */ /** - * Wait before the next attempt. Split out from flushBatch so tests can observe - * the schedule without re-implementing the retry loop. + * Wait before the next attempt. Separate from flushBatch so tests can observe the + * retry schedule by overriding this alone. * * @param int $milliseconds how long to wait * @param bool $rateLimited true when the server sent Retry-After, false for counted backoff diff --git a/lib/Consumer/QueueConsumer.php b/lib/Consumer/QueueConsumer.php index 76381e3..8969279 100644 --- a/lib/Consumer/QueueConsumer.php +++ b/lib/Consumer/QueueConsumer.php @@ -186,10 +186,10 @@ protected function parseRetryAfter(?string $value): ?int return $seconds > 0 ? $seconds : null; } - // Try HTTP-date format (RFC 7231 section 7.1.1.1). strtotime() is far more - // permissive than the spec: it reads "-1" as a timezone offset (3600), - // "Wed" as next Wednesday and "tomorrow" as a date, any of which would send - // a malformed header down the rate-limit path, which spends no retry budget. + // Try HTTP-date (RFC 7231 section 7.1.1.1). Parsed strictly rather than with + // strtotime(), which reads "-1" as a timezone offset and "tomorrow" as a date. + // A malformed header must not reach the rate-limit path, which spends no + // retry budget. foreach (self::HTTP_DATE_FORMATS as $format) { $date = \DateTimeImmutable::createFromFormat($format, $value, new \DateTimeZone('UTC')); if ($date === false) { diff --git a/lib/Consumer/Socket.php b/lib/Consumer/Socket.php index 19054dc..b3e37a3 100644 --- a/lib/Consumer/Socket.php +++ b/lib/Consumer/Socket.php @@ -214,8 +214,7 @@ private function makeRequest($socket, string $req, string $payload): bool return false; } - // Rebuild the request so X-Retry-Count reflects this attempt. Previously - // the original buffer was resent unchanged, so the header was never sent. + // The request buffer is per-attempt: rebuild it so X-Retry-Count is correct. $rebuilt = $this->createBody($this->options['host'], $payload, $attempt); if ($rebuilt === false) { return false; From d6f73059502b486c4dde989031526eb4eb55b94f Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Fri, 11 Sep 2026 16:50:04 -0400 Subject: [PATCH 07/11] Satisfy phpcs so the coding-standard job passes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit phpcs is clean on master and reported six errors on this branch. That matters more than it looks: the cs2pr step which surfaces them is not continue-on-error, so the coding-standard job fails, and the test job declares needs: [coding-standard, lint] — the entire PHP test matrix never runs. Five were column-aligned curl_setopt arguments in executeHttpRequest, where PSR-12 allows a single space after a comma; phpcbf fixed those. The sixth was PSR1.Classes.ClassDeclaration.MultipleClasses: MockLibCurl was declared alongside ConsumerLibCurlTest in one file, and phpcs covers ./test/ as well as ./lib/. MockLibCurl now lives in test/MockLibCurl.php, which autoloads through the existing Segment\Test\ PSR-4 dev mapping, and the now-unused LibCurl import is gone from the test file. phpcs exits 0 with an empty checkstyle report. composer lint passes. phpunit is 75 tests with the two failures that master has too — ConsumerSocketTest ::testShortTimeout and ConsumerFileTest::testSend — and all of ConsumerLibCurlTest passes. All 58 e2e tests pass. --- lib/Consumer/LibCurl.php | 10 +++--- test/ConsumerLibCurlTest.php | 56 -------------------------------- test/MockLibCurl.php | 62 ++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 61 deletions(-) create mode 100644 test/MockLibCurl.php diff --git a/lib/Consumer/LibCurl.php b/lib/Consumer/LibCurl.php index 6ae2bd2..8c1bd97 100644 --- a/lib/Consumer/LibCurl.php +++ b/lib/Consumer/LibCurl.php @@ -133,12 +133,12 @@ protected function executeHttpRequest(string $url, string $secret, string $paylo $ch = curl_init(); - curl_setopt($ch, CURLOPT_USERPWD, $secret . ':'); - curl_setopt($ch, CURLOPT_POSTFIELDS, $payload); - curl_setopt($ch, CURLOPT_TIMEOUT, $this->curl_timeout); + curl_setopt($ch, CURLOPT_USERPWD, $secret . ':'); + curl_setopt($ch, CURLOPT_POSTFIELDS, $payload); + curl_setopt($ch, CURLOPT_TIMEOUT, $this->curl_timeout); curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, $this->curl_connecttimeout); - curl_setopt($ch, CURLOPT_HTTPHEADER, $headers); - curl_setopt($ch, CURLOPT_URL, $url); + curl_setopt($ch, CURLOPT_HTTPHEADER, $headers); + curl_setopt($ch, CURLOPT_URL, $url); curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); curl_setopt($ch, CURLOPT_HEADERFUNCTION, function ($curl, $header) use (&$responseHeaders) { $parts = explode(':', $header, 2); diff --git a/test/ConsumerLibCurlTest.php b/test/ConsumerLibCurlTest.php index 9e85c50..978185a 100644 --- a/test/ConsumerLibCurlTest.php +++ b/test/ConsumerLibCurlTest.php @@ -7,62 +7,6 @@ use PHPUnit\Framework\TestCase; use RuntimeException; use Segment\Client; -use Segment\Consumer\LibCurl; - -/** - * Testable subclass of LibCurl that intercepts HTTP calls and sleep. - * - * Inject a queue of responses via $responses. Each entry: - * [statusCode, headers (assoc, lower-cased), body, curlError] - * When the queue is exhausted, returns a 200 success. - */ -class MockLibCurl extends LibCurl -{ - /** @var array, string, string}> */ - public array $responses = []; - - /** @var int[] microseconds recorded from each usleep call */ - public array $sleepCalls = []; - - /** @var int how many counted-backoff waits were performed */ - public int $backoffSleeps = 0; - - public function __construct(string $secret, array $options = []) - { - parent::__construct($secret, $options); - } - - protected function executeHttpRequest(string $url, string $secret, string $payload, array $headers): array - { - if (empty($this->responses)) { - return [200, [], '{"success":true}', '']; - } - - return array_shift($this->responses); - } - - /** - * Record the retry schedule instead of sleeping. This overrides only the wait, - * so the tests exercise the real LibCurl::flushBatch rather than a copy of it. - */ - protected function sleepBeforeRetry(int $milliseconds, bool $rateLimited): void - { - $this->sleepCalls[] = $milliseconds * 1000; - if (!$rateLimited) { - $this->backoffSleeps++; - } - } - - public function publicParseRetryAfter(?string $value): ?int - { - return $this->parseRetryAfter($value); - } - - public function publicIsRetryable(int $code): bool - { - return $this->isRetryable($code); - } -} /** Minimal message fixture for flushBatch calls */ function makeTestMessages(): array diff --git a/test/MockLibCurl.php b/test/MockLibCurl.php new file mode 100644 index 0000000..4af77ce --- /dev/null +++ b/test/MockLibCurl.php @@ -0,0 +1,62 @@ +, string, string}> */ + public array $responses = []; + + /** @var int[] microseconds recorded from each usleep call */ + public array $sleepCalls = []; + + /** @var int how many counted-backoff waits were performed */ + public int $backoffSleeps = 0; + + public function __construct(string $secret, array $options = []) + { + parent::__construct($secret, $options); + } + + protected function executeHttpRequest(string $url, string $secret, string $payload, array $headers): array + { + if (empty($this->responses)) { + return [200, [], '{"success":true}', '']; + } + + return array_shift($this->responses); + } + + /** + * Record the retry schedule instead of sleeping. This overrides only the wait, + * so the tests exercise the real LibCurl::flushBatch rather than a copy of it. + */ + protected function sleepBeforeRetry(int $milliseconds, bool $rateLimited): void + { + $this->sleepCalls[] = $milliseconds * 1000; + if (!$rateLimited) { + $this->backoffSleeps++; + } + } + + public function publicParseRetryAfter(?string $value): ?int + { + return $this->parseRetryAfter($value); + } + + public function publicIsRetryable(int $code): bool + { + return $this->isRetryable($code); + } +} From d4a297a9912c090bd1911285e249b2a12bb7d530 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Thu, 17 Sep 2026 15:22:03 -0400 Subject: [PATCH 08/11] Opt in to the e2e Authorization check The header assertion in sdk-e2e-tests is opt-in per SDK, since analytics-kotlin and analytics-swift do not send it yet. This SDK does, so it runs the check. --- e2e-cli/e2e-config.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/e2e-cli/e2e-config.json b/e2e-cli/e2e-config.json index cf3ee4d..c11da2a 100644 --- a/e2e-cli/e2e-config.json +++ b/e2e-cli/e2e-config.json @@ -3,5 +3,7 @@ "test_suites": "basic,retry", "auto_settings": false, "patch": null, - "env": {} + "env": { + "AUTH_HEADER": "true" + } } From 8d6d53b2526cd2811a3fc6e22eaab063e8dad9cc Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Tue, 22 Sep 2026 12:04:19 -0400 Subject: [PATCH 09/11] Treat only 2xx as a successful upload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every one of these SDKs treated a 3xx as a failure before this work, and the change to 200-399 came from the design doc's "Spec item 1: 2xx and 3xx are success". That line is wrong, and the doc is what needs correcting. Measured against a local server, with the same HTTP clients these SDKs use: 307/308 + Location -> followed as POST with the body, arrives as 200 301/302/303 + Loc. -> followed as GET with no body, arrives as 200 302 without Location-> surfaces raw as 302 300 Multiple Choices-> surfaces raw as 300 304 Not Modified -> surfaces raw as 304 So a raw 3xx only reaches the classifier when the client has already declined to follow it, meaning nothing was uploaded. The one redirect that genuinely works, 307/308, never produces a 3xx here at all — it produces 200 — so narrowing the bound cannot break it. Nothing was gained by the wider range; a 300, 304, or Location-less 302 from a proxy was being logged as a delivered batch and dropped with no error callback. The narrower bound also needs no new branches: a 3xx is neither 5xx nor in the retryable 4xx set, so it already falls through to the non-retryable path and reports a failure. TAPI does not emit 3xx and has no plans to. This matters because host is customer-configurable and proxies in front of it are common. curl is not configured to follow redirects, so php can also see a raw 3xx; it now reports a named redirect error instead of an empty body. Socket narrowed to match. --- lib/Consumer/LibCurl.php | 16 ++++++++++++++-- lib/Consumer/Socket.php | 5 +++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/Consumer/LibCurl.php b/lib/Consumer/LibCurl.php index 8c1bd97..9213c70 100644 --- a/lib/Consumer/LibCurl.php +++ b/lib/Consumer/LibCurl.php @@ -62,11 +62,23 @@ public function flushBatch(array $messages): bool return false; } - // 2xx and 3xx are success - if ($responseCode >= 200 && $responseCode < 400) { + // Only 2xx is success. curl is not configured to follow redirects, so a + // 3xx means nothing was uploaded; treating it as success would drop the + // batch silently. TAPI does not emit 3xx — this shows up when the + // configured host is a proxy or redirector. + if ($responseCode >= 200 && $responseCode < 300) { return true; } + if ($responseCode >= 300 && $responseCode < 400) { + $this->handleError( + $responseCode, + 'Unexpected redirect; batch not uploaded. Check whether the configured ' + . 'host points at a proxy or redirector.' + ); + return false; + } + $this->handleError($responseCode, $responseContent); if (!$this->isRetryable($responseCode)) { diff --git a/lib/Consumer/Socket.php b/lib/Consumer/Socket.php index b3e37a3..5afb08a 100644 --- a/lib/Consumer/Socket.php +++ b/lib/Consumer/Socket.php @@ -187,8 +187,9 @@ private function makeRequest($socket, string $req, string $payload): bool } fclose($socket); - // 2xx and 3xx are success - if ($statusCode >= 200 && $statusCode < 400) { + // Only 2xx is success; a raw socket never follows redirects, so a 3xx + // means nothing was uploaded. + if ($statusCode >= 200 && $statusCode < 300) { return true; } From 4ed18e3e3717c0e87a4e3652e02ccb7633f2d084 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Tue, 22 Sep 2026 18:44:34 -0400 Subject: [PATCH 10/11] Stop an oversized batch wedging the queue, and use a monotonic clock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Master spliced the batch out of the queue before checking its size, so an oversized batch left the queue on the way to returning false. This branch checked the size against a non-destructive array_slice and spliced only once the check passed, which meant an oversized batch stayed put: every later flush took the same batch, failed the same check, and track() returned false for the rest of the process. The splice happens first again. Reproducing it needs more than one big event — enqueue() rejects any single item over 32KB before it reaches the queue — so the regression test builds a batch from twenty 30KB items, which together pass the 500KB batch limit. With the old order that test reports "queue is wedged"; with the splice restored it passes. The rate-limit and backoff duration budgets used microtime(), so a clock adjustment could expire or extend them. Both now use hrtime(), comparing nanoseconds as milliseconds. The DateTimeImmutable::getLastErrors() check was reported as broken on PHP 8.2 and up. It is not: getLastErrors() returns false when the parse was clean and an array when it was not, so the check still rejects values createFromFormat accepts with warnings. Removing it let "Wed, 32 Oct 2099" through as a November date, so it stays, with a comment recording why. phpcs is clean and all 61 e2e tests pass. ConsumerSocketTest::testShortTimeout and ConsumerFileTest::testSend fail identically on master. --- lib/Consumer/LibCurl.php | 10 +++++---- lib/Consumer/QueueConsumer.php | 13 ++++++----- test/ConsumerLibCurlTest.php | 40 ++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/lib/Consumer/LibCurl.php b/lib/Consumer/LibCurl.php index 9213c70..803f73a 100644 --- a/lib/Consumer/LibCurl.php +++ b/lib/Consumer/LibCurl.php @@ -89,9 +89,11 @@ public function flushBatch(array $messages): bool $retryAfterS = $this->parseRetryAfter($responseHeaders['retry-after'] ?? null); if ($retryAfterS !== null) { if ($rateLimitStartTime === null) { - $rateLimitStartTime = microtime(true); + // hrtime is monotonic; microtime would let a clock adjustment + // expire or extend this budget. + $rateLimitStartTime = hrtime(true); } - if ((microtime(true) - $rateLimitStartTime) * 1000 >= $this->max_rate_limit_duration_ms) { + if ((hrtime(true) - $rateLimitStartTime) / 1e6 >= $this->max_rate_limit_duration_ms) { return false; } $sleepMs = min($retryAfterS * 1000, $this->rate_limit_retry_after_cap_s * 1000); @@ -105,9 +107,9 @@ public function flushBatch(array $messages): bool return false; } if ($backoffStartTime === null) { - $backoffStartTime = microtime(true); + $backoffStartTime = hrtime(true); } - if ((microtime(true) - $backoffStartTime) * 1000 >= $this->max_total_backoff_duration_ms) { + if ((hrtime(true) - $backoffStartTime) / 1e6 >= $this->max_total_backoff_duration_ms) { return false; } $this->sleepBeforeRetry($backoffMs, false); diff --git a/lib/Consumer/QueueConsumer.php b/lib/Consumer/QueueConsumer.php index 8969279..86633fb 100644 --- a/lib/Consumer/QueueConsumer.php +++ b/lib/Consumer/QueueConsumer.php @@ -121,8 +121,11 @@ public function flush(): bool $success = true; while ($count > 0 && $success) { - $batchSize = min($this->flush_at, $count); - $batch = array_slice($this->queue, 0, $batchSize); + // Remove the batch before doing anything else. Leaving it in place on the + // oversize bail below would wedge the queue: every later flush would take + // the same batch, fail the same check, and track() would return false + // forever. + $batch = array_splice($this->queue, 0, min($this->flush_at, $count)); if (mb_strlen(serialize($batch), '8bit') >= $this->max_batch_size_bytes) { $msg = 'Batch size is larger than 500KB'; @@ -131,9 +134,6 @@ public function flush(): bool return false; } - // Remove batch before sending — flushBatch() handles all retries internally - array_splice($this->queue, 0, $batchSize); - $success = $this->flushBatch($batch); $count = count($this->queue); @@ -196,6 +196,9 @@ protected function parseRetryAfter(?string $value): ?int continue; } + // getLastErrors() returns false when the parse was clean and an array + // when it was not, so this rejects values createFromFormat accepts with + // warnings — "Wed, 32 Oct 2099" rolling over into November, for instance. $errors = \DateTimeImmutable::getLastErrors(); if (!empty($errors['warning_count']) || !empty($errors['error_count'])) { continue; diff --git a/test/ConsumerLibCurlTest.php b/test/ConsumerLibCurlTest.php index 978185a..13f00db 100644 --- a/test/ConsumerLibCurlTest.php +++ b/test/ConsumerLibCurlTest.php @@ -302,6 +302,46 @@ public function testParseRetryAfterGarbageReturnsNull(): void /** * Retry-After cap is respected: if header says 600s and cap is 300s → sleep 300s. */ + public function testOversizedBatchDoesNotWedgeTheQueue(): void + { + // A single item over 32KB is rejected by enqueue(), so an oversized *batch* + // is built from many smaller ones: 20 items just under the item limit sum to + // roughly 600KB, past the 500KB batch limit. + $consumer = new MockLibCurl('test-secret', ['flush_at' => 20, 'max_queue_size' => 1000]); + + $chunk = str_repeat('x', 30 * 1024); + $bigMessage = static function (string $payload): array { + return [ + 'type' => 'track', + 'event' => $payload, + 'userId' => 'u1', + 'context' => ['library' => ['name' => 'analytics-php', 'version' => '0.0.0']], + 'timestamp' => date('c'), + ]; + }; + + for ($i = 0; $i < 19; $i++) { + self::assertTrue($consumer->track($bigMessage($chunk))); + } + + // The 20th reaches flush_at, so enqueue() flushes and the batch trips the + // size guard. + self::assertFalse( + $consumer->track($bigMessage($chunk)), + 'the oversized batch should fail this flush' + ); + + // The batch must have left the queue. If it did not, every later flush takes + // it again and track() returns false forever. + $consumer->responses = [[200, [], '{"success":true}', '']]; + $consumer->track($bigMessage('small')); + + self::assertTrue( + $consumer->flush(), + 'queue is wedged: the oversized batch was never removed' + ); + } + public function testRetryAfterCapIsRespected(): void { $consumer = new MockLibCurl('test-secret', [ From dc4c0b1ae9a0b348af05f38e018c2de0ba2a8e13 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Tue, 22 Sep 2026 18:57:13 -0400 Subject: [PATCH 11/11] Add release notes for the HTTP response and retry work Records the retry/Retry-After work and, for the SDKs where a header is newly on the wire, an upgrade note: customers whose proxies allowlist request headers had uploads rejected by the already-released analytics-next change, and the same trap applies here. --- HISTORY.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/HISTORY.md b/HISTORY.md index 1642c3f..2d04dbb 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,3 +1,23 @@ +Unreleased +================== + +### Upgrade note: new request header and proxy allowlists + +This release sends an `X-Retry-Count` request header on retries. If your +traffic to Segment goes through a proxy, gateway or WAF that allowlists +request headers, add it before upgrading or retried uploads will be +rejected. The `Authorization` header is unchanged: this client has always +sent the write key as HTTP Basic credentials. + + * Send `X-Retry-Count` on retries from both the LibCurl and Socket consumers, so the server can distinguish a retry from a first attempt. Omitted on the first attempt. + * Unified retry handling: 429, 408, 410, 460 and 5xx (except 501, 505 and 511) are retried. `Retry-After` is honoured on all of them, not just 429, which brings 529 in through the generic 5xx rule. + * `Retry-After` accepts numeric seconds and the RFC 7231 HTTP-date formats, capped at 300s (`rate_limit_retry_after_cap`). Malformed values are rejected rather than parsed into an arbitrary date. + * Rate-limited retries are bounded by elapsed time rather than counted against the retry limit, so a long `Retry-After` no longer exhausts the budget. + * New options `max_total_backoff_duration` and `max_rate_limit_duration` (default 12 hours each) bound the two waits. + * Only 2xx responses count as a successful upload. A 3xx is now reported as an error rather than silently treated as delivered; the Segment endpoint does not redirect, so this only affects custom `host` values. + * Retry timing uses `hrtime()`, so a system clock change cannot stretch or collapse a backoff. + * Fix an oversized batch wedging the queue: the batch is now removed before the size check, so one too-large batch no longer makes every later `track()` return false. + 3.8.2 / 2026-03-11 ==================