Handle non-numeric Retry-After header values gracefully
Validate the parsed Retry-After value is a finite positive number before using it as a delay, falling back to exponential backoff otherwise. This prevents a near-zero delay tight retry loop when the header contains an HTTP-date or unexpected value. Signed-off-by: Fredrik Adelöw <freben@spotify.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Fredrik Adelöw <freben@spotify.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Fredrik Adelöw <freben@spotify.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -19,6 +19,7 @@ import { rest } from 'msw';
|
||||
import { ConfigReader } from '@backstage/config';
|
||||
import {
|
||||
GitLabIntegration,
|
||||
parseRetryAfterMs,
|
||||
replaceGitLabUrlType,
|
||||
sleep,
|
||||
} from './GitLabIntegration';
|
||||
@@ -477,6 +478,39 @@ describe('sleep', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseRetryAfterMs', () => {
|
||||
it('parses delay-seconds', () => {
|
||||
expect(parseRetryAfterMs('120', 5000)).toBe(120_000);
|
||||
expect(parseRetryAfterMs('1', 5000)).toBe(1000);
|
||||
});
|
||||
|
||||
it('parses an HTTP-date and computes a delta', () => {
|
||||
const futureDate = new Date(Date.now() + 30_000).toUTCString();
|
||||
const result = parseRetryAfterMs(futureDate, 5000);
|
||||
expect(result).toBeGreaterThan(29_000);
|
||||
expect(result).toBeLessThanOrEqual(30_000);
|
||||
});
|
||||
|
||||
it('returns 0 for an HTTP-date in the past', () => {
|
||||
const pastDate = new Date(Date.now() - 10_000).toUTCString();
|
||||
expect(parseRetryAfterMs(pastDate, 5000)).toBe(0);
|
||||
});
|
||||
|
||||
it('returns fallback for null or unparseable values', () => {
|
||||
expect(parseRetryAfterMs(null, 5000)).toBe(5000);
|
||||
expect(parseRetryAfterMs('', 5000)).toBe(5000);
|
||||
expect(parseRetryAfterMs('not-a-date-or-number', 5000)).toBe(5000);
|
||||
});
|
||||
|
||||
it('treats zero seconds as retry-immediately', () => {
|
||||
expect(parseRetryAfterMs('0', 5000)).toBe(0);
|
||||
});
|
||||
|
||||
it('returns fallback for negative seconds', () => {
|
||||
expect(parseRetryAfterMs('-5', 5000)).toBe(5000);
|
||||
});
|
||||
});
|
||||
|
||||
describe('replaceGitLabUrlType', () => {
|
||||
it('should replace with expected type', () => {
|
||||
expect(
|
||||
|
||||
@@ -124,6 +124,7 @@ export class GitLabIntegration implements ScmIntegration {
|
||||
// No more attempts left — propagate the network error.
|
||||
if (attempt++ >= maxRetries) throw e;
|
||||
await sleep(backoffDelay(attempt), abortSignal);
|
||||
if (abortSignal?.aborted) throw e;
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -137,22 +138,51 @@ export class GitLabIntegration implements ScmIntegration {
|
||||
return response;
|
||||
}
|
||||
|
||||
// Determine delay from Retry-After header if present, otherwise exponential backoff
|
||||
const retryAfter = response.headers.get('Retry-After');
|
||||
const delay = retryAfter
|
||||
? parseInt(retryAfter, 10) * 1000
|
||||
: backoffDelay(attempt);
|
||||
// Retry-After is either delay-seconds or an HTTP-date (RFC 9110 §10.2.3).
|
||||
const delay = parseRetryAfterMs(
|
||||
response.headers.get('Retry-After'),
|
||||
backoffDelay(attempt),
|
||||
);
|
||||
|
||||
// Release the underlying connection so it can be reused, since we're
|
||||
// about to discard this response in favor of a retry.
|
||||
await response.body?.cancel().catch(() => {});
|
||||
|
||||
await sleep(delay, abortSignal);
|
||||
if (abortSignal?.aborted) return response;
|
||||
}
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
/** @internal */
|
||||
export function parseRetryAfterMs(
|
||||
headerValue: string | null,
|
||||
fallbackMs: number,
|
||||
): number {
|
||||
if (!headerValue) {
|
||||
return fallbackMs;
|
||||
}
|
||||
|
||||
// delay-seconds per RFC 9110 is 1*DIGIT
|
||||
if (/^\d+$/.test(headerValue)) {
|
||||
return Number(headerValue) * 1000;
|
||||
}
|
||||
|
||||
// HTTP-dates (IMF-fixdate) always contain a comma, e.g.
|
||||
// "Sun, 06 Nov 1994 08:49:37 GMT" — use that as a prerequisite
|
||||
// to avoid Date.parse interpreting random strings as dates.
|
||||
if (headerValue.includes(',')) {
|
||||
const dateMs = Date.parse(headerValue);
|
||||
if (Number.isFinite(dateMs)) {
|
||||
const deltaMs = dateMs - Date.now();
|
||||
return deltaMs > 0 ? deltaMs : 0;
|
||||
}
|
||||
}
|
||||
|
||||
return fallbackMs;
|
||||
}
|
||||
|
||||
/** @internal */
|
||||
export async function sleep(
|
||||
durationMs: number,
|
||||
|
||||
Reference in New Issue
Block a user