diff --git a/src/Transport/RateLimiter.php b/src/Transport/RateLimiter.php index fa6ad31e9..a1bc4be47 100644 --- a/src/Transport/RateLimiter.php +++ b/src/Transport/RateLimiter.php @@ -70,22 +70,29 @@ public function handleResponse(Response $response): bool if ($response->hasHeader(self::RATE_LIMITS_HEADER)) { foreach (explode(',', $response->getHeaderLine(self::RATE_LIMITS_HEADER)) as $limit) { + $limit = trim($limit); + + // Skip empty limits, e.g. caused by a trailing comma + if ($limit === '') { + continue; + } + /** * $parameters[0] - retry_after - * $parameters[1] - categories + * $parameters[1] - categories (if missing or empty, the limit applies to all categories) * $parameters[2] - scope (not used) * $parameters[3] - reason_code (not used) * $parameters[4] - namespaces (only returned if categories contains "metric_bucket"). */ - $parameters = explode(':', trim($limit), 5); + $parameters = explode(':', $limit, 5); - $retryAfter = $now + (ctype_digit($parameters[0]) ? (int) $parameters[0] : self::DEFAULT_RETRY_AFTER_SECONDS); + $retryAfter = $now + $this->parseRateLimitRetryAfter($parameters[0]); - foreach (explode(';', $parameters[1]) as $category) { - $this->rateLimits[$category ?: 'all'] = $retryAfter; + foreach (explode(';', $parameters[1] ?? '') as $category) { + $disabledUntil = $this->updateRateLimit($category ?: 'all', $retryAfter); $this->logger->warning( - \sprintf('Rate limited exceeded for category "%s", backing off until "%s".', $category, gmdate(\DATE_ATOM, $retryAfter)) + \sprintf('Rate limited exceeded for category "%s", backing off until "%s".', $category, gmdate(\DATE_ATOM, $disabledUntil)) ); } } @@ -95,17 +102,20 @@ public function handleResponse(Response $response): bool if ($response->hasHeader(self::RETRY_AFTER_HEADER)) { $retryAfter = $now + $this->parseRetryAfterHeader($now, $response->getHeaderLine(self::RETRY_AFTER_HEADER)); + } elseif ($response->getStatusCode() === 429) { + // Rate limited responses without any rate limit headers back off all categories for the default duration + $retryAfter = $now + self::DEFAULT_RETRY_AFTER_SECONDS; + } else { + return false; + } - $this->rateLimits['all'] = $retryAfter; - - $this->logger->warning( - \sprintf('Rate limited exceeded for all categories, backing off until "%s".', gmdate(\DATE_ATOM, $retryAfter)) - ); + $disabledUntil = $this->updateRateLimit('all', $retryAfter); - return true; - } + $this->logger->warning( + \sprintf('Rate limited exceeded for all categories, backing off until "%s".', gmdate(\DATE_ATOM, $disabledUntil)) + ); - return false; + return true; } /** @@ -148,4 +158,28 @@ private function parseRetryAfterHeader(int $currentTime, string $header): int return self::DEFAULT_RETRY_AFTER_SECONDS; } + + /** + * Stores the rate limit for the given category, keeping the existing one if it + * lasts longer, and returns the time until it is rate limited. + */ + private function updateRateLimit(string $category, int $disabledUntil): int + { + $this->rateLimits[$category] = max($this->rateLimits[$category] ?? 0, $disabledUntil); + + return $this->rateLimits[$category]; + } + + /** + * Parses the number of seconds of a limit in the X-Sentry-Rate-Limits header, + * which can be either an integer or a floating point number. + */ + private function parseRateLimitRetryAfter(string $retryAfter): int + { + if (is_numeric($retryAfter)) { + return (int) ceil(max(0.0, (float) $retryAfter)); + } + + return self::DEFAULT_RETRY_AFTER_SECONDS; + } } diff --git a/tests/Transport/RateLimiterTest.php b/tests/Transport/RateLimiterTest.php index 70285a3e8..436ef1b27 100644 --- a/tests/Transport/RateLimiterTest.php +++ b/tests/Transport/RateLimiterTest.php @@ -83,6 +83,17 @@ public static function handleResponseDataProvider(): \Generator true, EventType::cases(), ]; + + yield 'Back-off on 429 response without rate limit headers should lock them all' => [ + new Response(429, [], ''), + true, + EventType::cases(), + ]; + + yield 'Do not back-off on error response without rate limit headers' => [ + new Response(500, [], ''), + false, + ]; } public function testHandleResponseWithMultipleCommaSpaceSeparatedLimits(): void @@ -124,6 +135,108 @@ public function testIsRateLimited(): void $this->assertEventTypesAreRateLimited([]); } + /** + * @dataProvider getDisabledUntilDataProvider + * + * @param Response[] $responses + */ + public function testGetDisabledUntil(array $responses, EventType $eventType, int $expectedDisabledUntil): void + { + ClockMock::withClockMock(1644105600); + + foreach ($responses as $response) { + $this->rateLimiter->handleResponse($response); + } + + $this->assertSame($expectedDisabledUntil, $this->rateLimiter->getDisabledUntil($eventType)); + } + + public static function getDisabledUntilDataProvider(): \Generator + { + yield 'Keep the longest limit of a category within the same header' => [ + [ + new Response(429, ['X-Sentry-Rate-Limits' => ['2700:default;error;security:organization, 60:error:key']], ''), + ], + EventType::event(), + 1644105600 + 2700, + ]; + + yield 'Keep the longest limit of a category across responses' => [ + [ + new Response(429, ['X-Sentry-Rate-Limits' => ['2700:error:organization']], ''), + new Response(429, ['X-Sentry-Rate-Limits' => ['60:error:key']], ''), + ], + EventType::event(), + 1644105600 + 2700, + ]; + + yield 'Extend the limit of a category if a longer one is received' => [ + [ + new Response(429, ['X-Sentry-Rate-Limits' => ['60:error:key']], ''), + new Response(429, ['X-Sentry-Rate-Limits' => ['2700:error:organization']], ''), + ], + EventType::event(), + 1644105600 + 2700, + ]; + + yield 'Keep the longest limit of all categories across Retry-After headers' => [ + [ + new Response(429, ['Retry-After' => ['2700']], ''), + new Response(429, ['Retry-After' => ['10']], ''), + ], + EventType::event(), + 1644105600 + 2700, + ]; + + yield 'Back-off for the default duration on 429 response without rate limit headers' => [ + [ + new Response(429, [], ''), + ], + EventType::transaction(), + 1644105600 + 60, + ]; + + yield 'Round up floating point retry_after' => [ + [ + new Response(429, ['X-Sentry-Rate-Limits' => ['2700.5:error:organization']], ''), + ], + EventType::event(), + 1644105600 + 2701, + ]; + + yield 'Fall back to the default duration for invalid retry_after' => [ + [ + new Response(429, ['X-Sentry-Rate-Limits' => ['foo:error:organization']], ''), + ], + EventType::event(), + 1644105600 + 60, + ]; + + yield 'Ignore empty limits' => [ + [ + new Response(429, ['X-Sentry-Rate-Limits' => ['60:error:organization,']], ''), + ], + EventType::transaction(), + 0, + ]; + + yield 'Apply limits without categories to all categories' => [ + [ + new Response(429, ['X-Sentry-Rate-Limits' => ['10']], ''), + ], + EventType::transaction(), + 1644105600 + 10, + ]; + + yield 'Apply limits following a limit without categories' => [ + [ + new Response(429, ['X-Sentry-Rate-Limits' => ['60:error:organization, 10, 2700:transaction:key']], ''), + ], + EventType::transaction(), + 1644105600 + 2700, + ]; + } + private function assertEventTypesAreRateLimited(array $eventTypesLimited): void { foreach ($eventTypesLimited as $eventType) {