Bound the rate-limit retry budget at 5 minutes

HISTORY.md

@@ -1,32 +1,56 @@
 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`, both in seconds and defaulting to 12 hours, bound the two waits.
-  * The Socket consumer now honours `retry_count` and `max_total_backoff_duration` too, and backs off from 500ms like the LibCurl consumer rather than 100ms. It previously ignored both and gave up after a fixed seven retries over roughly 13 seconds. `maximum_backoff_duration` now caps each individual wait rather than ending the loop, so its 10s default still bounds how long any one retry sleeps. `max_rate_limit_duration` does not apply there, since Socket still does not read `Retry-After` — use the LibCurl consumer if you need that.
-  * `retry_count` grants exactly that many retries. It previously granted one fewer, and a `retry_count` of 1 granted none.
-  * The new budget options reject negative values and keep the default, logging as `flush_at` and `flush_interval` already do. A negative previously disabled retrying outright. Zero is still accepted and meaningful: `retry_count` of 0 means do not retry.
-  * Transport failures report the real libcurl error number to `error_handler` again, so a DNS failure, a timeout and a TLS error can be told apart.
-  * Only 2xx responses count as a successful upload. A 3xx is now reported as a failed upload rather than silently treated as delivered. It is not retried: a redirect curl already declined to follow will not succeed on a retry. 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
-==================
-
-
+### Upgrade note: new request header
+
+This release sends an `X-Retry-Count` request header on retries. If traffic to
+Segment passes 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.
+
+### Upgrade note: retries take longer than they used to
+
+Both the LibCurl and Socket consumers previously gave up after roughly 13
+seconds of waiting — seven retries for Socket, six for LibCurl, which checked
+its backoff ceiling before each attempt rather than after. Both now honour `retry_count` (default 10) and
+back off from 500ms rather than 100ms, so a failing upload is retried for
+considerably longer than before.
+
+This matters most for the default LibCurl consumer, which retries inline on the
+calling thread: with the default `retry_count` a persistently failing upload
+spends around four minutes in waits, plus however long each attempt takes,
+before giving up. On a web request that is a worker held for the duration. Lower
+`retry_count` to restore a shorter schedule, or use the [file consumer](https://www.twilio.com/docs/segment/connections/sources/catalog/libraries/server/php#file-consumer),
+which records events without making a network call.
+
+The two consumers cap an individual wait differently: LibCurl caps each wait at
+60 seconds, while Socket caps it at `maximum_backoff_duration`, whose default is
+10 seconds.
+
+### Upgrade note: `track()` return value
+
+`track()` and the other message methods return the result of a flush when the
+queue reaches `flush_at`. Previously the LibCurl consumer reported success for
+any response it had finished with, including a 4xx and a retry-exhausted upload,
+so that flush almost always returned `true`. It now returns `false` when the
+batch was not delivered, and `flush()` stops at the first failing batch rather
+than continuing through the queue. Code branching on the return value of
+`track()` will see failures it did not see before.
+
+### Retry handling
+
+  * Uploads are retried on 408, 410, 429, 460, and 5xx except 501, 505 and 511.
+  * A `Retry-After` header is honoured on any retryable response, not only 429. Numeric seconds and the RFC 7231 HTTP-date formats are both accepted, malformed values are ignored, and the value is capped at `rate_limit_retry_after_cap`.
+  * Responses carrying `Retry-After` are retried for up to `max_rate_limit_duration` and do not consume the retry count. Other failures use exponential backoff limited by `retry_count` and by `max_total_backoff_duration` as an upper bound.
+  * New options, all in seconds: `max_rate_limit_duration` (default 300 — deliberately shorter than in Segment's other server libraries, because the LibCurl consumer retries inline on the calling thread, so this budget is time a web request spends blocked and a PHP-FPM worker spends occupied), `max_total_backoff_duration` (default 43200) and `rate_limit_retry_after_cap` (default 300). Negative values are ignored, logged, and the default kept, as is a `rate_limit_retry_after_cap` of 0 — a cap of zero would clamp every wait to nothing and, since that path does not consume a retry, post continuously for the whole budget. A `retry_count` of 0 does mean do not retry, and `curl_timeout` of 0 still means no limit.
+  * `Retry-After` is not read by the Socket consumer, which uses exponential backoff for every retryable response. Use the default LibCurl consumer if you need it.
+  * Exhausting the rate-limit budget is logged regardless of the `debug` setting.
+  * These budgets bound one batch: it is retried for up to `max_rate_limit_duration` on responses carrying `Retry-After` and, independently, up to `max_total_backoff_duration` on those without, so a response stream that mixes the two spends both — plus however long the request in flight takes, which `curl_timeout` bounds only if it has been set — it defaults to no limit. A single `flush()` sends as many batches as the queue holds, waiting `flush_interval` between them, so it can take considerably longer than any one batch's budget. Applications that cannot block for that long can use the [file consumer](https://www.twilio.com/docs/segment/connections/sources/catalog/libraries/server/php#file-consumer), which records events to a log file with no network call and uploads them out of band.
+
+### Other changes
+
+  * `X-Retry-Count` is sent on retries by the LibCurl and Socket consumers, allowing the server to distinguish a retry from a first attempt. It is omitted on the first attempt.
+  * Only 2xx responses count as a successful upload. A 3xx is reported as a failed upload rather than treated as delivered, and is not retried: a redirect curl has already declined to follow will not succeed on one. The Segment endpoint does not redirect, so this affects only custom `host` values.
 
 3.8.2 / 2026-03-11
 ==================

lib/Consumer/LibCurl.php

@@ -95,11 +95,37 @@ public function flushBatch(array $messages): bool
                     // expire or extend this budget.
                     $rateLimitStartTime = hrtime(true);
                 }
-                if ((hrtime(true) - $rateLimitStartTime) / 1e6 >= $this->max_rate_limit_duration_ms) {
+                $elapsedMs = (hrtime(true) - $rateLimitStartTime) / 1e6;
+                if ($elapsedMs >= $this->max_rate_limit_duration_ms) {
+                    // Logged unconditionally: this consumer blocks the caller, and a
+                    // request that stopped for the whole budget should not have to be
+                    // diagnosed from an absence of output. handleError only writes when
+                    // debug is on, which it is not by default.
+                    error_log(sprintf(
+                        '[Analytics][%s] Rate-limit budget of %dms exhausted; dropping batch',
+                        $this->type,
+                        $this->max_rate_limit_duration_ms
+                    ));
                     return false;
                 }
+                // Capped, then required to fit. Shortening the wait would resume inside
+                // the window the server named -- one it has already said it will not
+                // serve -- and the budget is spent by then, so that attempt would be the
+                // last either way. Compared in floats: an int cast here truncates a
+                // sub-millisecond remainder to 0, which would sleep not at all and
+                // re-POST at round-trip rate without consuming a retry.
+                $remainingMs = $this->max_rate_limit_duration_ms - $elapsedMs;
                 $sleepMs = min($retryAfterS * 1000, $this->rate_limit_retry_after_cap_s * 1000);
-                $this->sleepBeforeRetry($sleepMs, true);
+                if ($sleepMs > $remainingMs) {
+                    error_log(sprintf(
+                        '[Analytics][%s] Retry-After of %ds does not fit the remaining '
+                        . 'rate-limit budget; dropping batch',
+                        $this->type,
+                        (int)($sleepMs / 1000)
+                    ));
+                    return false;
+                }
+                $this->sleepBeforeRetry((int)$sleepMs, true);
                 continue; // Do NOT decrement retriesRemaining
             }
 
@@ -111,10 +137,20 @@ public function flushBatch(array $messages): bool
                 return false;
             }
             $retriesRemaining--;
+            // One reading serves the budget test and the clamp below, as on the
+            // rate-limit path above.
+            $backoffNow = hrtime(true);
             if ($backoffStartTime === null) {
-                $backoffStartTime = hrtime(true);
+                $backoffStartTime = $backoffNow;
+            }
+            $backoffRemainingMs =
+                $this->max_total_backoff_duration_ms - ($backoffNow - $backoffStartTime) / 1e6;
+            if ($backoffRemainingMs <= 0) {
+                return false;
             }
-            if ((hrtime(true) - $backoffStartTime) / 1e6 >= $this->max_total_backoff_duration_ms) {
+            // Same rule as the rate-limit path, for the simpler reason that a backoff
+            // outlasting the budget is a wait whose attempt can never run.
+            if ($backoffMs > $backoffRemainingMs) {
                 return false;
             }
             $this->sleepBeforeRetry($backoffMs, false);

lib/Consumer/QueueConsumer.php

@@ -20,12 +20,27 @@ abstract class QueueConsumer extends Consumer
     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
+
+    /**
+     * Rate-limited attempts are deliberately uncounted, so this duration is the only
+     * thing bounding them. It matters more here than in a client with a background
+     * worker: LibCurl retries inline on the caller's thread, so this is time a web
+     * request spends blocked and an FPM worker spends occupied.
+     */
+    protected int $max_rate_limit_duration_ms = 300000; // 5 minutes
+
+    /**
+     * A guard against an absurd header, not a second budget. Waiting less than the
+     * server asked for does not make the next attempt more likely to succeed, it
+     * just sends more requests at something already rate-limiting us; how long we
+     * keep trying is max_rate_limit_duration's job.
+     */
+    protected int $rate_limit_retry_after_cap_s = 300;
     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
+
     protected int $curl_timeout = 0; // by default this is infinite
     protected int $curl_connecttimeout = 300;
 
@@ -108,8 +123,13 @@ public function __construct(string $secret, array $options = [])
             }
         }
 
+        // Positive, not merely non-negative. A cap of 0 clamps every wait to 0, and
+        // the rate-limit path does not consume a retry, so the client would post
+        // back-to-back at RTT rate for the whole budget against a server that is
+        // already rate-limiting it. 0 is meaningful for retry_count and curl_timeout,
+        // but there is no sensible reading of "cap the wait at nothing".
         if (isset($options['rate_limit_retry_after_cap'])) {
-            if ($this->isNonNegativeInt($options['rate_limit_retry_after_cap'], 'rate_limit_retry_after_cap')) {
+            if ($this->isPositiveInt($options['rate_limit_retry_after_cap'], 'rate_limit_retry_after_cap')) {
                 $this->rate_limit_retry_after_cap_s = (int)$options['rate_limit_retry_after_cap'];
             }
         }
@@ -163,6 +183,25 @@ public function flush(): bool
         return $success;
     }
 
+    /**
+     * Whether an option value is usable where zero is not meaningful.
+     *
+     * Logs and returns false otherwise, so the caller keeps the default.
+     */
+    protected function isPositiveInt($value, string $name): bool
+    {
+        if (!is_numeric($value) || (int)$value < 1) {
+            error_log(sprintf(
+                '[Analytics][%s] %s must be a positive integer; keeping the default',
+                $this->type,
+                $name
+            ));
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * Whether an option value is usable as a count or duration.
      *

lib/Consumer/Socket.php

@@ -149,7 +149,10 @@ private function createBody(string $host, string $content, int $attempt = 1)
      * - X-Retry-Count: Supported.
      * - Backoff: Exponential from 500ms, each wait capped at
      *   maximum_backoff_duration, bounded by retry_count and
-     *   max_total_backoff_duration — the same budgets the LibCurl consumer uses.
+     *   max_total_backoff_duration. retry_count and max_total_backoff_duration are
+     *   shared with LibCurl; the per-wait cap is not. LibCurl caps each wait at a
+     *   fixed 60s and does not read maximum_backoff_duration at all, so the same
+     *   configuration produces different schedules on the two consumers.
      * - max_rate_limit_duration: not applicable, since there is no Retry-After
      *   path here for it to bound.
      *