Skip to content

Commit 0b345a1

Browse files
authored
Honor fractional Retry-After header values (#6928)
RetryAfterHeaderStrategy parsed the Retry-After header with Integer.parseInt, which throws on fractional second values that some HTTP services return (e.g. 299.997). The exception was swallowed and the header ignored, so the client fell back to a short fixed backoff and retried far sooner than the server requested. Under HTTP 429 this collapses one backoff window into a burst of premature retries. Parse the value as a decimal and round up so the client never waits less than requested. Guard against non-finite and excessively large values, which the decimal parser would otherwise accept where the integer parser previously rejected them, to avoid integer overflow in the sleep calculation. Resolves #6927 Signed-off-by: Nikhil Bagmar <nikhilbagmar73@gmail.com>
1 parent 1830be0 commit 0b345a1

2 files changed

Lines changed: 72 additions & 2 deletions

File tree

data-prepper-plugins/saas-source-plugins/source-crawler/src/main/java/org/opensearch/dataprepper/plugins/source/source_crawler/utils/retry/RetryAfterHeaderStrategy.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public class RetryAfterHeaderStrategy implements RetryStrategy {
3131
private static final String RATE_LIMIT_REMAINING = "X-RateLimit-Remaining";
3232
private static final String RATE_LIMIT_RESET = "X-RateLimit-Reset";
3333
private static final String RETRY_AFTER = "Retry-After";
34+
private static final int MAX_RETRY_AFTER_SECONDS = 86400; // ceiling guards against int overflow in the sleep calculation
3435
private static final List<HttpStatus> DEFAULT_RATE_LIMIT_STATUS_CODES = Arrays.asList(HttpStatus.TOO_MANY_REQUESTS);
3536

3637
private final List<Integer> retryAttemptSleepTime;
@@ -127,8 +128,12 @@ private Optional<Integer> extractRetryAfterHeader(Exception ex) {
127128
if (headers != null && headers.containsKey(RETRY_AFTER)) {
128129
String retryAfter = headers.getFirst(RETRY_AFTER);
129130
if (retryAfter != null) {
130-
int seconds = Integer.parseInt(retryAfter);
131-
return Optional.of(Math.max(seconds, 1));
131+
// Some services send a fractional Retry-After (e.g. 299.997); round up so we never wait less than requested.
132+
final double parsedSeconds = Double.parseDouble(retryAfter);
133+
if (Double.isFinite(parsedSeconds)) {
134+
final int seconds = (int) Math.min(Math.ceil(parsedSeconds), MAX_RETRY_AFTER_SECONDS);
135+
return Optional.of(Math.max(seconds, 1));
136+
}
132137
}
133138
}
134139
if (headers != null && headers.containsKey(RATE_LIMIT_REMAINING) && headers.containsKey(RATE_LIMIT_RESET)) {

data-prepper-plugins/saas-source-plugins/source-crawler/src/test/java/org/opensearch/dataprepper/plugins/source/source_crawler/utils/retry/RetryAfterHeaderStrategyTest.java

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,71 @@ void calculateSleepTime_WithLargeRetryAfterHeader_UsesHeaderValue() {
242242
assertThat(sleepTime, equalTo(600000L));
243243
}
244244

245+
@Test
246+
void calculateSleepTime_WithFractionalRetryAfterHeader_RoundsUpToNextSecond() {
247+
final RetryAfterHeaderStrategy retryAfterHeaderStrategy = new RetryAfterHeaderStrategy(1);
248+
final HttpHeaders headers = new HttpHeaders();
249+
headers.set("retry-after", "299.997");
250+
final HttpClientErrorException exception = new HttpClientErrorException(
251+
HttpStatus.TOO_MANY_REQUESTS, "Too Many Requests", headers, null, null);
252+
253+
final long sleepTime = retryAfterHeaderStrategy.calculateSleepTime(exception, 0);
254+
255+
assertThat(sleepTime, equalTo(300000L));
256+
}
257+
258+
@Test
259+
void calculateSleepTime_WithSmallFractionalRetryAfterHeader_RoundsUpToNextSecond() {
260+
final RetryAfterHeaderStrategy retryAfterHeaderStrategy = new RetryAfterHeaderStrategy(1);
261+
final HttpHeaders headers = new HttpHeaders();
262+
headers.set("retry-after", "123.39");
263+
final HttpClientErrorException exception = new HttpClientErrorException(
264+
HttpStatus.TOO_MANY_REQUESTS, "Too Many Requests", headers, null, null);
265+
266+
final long sleepTime = retryAfterHeaderStrategy.calculateSleepTime(exception, 0);
267+
268+
assertThat(sleepTime, equalTo(124000L));
269+
}
270+
271+
@Test
272+
void calculateSleepTime_WithSubSecondRetryAfterHeader_UsesMinimumOneSecond() {
273+
final RetryAfterHeaderStrategy retryAfterHeaderStrategy = new RetryAfterHeaderStrategy(1);
274+
final HttpHeaders headers = new HttpHeaders();
275+
headers.set("retry-after", "0.5");
276+
final HttpClientErrorException exception = new HttpClientErrorException(
277+
HttpStatus.TOO_MANY_REQUESTS, "Too Many Requests", headers, null, null);
278+
279+
final long sleepTime = retryAfterHeaderStrategy.calculateSleepTime(exception, 0);
280+
281+
assertThat(sleepTime, equalTo(1000L));
282+
}
283+
284+
@Test
285+
void calculateSleepTime_WithNonFiniteRetryAfterHeader_FallsBackToDefault() {
286+
final RetryAfterHeaderStrategy retryAfterHeaderStrategy = new RetryAfterHeaderStrategy(1);
287+
final HttpHeaders headers = new HttpHeaders();
288+
headers.set("retry-after", "Infinity");
289+
final HttpClientErrorException exception = new HttpClientErrorException(
290+
HttpStatus.TOO_MANY_REQUESTS, "Too Many Requests", headers, null, null);
291+
292+
final long sleepTime = retryAfterHeaderStrategy.calculateSleepTime(exception, 0);
293+
294+
assertThat(sleepTime, equalTo(5000L));
295+
}
296+
297+
@Test
298+
void calculateSleepTime_WithExcessiveRetryAfterHeader_IsClampedToMaximum() {
299+
final RetryAfterHeaderStrategy retryAfterHeaderStrategy = new RetryAfterHeaderStrategy(1);
300+
final HttpHeaders headers = new HttpHeaders();
301+
headers.set("retry-after", "999999999999");
302+
final HttpClientErrorException exception = new HttpClientErrorException(
303+
HttpStatus.TOO_MANY_REQUESTS, "Too Many Requests", headers, null, null);
304+
305+
final long sleepTime = retryAfterHeaderStrategy.calculateSleepTime(exception, 0);
306+
307+
assertThat(sleepTime, equalTo(86400000L));
308+
}
309+
245310
@Test
246311
void calculateSleepTime_WithEmptyRetryAfterHeader_FallsBackToDefault() {
247312
final RetryAfterHeaderStrategy retryAfterHeaderStrategy = new RetryAfterHeaderStrategy(1);

0 commit comments

Comments
 (0)