From d9f3d9b65c6c7bbc74cc3cdfed128b49186b36bf Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sun, 19 Nov 2023 00:40:47 -0300 Subject: [PATCH 01/32] feat(http): Support for `Retry-After` header --- lib/util/http/index.ts | 10 ++- lib/util/http/retry-after.spec.ts | 111 ++++++++++++++++++++++++++++++ lib/util/http/retry-after.ts | 67 ++++++++++++++++++ lib/util/http/types.ts | 3 +- 4 files changed, 188 insertions(+), 3 deletions(-) create mode 100644 lib/util/http/retry-after.spec.ts create mode 100644 lib/util/http/retry-after.ts diff --git a/lib/util/http/index.ts b/lib/util/http/index.ts index eda337cd5160ed..c58e8cc6dacb54 100644 --- a/lib/util/http/index.ts +++ b/lib/util/http/index.ts @@ -10,11 +10,12 @@ import * as memCache from '../cache/memory'; import { clone } from '../clone'; import { hash } from '../hash'; import { type AsyncResult, Result } from '../result'; -import { resolveBaseUrl } from '../url'; +import { parseUrl, resolveBaseUrl } from '../url'; import { applyAuthorization, removeAuthorization } from './auth'; import { hooks } from './hooks'; import { applyHostRules } from './host-rules'; import { getQueue } from './queue'; +import { extractRetryAfterHeaderSeconds, wrapWithRetry } from './retry-after'; import { Throttle, getThrottle } from './throttle'; import type { GotJSONOptions, @@ -225,7 +226,12 @@ export class Http { ? () => queue.add>(throttledTask) : throttledTask; - resPromise = queuedTask(); + const host = parseUrl(url)?.host ?? /* istanbul ignore next */ url; + resPromise = wrapWithRetry( + host, + queuedTask, + extractRetryAfterHeaderSeconds, + ); if (memCacheKey) { memCache.set(memCacheKey, resPromise); diff --git a/lib/util/http/retry-after.spec.ts b/lib/util/http/retry-after.spec.ts new file mode 100644 index 00000000000000..a0fae918aa3e6c --- /dev/null +++ b/lib/util/http/retry-after.spec.ts @@ -0,0 +1,111 @@ +import { RequestError } from 'got'; +import { extractRetryAfterHeaderSeconds, wrapWithRetry } from './retry-after'; + +describe('util/http/retry-after', () => { + describe('wrapWithRetry', () => { + beforeEach(() => { + jest.useFakeTimers({ advanceTimers: true }); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('works', async () => { + const task = jest.fn(() => Promise.resolve(42)); + const res = await wrapWithRetry('http://example.com', task, () => null); + expect(res).toBe(42); + expect(task).toHaveBeenCalledTimes(1); + }); + + it('throws', async () => { + const task = jest.fn(() => Promise.reject(new Error('foo'))); + + await expect( + wrapWithRetry('http://example.com', task, () => null), + ).rejects.toThrow('foo'); + + expect(task).toHaveBeenCalledTimes(1); + }); + + it('retries', async () => { + const task = jest + .fn() + .mockRejectedValueOnce(new Error('foo')) + .mockRejectedValueOnce(new Error('bar')) + .mockResolvedValueOnce(42); + + const p = wrapWithRetry('http://example.com', task, () => 1); + await jest.advanceTimersByTimeAsync(2000); + + const res = await p; + expect(res).toBe(42); + expect(task).toHaveBeenCalledTimes(3); + }); + + it('gives up after max retries', async () => { + const task = jest + .fn() + .mockRejectedValueOnce('foo') + .mockRejectedValueOnce('bar') + .mockRejectedValueOnce('baz') + .mockRejectedValue('qux'); + + const p = wrapWithRetry('http://example.com', task, () => 1).catch( + (err) => err, + ); + await jest.advanceTimersByTimeAsync(2000); + + await expect(p).resolves.toBe('baz'); + expect(task).toHaveBeenCalledTimes(3); + }); + }); + + describe('extractRetryAfterHeaderSeconds', () => { + it('returns null for non-RequestError', () => { + expect(extractRetryAfterHeaderSeconds(new Error())).toBeNull(); + }); + + it('returns null for RequestError without response', () => { + expect( + extractRetryAfterHeaderSeconds( + new RequestError('foo', {}, null as never), + ), + ).toBeNull(); + }); + + it('returns null for status other than 429', () => { + const err = new RequestError('foo', {}, null as never); + (err as any).response = { statusCode: 302 }; + expect(extractRetryAfterHeaderSeconds(err)).toBeNull(); + }); + + it('returns null missing "retry-after" header', () => { + const err = new RequestError('foo', {}, null as never); + (err as any).response = { statusCode: 429, headers: {} }; + expect(extractRetryAfterHeaderSeconds(err)).toBeNull(); + }); + + it('returns null for non-integer "retry-after" header', () => { + const err = new RequestError('foo', {}, null as never); + (err as any).response = { + statusCode: 429, + headers: { + 'retry-after': 'Wed, 21 Oct 2015 07:28:00 GMT', + }, + }; + expect(extractRetryAfterHeaderSeconds(err)).toBeNull(); + }); + + it('returns delay in seconds', () => { + const err = new RequestError('foo', {}, null as never); + (err as any).response = { + statusCode: 429, + headers: { + 'retry-after': '42', + }, + }; + expect(extractRetryAfterHeaderSeconds(err)).toBe(42); + }); + }); +}); diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts new file mode 100644 index 00000000000000..cc18c84cd2a979 --- /dev/null +++ b/lib/util/http/retry-after.ts @@ -0,0 +1,67 @@ +import { RequestError } from 'got'; +import type { Task } from './types'; + +const hostBlocks = new Map>(); + +const maxRetries = 2; + +function sleep(seconds: number): Promise { + return new Promise((resolve) => global.setTimeout(resolve, seconds * 1000)); +} + +export function extractRetryAfterHeaderSeconds(err: unknown): number | null { + if (!(err instanceof RequestError)) { + return null; + } + + if (!err.response) { + return null; + } + + if (err.response.statusCode !== 429) { + return null; + } + + const retryAfter = err.response.headers['retry-after']; + if (!retryAfter) { + return null; + } + + const seconds = parseInt(retryAfter, 10); + if (Number.isNaN(seconds)) { + return null; + } + + return seconds; +} + +export async function wrapWithRetry( + key: string, + task: Task, + getDelaySeconds: (err: unknown) => number | null, +): Promise { + let retries = 0; + for (;;) { + try { + const delayPromise = hostBlocks.get(key); + if (delayPromise) { + await delayPromise; + hostBlocks.delete(key); + } + + return await task(); + } catch (err) { + if (retries === maxRetries) { + throw err; + } + + const delaySeconds = getDelaySeconds(err); + if (delaySeconds === null) { + throw err; + } + + hostBlocks.set(key, sleep(delaySeconds)); + retries += 1; + } + } +} diff --git a/lib/util/http/types.ts b/lib/util/http/types.ts index 4f576ef17a1788..86b5366ac53b64 100644 --- a/lib/util/http/types.ts +++ b/lib/util/http/types.ts @@ -92,4 +92,5 @@ export interface HttpResponse { authorization?: boolean; } -export type GotTask = () => Promise>; +export type Task = () => Promise; +export type GotTask = Task>; From 65a7e5d0957e209c5d0b1b9130ed676658bff220 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sun, 19 Nov 2023 11:24:13 -0300 Subject: [PATCH 02/32] Use `timers/promises` --- lib/util/http/retry-after.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index cc18c84cd2a979..52175e3e77a089 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -1,3 +1,4 @@ +import { setTimeout } from 'timers/promises'; import { RequestError } from 'got'; import type { Task } from './types'; @@ -5,10 +6,6 @@ const hostBlocks = new Map>(); const maxRetries = 2; -function sleep(seconds: number): Promise { - return new Promise((resolve) => global.setTimeout(resolve, seconds * 1000)); -} - export function extractRetryAfterHeaderSeconds(err: unknown): number | null { if (!(err instanceof RequestError)) { return null; @@ -60,7 +57,7 @@ export async function wrapWithRetry( throw err; } - hostBlocks.set(key, sleep(delaySeconds)); + hostBlocks.set(key, setTimeout(delaySeconds * 1000)); retries += 1; } } From 387414068efa88a7bdd346bb1e4145ebcf99162c Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sun, 19 Nov 2023 16:30:58 -0300 Subject: [PATCH 03/32] Add maxRetryAfter option to host rules --- lib/config/options/index.ts | 10 ++++ lib/types/host-rules.ts | 1 + lib/util/http/index.ts | 12 ++-- lib/util/http/retry-after.spec.ts | 91 ++++++++++++++++++++++--------- lib/util/http/retry-after.ts | 67 +++++++++++++---------- 5 files changed, 123 insertions(+), 58 deletions(-) diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts index a3d0eed47ef532..8663aa6045287b 100644 --- a/lib/config/options/index.ts +++ b/lib/config/options/index.ts @@ -2734,6 +2734,16 @@ const options: RenovateOptions[] = [ globalOnly: true, default: [], }, + { + name: 'maxRetryAfter', + description: + 'Maximum number of seconds to wait before retrying a failed request.', + type: 'integer', + stage: 'package', + parent: 'hostRules', + cli: false, + env: false, + }, ]; export function getOptions(): RenovateOptions[] { diff --git a/lib/types/host-rules.ts b/lib/types/host-rules.ts index 058ed21e1e6d30..b95786c74cda5a 100644 --- a/lib/types/host-rules.ts +++ b/lib/types/host-rules.ts @@ -11,6 +11,7 @@ export interface HostRuleSearchResult { enableHttp2?: boolean; concurrentRequestLimit?: number; maxRequestsPerSecond?: number; + maxRetryAfter?: number; dnsCache?: boolean; keepalive?: boolean; diff --git a/lib/util/http/index.ts b/lib/util/http/index.ts index c58e8cc6dacb54..ae499685f61aa8 100644 --- a/lib/util/http/index.ts +++ b/lib/util/http/index.ts @@ -10,12 +10,16 @@ import * as memCache from '../cache/memory'; import { clone } from '../clone'; import { hash } from '../hash'; import { type AsyncResult, Result } from '../result'; -import { parseUrl, resolveBaseUrl } from '../url'; +import { resolveBaseUrl } from '../url'; import { applyAuthorization, removeAuthorization } from './auth'; import { hooks } from './hooks'; import { applyHostRules } from './host-rules'; import { getQueue } from './queue'; -import { extractRetryAfterHeaderSeconds, wrapWithRetry } from './retry-after'; +import { + extractRetryAfterHeaderSeconds, + getMaxRetryAfter, + wrapWithRetry, +} from './retry-after'; import { Throttle, getThrottle } from './throttle'; import type { GotJSONOptions, @@ -226,11 +230,11 @@ export class Http { ? () => queue.add>(throttledTask) : throttledTask; - const host = parseUrl(url)?.host ?? /* istanbul ignore next */ url; resPromise = wrapWithRetry( - host, + url, queuedTask, extractRetryAfterHeaderSeconds, + getMaxRetryAfter(url), ); if (memCacheKey) { diff --git a/lib/util/http/retry-after.spec.ts b/lib/util/http/retry-after.spec.ts index a0fae918aa3e6c..fd355e0e9f8855 100644 --- a/lib/util/http/retry-after.spec.ts +++ b/lib/util/http/retry-after.spec.ts @@ -1,5 +1,23 @@ import { RequestError } from 'got'; -import { extractRetryAfterHeaderSeconds, wrapWithRetry } from './retry-after'; +import * as hostRules from '../host-rules'; +import { + extractRetryAfterHeaderSeconds, + getMaxRetryAfter, + wrapWithRetry, +} from './retry-after'; + +function requestError( + response: { + statusCode?: number; + headers?: Record; + } | null = null, +) { + const err = new RequestError('request error', {}, null as never); + if (response) { + (err as any).response = response; + } + return err; +} describe('util/http/retry-after', () => { describe('wrapWithRetry', () => { @@ -67,45 +85,66 @@ describe('util/http/retry-after', () => { }); it('returns null for RequestError without response', () => { - expect( - extractRetryAfterHeaderSeconds( - new RequestError('foo', {}, null as never), - ), - ).toBeNull(); + expect(extractRetryAfterHeaderSeconds(requestError())).toBeNull(); }); it('returns null for status other than 429', () => { const err = new RequestError('foo', {}, null as never); (err as any).response = { statusCode: 302 }; - expect(extractRetryAfterHeaderSeconds(err)).toBeNull(); + expect( + extractRetryAfterHeaderSeconds(requestError({ statusCode: 302 })), + ).toBeNull(); }); it('returns null missing "retry-after" header', () => { - const err = new RequestError('foo', {}, null as never); - (err as any).response = { statusCode: 429, headers: {} }; - expect(extractRetryAfterHeaderSeconds(err)).toBeNull(); + expect( + extractRetryAfterHeaderSeconds( + requestError({ statusCode: 429, headers: {} }), + ), + ).toBeNull(); }); it('returns null for non-integer "retry-after" header', () => { - const err = new RequestError('foo', {}, null as never); - (err as any).response = { - statusCode: 429, - headers: { - 'retry-after': 'Wed, 21 Oct 2015 07:28:00 GMT', - }, - }; - expect(extractRetryAfterHeaderSeconds(err)).toBeNull(); + expect( + extractRetryAfterHeaderSeconds( + requestError({ + statusCode: 429, + headers: { + 'retry-after': 'Wed, 21 Oct 2015 07:28:00 GMT', + }, + }), + ), + ).toBeNull(); }); it('returns delay in seconds', () => { - const err = new RequestError('foo', {}, null as never); - (err as any).response = { - statusCode: 429, - headers: { - 'retry-after': '42', - }, - }; - expect(extractRetryAfterHeaderSeconds(err)).toBe(42); + expect( + extractRetryAfterHeaderSeconds( + requestError({ + statusCode: 429, + headers: { + 'retry-after': '42', + }, + }), + ), + ).toBe(42); + }); + }); + + describe('getMaxRetryAfter', () => { + afterEach(() => { + hostRules.clear(); + }); + + it('returns default value of 60 when host rule is not set', () => { + expect(getMaxRetryAfter('http://example.com')).toBe(60); + }); + + it('returns maxRetryAfter', () => { + hostRules.add({ matchHost: 'foo.com', maxRetryAfter: 111 }); + hostRules.add({ matchHost: 'bar.com', maxRetryAfter: 222 }); + expect(getMaxRetryAfter('http://foo.com')).toBe(111); + expect(getMaxRetryAfter('http://bar.com')).toBe(222); }); }); }); diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index 52175e3e77a089..d18ca18a3010e5 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -1,42 +1,21 @@ import { setTimeout } from 'timers/promises'; import { RequestError } from 'got'; +import * as hostRules from '../host-rules'; +import { parseUrl } from '../url'; import type { Task } from './types'; const hostBlocks = new Map>(); const maxRetries = 2; -export function extractRetryAfterHeaderSeconds(err: unknown): number | null { - if (!(err instanceof RequestError)) { - return null; - } - - if (!err.response) { - return null; - } - - if (err.response.statusCode !== 429) { - return null; - } - - const retryAfter = err.response.headers['retry-after']; - if (!retryAfter) { - return null; - } - - const seconds = parseInt(retryAfter, 10); - if (Number.isNaN(seconds)) { - return null; - } - - return seconds; -} - export async function wrapWithRetry( - key: string, + url: string, task: Task, getDelaySeconds: (err: unknown) => number | null, + maxRetryAfter = 60, ): Promise { + const key = parseUrl(url)?.host ?? /* istanbul ignore next */ url; + let retries = 0; for (;;) { try { @@ -57,8 +36,40 @@ export async function wrapWithRetry( throw err; } - hostBlocks.set(key, setTimeout(delaySeconds * 1000)); + const delay = 1000 * Math.max(0, Math.min(delaySeconds, maxRetryAfter)); + hostBlocks.set(key, setTimeout(delay)); retries += 1; } } } + +export function extractRetryAfterHeaderSeconds(err: unknown): number | null { + if (!(err instanceof RequestError)) { + return null; + } + + if (!err.response) { + return null; + } + + if (err.response.statusCode !== 429) { + return null; + } + + const retryAfter = err.response.headers['retry-after']; + if (!retryAfter) { + return null; + } + + const seconds = parseInt(retryAfter, 10); + if (Number.isNaN(seconds)) { + return null; + } + + return seconds; +} + +export function getMaxRetryAfter(url: string): number { + const { maxRetryAfter = 60 } = hostRules.find({ url }); + return maxRetryAfter; +} From 1f1e1d390dacc72dacd557f703d4ae7a619374fa Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sun, 19 Nov 2023 16:48:28 -0300 Subject: [PATCH 04/32] Add doc --- docs/usage/configuration-options.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 25497b2c9f305a..416a22363b46a0 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -1763,6 +1763,23 @@ Example config: } ``` +### maxRetryAfter + +By default, Renovate will retry after any `Retry-After` header value, up to a maximum of 60 seconds. + +You can configure a different maximum value using `maxRetryAfter`: + +```json +{ + "hostRules": [ + { + "matchHost": "api.github.com", + "maxRetryAfter": 25 + } + ] +} +``` + ### dnsCache Enable got [dnsCache](https://github.com/sindresorhus/got/blob/v11.5.2/readme.md#dnsCache) support. From 29beaf34fc8d5e1d0c58e2598024994662b6feee Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Mon, 20 Nov 2023 09:41:27 -0300 Subject: [PATCH 05/32] Update lib/config/options/index.ts Co-authored-by: Rhys Arkins --- lib/config/options/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts index 8663aa6045287b..d9da7ae2418840 100644 --- a/lib/config/options/index.ts +++ b/lib/config/options/index.ts @@ -2739,6 +2739,7 @@ const options: RenovateOptions[] = [ description: 'Maximum number of seconds to wait before retrying a failed request.', type: 'integer', + default: 60, stage: 'package', parent: 'hostRules', cli: false, From e473548f192fdfb7464668f1fa3297e506308065 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Mon, 20 Nov 2023 18:23:09 -0300 Subject: [PATCH 06/32] Add logging --- lib/util/http/retry-after.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index d18ca18a3010e5..c6648d7a38c928 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -1,5 +1,6 @@ import { setTimeout } from 'timers/promises'; import { RequestError } from 'got'; +import { logger } from '../../logger'; import * as hostRules from '../host-rules'; import { parseUrl } from '../url'; import type { Task } from './types'; @@ -27,15 +28,21 @@ export async function wrapWithRetry( return await task(); } catch (err) { - if (retries === maxRetries) { + const delaySeconds = getDelaySeconds(err); + if (delaySeconds === null) { throw err; } - const delaySeconds = getDelaySeconds(err); - if (delaySeconds === null) { + if (retries === maxRetries) { + logger.debug({ url }, 'Retry-After: giving up'); throw err; } + logger.debug( + { url, delaySeconds }, + `Retry-After: will retry after ${delaySeconds} seconds`, + ); + const delay = 1000 * Math.max(0, Math.min(delaySeconds, maxRetryAfter)); hostBlocks.set(key, setTimeout(delay)); retries += 1; @@ -52,7 +59,7 @@ export function extractRetryAfterHeaderSeconds(err: unknown): number | null { return null; } - if (err.response.statusCode !== 429) { + if (err.response.statusCode !== 429 && err.response.statusCode !== 403) { return null; } @@ -63,6 +70,10 @@ export function extractRetryAfterHeaderSeconds(err: unknown): number | null { const seconds = parseInt(retryAfter, 10); if (Number.isNaN(seconds)) { + logger.debug( + { url: err.response.url, retryAfter }, + 'Retry-After: unsupported format', + ); return null; } From 9bebe1ea111535b41bc21e3ed8e99f67a805c64c Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Tue, 21 Nov 2023 11:43:41 -0300 Subject: [PATCH 07/32] Apply suggestions from code review Co-authored-by: Rhys Arkins --- lib/config/options/index.ts | 2 +- lib/util/http/retry-after.ts | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts index d9da7ae2418840..f663e5ceb8943c 100644 --- a/lib/config/options/index.ts +++ b/lib/config/options/index.ts @@ -2737,7 +2737,7 @@ const options: RenovateOptions[] = [ { name: 'maxRetryAfter', description: - 'Maximum number of seconds to wait before retrying a failed request.', + 'Maximum retry-after header value to wait for before retrying a failed request.', type: 'integer', default: 60, stage: 'package', diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index c6648d7a38c928..d238bd70d51f4f 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -34,13 +34,12 @@ export async function wrapWithRetry( } if (retries === maxRetries) { - logger.debug({ url }, 'Retry-After: giving up'); + logger.debug(`Retry-After: reached maximum retries (${maxRetries}) for ${url}`); throw err; } logger.debug( - { url, delaySeconds }, - `Retry-After: will retry after ${delaySeconds} seconds`, + `Retry-After: will retry ${url} after ${delaySeconds} seconds`, ); const delay = 1000 * Math.max(0, Math.min(delaySeconds, maxRetryAfter)); From fe9bf36af566a11ca43c0f925d11650eb31d2ffc Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Tue, 21 Nov 2023 12:09:52 -0300 Subject: [PATCH 08/32] Fix error handling and retry logic in wrapWithRetry function --- lib/util/http/retry-after.spec.ts | 31 +++++++++++++++++++++---------- lib/util/http/retry-after.ts | 14 +++++++++++--- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/lib/util/http/retry-after.spec.ts b/lib/util/http/retry-after.spec.ts index fd355e0e9f8855..1db63900927ecf 100644 --- a/lib/util/http/retry-after.spec.ts +++ b/lib/util/http/retry-after.spec.ts @@ -37,11 +37,11 @@ describe('util/http/retry-after', () => { }); it('throws', async () => { - const task = jest.fn(() => Promise.reject(new Error('foo'))); + const task = jest.fn(() => Promise.reject(new Error('error'))); await expect( wrapWithRetry('http://example.com', task, () => null), - ).rejects.toThrow('foo'); + ).rejects.toThrow('error'); expect(task).toHaveBeenCalledTimes(1); }); @@ -49,8 +49,8 @@ describe('util/http/retry-after', () => { it('retries', async () => { const task = jest .fn() - .mockRejectedValueOnce(new Error('foo')) - .mockRejectedValueOnce(new Error('bar')) + .mockRejectedValueOnce(new Error('error-1')) + .mockRejectedValueOnce(new Error('error-2')) .mockResolvedValueOnce(42); const p = wrapWithRetry('http://example.com', task, () => 1); @@ -64,19 +64,30 @@ describe('util/http/retry-after', () => { it('gives up after max retries', async () => { const task = jest .fn() - .mockRejectedValueOnce('foo') - .mockRejectedValueOnce('bar') - .mockRejectedValueOnce('baz') - .mockRejectedValue('qux'); + .mockRejectedValueOnce('error-1') + .mockRejectedValueOnce('error-2') + .mockRejectedValueOnce('error-3') + .mockRejectedValue('error-4'); const p = wrapWithRetry('http://example.com', task, () => 1).catch( (err) => err, ); await jest.advanceTimersByTimeAsync(2000); - await expect(p).resolves.toBe('baz'); + await expect(p).resolves.toBe('error-3'); expect(task).toHaveBeenCalledTimes(3); }); + + it('gives up when delay exceeds maxRetryAfter', async () => { + const task = jest.fn().mockRejectedValue('error'); + + const p = wrapWithRetry('http://example.com', task, () => 61).catch( + (err) => err, + ); + + await expect(p).resolves.toBe('error'); + expect(task).toHaveBeenCalledTimes(1); + }); }); describe('extractRetryAfterHeaderSeconds', () => { @@ -89,7 +100,7 @@ describe('util/http/retry-after', () => { }); it('returns null for status other than 429', () => { - const err = new RequestError('foo', {}, null as never); + const err = new RequestError('request-error', {}, null as never); (err as any).response = { statusCode: 302 }; expect( extractRetryAfterHeaderSeconds(requestError({ statusCode: 302 })), diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index d238bd70d51f4f..47ddfce48a2ef7 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -34,7 +34,16 @@ export async function wrapWithRetry( } if (retries === maxRetries) { - logger.debug(`Retry-After: reached maximum retries (${maxRetries}) for ${url}`); + logger.debug( + `Retry-After: reached maximum retries (${maxRetries}) for ${url}`, + ); + throw err; + } + + if (delaySeconds > maxRetryAfter) { + logger.debug( + `Retry-After: delay ${delaySeconds} seconds exceeds maxRetryAfter ${maxRetryAfter} seconds for ${url}`, + ); throw err; } @@ -42,8 +51,7 @@ export async function wrapWithRetry( `Retry-After: will retry ${url} after ${delaySeconds} seconds`, ); - const delay = 1000 * Math.max(0, Math.min(delaySeconds, maxRetryAfter)); - hostBlocks.set(key, setTimeout(delay)); + hostBlocks.set(key, setTimeout(delaySeconds)); retries += 1; } } From 8bef63ad593cc35685a1a7825bfd789ec5263881 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Wed, 22 Nov 2023 21:00:54 -0300 Subject: [PATCH 09/32] Update docs/usage/configuration-options.md Co-authored-by: HonkingGoose <34918129+HonkingGoose@users.noreply.github.com> --- docs/usage/configuration-options.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 416a22363b46a0..810e36a3d8aa9f 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -1765,7 +1765,10 @@ Example config: ### maxRetryAfter -By default, Renovate will retry after any `Retry-After` header value, up to a maximum of 60 seconds. +A remote host may return a `4xx` response with a `Retry-After` header value, which indicates that Renovate has been rate-limited. +Renovate may try to contact the host again after waiting a certain time, that's set by the host. +By default, Renovate tries again after the `Retry-After` header value has passed, up to a maximum of 60 seconds. +If the `Retry-After` value is more than 60 seconds, Renovate will abort the job instead of waiting. You can configure a different maximum value using `maxRetryAfter`: From 0a34a2d22d454f43c4fe31b3b783796b7ce322a1 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sun, 26 Nov 2023 10:10:33 -0300 Subject: [PATCH 10/32] Update docs/usage/configuration-options.md Co-authored-by: HonkingGoose <34918129+HonkingGoose@users.noreply.github.com> --- docs/usage/configuration-options.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index f571a08b40d876..692c6a33aaceaa 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -1768,7 +1768,7 @@ Example config: A remote host may return a `4xx` response with a `Retry-After` header value, which indicates that Renovate has been rate-limited. Renovate may try to contact the host again after waiting a certain time, that's set by the host. By default, Renovate tries again after the `Retry-After` header value has passed, up to a maximum of 60 seconds. -If the `Retry-After` value is more than 60 seconds, Renovate will abort the job instead of waiting. +If the `Retry-After` value is more than 60 seconds, Renovate will abort the request instead of waiting. You can configure a different maximum value using `maxRetryAfter`: From a77737231ec11e2b933a28d75a1a124f34b78e76 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Mon, 27 Nov 2023 11:14:05 -0300 Subject: [PATCH 11/32] Set `maxRetryAfter` to zero for `got` --- lib/util/http/index.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/util/http/index.ts b/lib/util/http/index.ts index ae499685f61aa8..8a671d984d3599 100644 --- a/lib/util/http/index.ts +++ b/lib/util/http/index.ts @@ -135,11 +135,14 @@ export class Http { protected hostType: string, options: HttpOptions = {}, ) { - this.options = merge(options, { context: { hostType } }); - - if (process.env.NODE_ENV === 'test') { - this.options.retry = 0; - } + const retryLimit = process.env.NODE_ENV === 'test' ? 0 : 2; + this.options = merge(options, { + context: { hostType }, + retry: { + limit: retryLimit, + maxRetryAfter: 0, + }, + }); } protected getThrottle(url: string): Throttle | null { From 5f503cfb2e7aeaa625923cae6833c14caa0fdc48 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Tue, 28 Nov 2023 15:49:07 -0300 Subject: [PATCH 12/32] Fix milliseconds --- lib/util/http/retry-after.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index 47ddfce48a2ef7..034eb5c2e6334a 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -51,7 +51,7 @@ export async function wrapWithRetry( `Retry-After: will retry ${url} after ${delaySeconds} seconds`, ); - hostBlocks.set(key, setTimeout(delaySeconds)); + hostBlocks.set(key, setTimeout(1000 * delaySeconds)); retries += 1; } } From 99585548d592e8143c624081ce4fb179b4006ffb Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Thu, 30 Nov 2023 18:51:05 -0300 Subject: [PATCH 13/32] refactor(http): Separate host rule search and apply --- lib/util/http/github.ts | 9 +- lib/util/http/host-rules.spec.ts | 382 +++++++++++++++++++++++-------- lib/util/http/host-rules.ts | 43 ++-- lib/util/http/index.ts | 8 +- 4 files changed, 321 insertions(+), 121 deletions(-) diff --git a/lib/util/http/github.ts b/lib/util/http/github.ts index bf8ce9848ff04a..bb313b12a642a0 100644 --- a/lib/util/http/github.ts +++ b/lib/util/http/github.ts @@ -14,7 +14,7 @@ import * as p from '../promises'; import { range } from '../range'; import { regEx } from '../regex'; import { joinUrlParts, parseLinkHeader, resolveBaseUrl } from '../url'; -import { findMatchingRules } from './host-rules'; +import { findMatchingRule } from './host-rules'; import type { GotLegacyError } from './legacy'; import type { GraphqlOptions, @@ -295,10 +295,9 @@ export class GithubHttp extends Http { ); } - const { token } = findMatchingRules( - { hostType: this.hostType }, - authUrl.toString(), - ); + const { token } = findMatchingRule(authUrl.toString(), { + hostType: this.hostType, + }); opts.token = token; } diff --git a/lib/util/http/host-rules.spec.ts b/lib/util/http/host-rules.spec.ts index 11c215d5edb438..bbd34eeac79e35 100644 --- a/lib/util/http/host-rules.spec.ts +++ b/lib/util/http/host-rules.spec.ts @@ -1,7 +1,8 @@ import { bootstrap } from '../../proxy'; +import type { HostRule } from '../../types'; import * as hostRules from '../host-rules'; import { dnsLookup } from './dns'; -import { applyHostRules } from './host-rules'; +import { applyHostRule, findMatchingRule } from './host-rules'; import type { GotOptions } from './types'; const url = 'https://github.com'; @@ -50,7 +51,14 @@ describe('util/http/host-rules', () => { }); it('adds token', () => { - expect(applyHostRules(url, { ...options })).toMatchInlineSnapshot(` + const opts = { ...options }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "token", + } + `); + expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` { "context": { "authType": undefined, @@ -62,7 +70,14 @@ describe('util/http/host-rules', () => { }); it('adds auth', () => { - expect(applyHostRules(url, { hostType: 'gitea' })).toMatchInlineSnapshot(` + const opts = { hostType: 'gitea' }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "password": "password", + } + `); + expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` { "hostType": "gitea", "password": "password", @@ -72,7 +87,16 @@ describe('util/http/host-rules', () => { }); it('adds custom auth', () => { - expect(applyHostRules(url, { hostType: 'npm' })).toMatchInlineSnapshot(` + const opts = { hostType: 'npm' }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "authType": "Basic", + "timeout": 5000, + "token": "XXX", + } + `); + expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` { "context": { "authType": "Basic", @@ -85,8 +109,14 @@ describe('util/http/host-rules', () => { }); it('skips', () => { - expect(applyHostRules(url, { ...options, token: 'xxx' })) - .toMatchInlineSnapshot(` + const opts = { ...options, token: 'xxx' }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "token", + } + `); + expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` { "hostType": "github", "token": "xxx", @@ -96,8 +126,16 @@ describe('util/http/host-rules', () => { it('uses http2', () => { hostRules.add({ enableHttp2: true }); - expect(applyHostRules(url, { ...options, token: 'xxx' })) - .toMatchInlineSnapshot(` + + const opts = { ...options, token: 'xxx' }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "enableHttp2": true, + "token": "token", + } + `); + expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` { "hostType": "github", "http2": true, @@ -108,7 +146,16 @@ describe('util/http/host-rules', () => { it('uses dnsCache', () => { hostRules.add({ dnsCache: true }); - expect(applyHostRules(url, { ...options, token: 'xxx' })).toMatchObject({ + + const opts = { ...options, token: 'xxx' }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "dnsCache": true, + "token": "token", + } + `); + expect(applyHostRule(url, opts, hostRule)).toMatchObject({ hostType: 'github', lookup: dnsLookup, token: 'xxx', @@ -117,17 +164,32 @@ describe('util/http/host-rules', () => { it('uses http keep-alive', () => { hostRules.add({ keepAlive: true }); - expect( - applyHostRules(url, { ...options, token: 'xxx' }).agent, - ).toBeDefined(); + + const opts = { ...options, token: 'xxx' }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "keepAlive": true, + "token": "token", + } + `); + expect(applyHostRule(url, opts, hostRule).agent).toBeDefined(); }); it('disables http2', () => { process.env.HTTP_PROXY = 'http://proxy'; bootstrap(); hostRules.add({ enableHttp2: true }); - expect(applyHostRules(url, { ...options, token: 'xxx' })) - .toMatchInlineSnapshot(` + + const opts = { ...options, token: 'xxx' }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "enableHttp2": true, + "token": "token", + } + `); + expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` { "hostType": "github", "token": "xxx", @@ -136,8 +198,14 @@ describe('util/http/host-rules', () => { }); it('noAuth', () => { - expect(applyHostRules(url, { ...options, noAuth: true })) - .toMatchInlineSnapshot(` + const opts = { ...options, noAuth: true }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "token", + } + `); + expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` { "hostType": "github", "noAuth": true, @@ -152,12 +220,15 @@ describe('util/http/host-rules', () => { httpsCertificateAuthority: 'ca-cert', }); - expect( - applyHostRules('https://custom.datasource.ca/data/path', { - ...options, - hostType: 'maven', - }), - ).toMatchInlineSnapshot(` + const url = 'https://custom.datasource.ca/data/path'; + const opts = { ...options, hostType: 'maven' }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "httpsCertificateAuthority": "ca-cert", + } + `); + expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` { "hostType": "maven", "https": { @@ -173,12 +244,16 @@ describe('util/http/host-rules', () => { matchHost: 'https://custom.datasource.key', httpsPrivateKey: 'key', }); - expect( - applyHostRules('https://custom.datasource.key/data/path', { - ...options, - hostType: 'maven', - }), - ).toMatchInlineSnapshot(` + + const url = 'https://custom.datasource.key/data/path'; + const opts = { ...options, hostType: 'maven' }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "httpsPrivateKey": "key", + } + `); + expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` { "hostType": "maven", "https": { @@ -195,12 +270,15 @@ describe('util/http/host-rules', () => { httpsCertificate: 'cert', }); - expect( - applyHostRules('https://custom.datasource.cert/data/path', { - ...options, - hostType: 'maven', - }), - ).toMatchInlineSnapshot(` + const url = 'https://custom.datasource.cert/data/path'; + const opts = { ...options, hostType: 'maven' }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "httpsCertificate": "cert", + } + `); + expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` { "hostType": "maven", "https": { @@ -229,30 +307,61 @@ describe('util/http/host-rules', () => { username: 'some', password: 'xxx', }); - expect( - applyHostRules(url, { ...options, hostType: 'github-releases' }), - ).toEqual({ + + let opts: GotOptions; + let hostRule: HostRule; + + opts = { ...options, hostType: 'github-releases' }; + hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "password": "xxx", + "username": "some", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ hostType: 'github-releases', username: 'some', password: 'xxx', }); - expect( - applyHostRules(url, { ...options, hostType: 'github-tags' }), - ).toEqual({ + + opts = { ...options, hostType: 'github-tags' }; + hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "password": "xxx2", + "username": "some2", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ hostType: 'github-tags', username: 'some2', password: 'xxx2', }); - expect(applyHostRules(url, { ...options, hostType: 'pod' })).toEqual({ + + opts = { ...options, hostType: 'pod' }; + hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "pod-token", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, }, hostType: 'pod', token: 'pod-token', }); - expect( - applyHostRules(url, { ...options, hostType: 'github-changelog' }), - ).toEqual({ + + opts = { ...options, hostType: 'github-changelog' }; + hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "changelogtoken", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, }, @@ -262,25 +371,47 @@ describe('util/http/host-rules', () => { }); it('fallback to github', () => { - expect( - applyHostRules(url, { ...options, hostType: 'github-tags' }), - ).toEqual({ + let opts: GotOptions; + let hostRule: HostRule; + + opts = { ...options, hostType: 'github-tags' }; + hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "token", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, }, hostType: 'github-tags', token: 'token', }); - expect( - applyHostRules(url, { ...options, hostType: 'github-changelog' }), - ).toEqual({ + + opts = { ...options, hostType: 'github-changelog' }; + hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "token", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, }, hostType: 'github-changelog', token: 'token', }); - expect(applyHostRules(url, { ...options, hostType: 'pod' })).toEqual({ + + opts = { ...options, hostType: 'pod' }; + hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "token", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, }, @@ -302,27 +433,48 @@ describe('util/http/host-rules', () => { hostType: 'gitlab-tags', token: 'tags-token', }); - expect( - applyHostRules(url, { ...options, hostType: 'gitlab-tags' }), - ).toEqual({ + + let opts: GotOptions; + let hostRule: HostRule; + + opts = { ...options, hostType: 'gitlab-tags' }; + hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "tags-token", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, }, hostType: 'gitlab-tags', token: 'tags-token', }); - expect( - applyHostRules(url, { ...options, hostType: 'gitlab-releases' }), - ).toEqual({ + + opts = { ...options, hostType: 'gitlab-releases' }; + hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "release-token", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, }, hostType: 'gitlab-releases', token: 'release-token', }); - expect( - applyHostRules(url, { ...options, hostType: 'gitlab-packages' }), - ).toEqual({ + + opts = { ...options, hostType: 'gitlab-packages' }; + hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "package-token", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, }, @@ -332,36 +484,62 @@ describe('util/http/host-rules', () => { }); it('fallback to gitlab', () => { - expect( - applyHostRules(url, { ...options, hostType: 'gitlab-tags' }), - ).toEqual({ + let opts: GotOptions; + let hostRule: HostRule; + + opts = { ...options, hostType: 'gitlab-tags' }; + hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "abc", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, }, hostType: 'gitlab-tags', token: 'abc', }); - expect( - applyHostRules(url, { ...options, hostType: 'gitlab-releases' }), - ).toEqual({ + + opts = { ...options, hostType: 'gitlab-releases' }; + hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "abc", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, }, hostType: 'gitlab-releases', token: 'abc', }); - expect( - applyHostRules(url, { ...options, hostType: 'gitlab-packages' }), - ).toEqual({ + + opts = { ...options, hostType: 'gitlab-packages' }; + hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "abc", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, }, hostType: 'gitlab-packages', token: 'abc', }); - expect( - applyHostRules(url, { ...options, hostType: 'gitlab-changelog' }), - ).toEqual({ + + opts = { ...options, hostType: 'gitlab-changelog' }; + hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "abc", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, }, @@ -376,9 +554,15 @@ describe('util/http/host-rules', () => { username: 'some', password: 'xxx', }); - expect( - applyHostRules(url, { ...options, hostType: 'bitbucket-tags' }), - ).toEqual({ + const opts = { ...options, hostType: 'bitbucket-tags' }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "password": "xxx", + "username": "some", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ hostType: 'bitbucket-tags', username: 'some', password: 'xxx', @@ -386,9 +570,14 @@ describe('util/http/host-rules', () => { }); it('fallback to bitbucket', () => { - expect( - applyHostRules(url, { ...options, hostType: 'bitbucket-tags' }), - ).toEqual({ + const opts = { ...options, hostType: 'bitbucket-tags' }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` + { + "token": "cdef", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, }, @@ -402,24 +591,35 @@ describe('util/http/host-rules', () => { hostType: 'gitea-tags', token: 'abc', }); - expect(applyHostRules(url, { ...options, hostType: 'gitea-tags' })).toEqual( + + const opts = { ...options, hostType: 'gitea-tags' }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` { - context: { - authType: undefined, - }, - hostType: 'gitea-tags', - token: 'abc', + "token": "abc", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ + context: { + authType: undefined, }, - ); + hostType: 'gitea-tags', + token: 'abc', + }); }); it('fallback to gitea', () => { - expect(applyHostRules(url, { ...options, hostType: 'gitea-tags' })).toEqual( + const opts = { ...options, hostType: 'gitea-tags' }; + const hostRule = findMatchingRule(url, opts); + expect(hostRule).toMatchInlineSnapshot(` { - hostType: 'gitea-tags', - password: 'password', - username: undefined, - }, - ); + "password": "password", + } + `); + expect(applyHostRule(url, opts, hostRule)).toEqual({ + hostType: 'gitea-tags', + password: 'password', + username: undefined, + }); }); }); diff --git a/lib/util/http/host-rules.ts b/lib/util/http/host-rules.ts index d0346584f00838..54e38dd97d3b9a 100644 --- a/lib/util/http/host-rules.ts +++ b/lib/util/http/host-rules.ts @@ -34,9 +34,9 @@ export type HostRulesGotOptions = Pick< | 'https' >; -export function findMatchingRules( - options: GotOptions, +export function findMatchingRule( url: string, + options: GotOptions, ): HostRule { const { hostType } = options; let res = hostRules.find({ hostType, url }); @@ -114,13 +114,12 @@ export function findMatchingRules( } // Apply host rules to requests -export function applyHostRules( +export function applyHostRule( url: string, - inOptions: GotOptions, + options: GotOptions, + hostRule: HostRule, ): GotOptions { - const options: GotOptions = { ...inOptions }; - const foundRules = findMatchingRules(options, url); - const { username, password, token, enabled, authType } = foundRules; + const { username, password, token, enabled, authType } = hostRule; const host = parseUrl(url)?.host; if (options.noAuth) { logger.trace({ url }, `Authorization disabled`); @@ -147,48 +146,48 @@ export function applyHostRules( logger.once.debug(`hostRules: no authentication for ${host}`); } // Apply optional params - if (foundRules.abortOnError) { - options.abortOnError = foundRules.abortOnError; + if (hostRule.abortOnError) { + options.abortOnError = hostRule.abortOnError; } - if (foundRules.abortIgnoreStatusCodes) { - options.abortIgnoreStatusCodes = foundRules.abortIgnoreStatusCodes; + if (hostRule.abortIgnoreStatusCodes) { + options.abortIgnoreStatusCodes = hostRule.abortIgnoreStatusCodes; } - if (foundRules.timeout) { - options.timeout = foundRules.timeout; + if (hostRule.timeout) { + options.timeout = hostRule.timeout; } - if (foundRules.dnsCache) { + if (hostRule.dnsCache) { options.lookup = dnsLookup; } - if (foundRules.keepAlive) { + if (hostRule.keepAlive) { options.agent = keepAliveAgents; } - if (!hasProxy() && foundRules.enableHttp2 === true) { + if (!hasProxy() && hostRule.enableHttp2 === true) { options.http2 = true; } - if (is.nonEmptyString(foundRules.httpsCertificateAuthority)) { + if (is.nonEmptyString(hostRule.httpsCertificateAuthority)) { options.https = { ...(options.https ?? {}), - certificateAuthority: foundRules.httpsCertificateAuthority, + certificateAuthority: hostRule.httpsCertificateAuthority, }; } - if (is.nonEmptyString(foundRules.httpsPrivateKey)) { + if (is.nonEmptyString(hostRule.httpsPrivateKey)) { options.https = { ...(options.https ?? {}), - key: foundRules.httpsPrivateKey, + key: hostRule.httpsPrivateKey, }; } - if (is.nonEmptyString(foundRules.httpsCertificate)) { + if (is.nonEmptyString(hostRule.httpsCertificate)) { options.https = { ...(options.https ?? {}), - certificate: foundRules.httpsCertificate, + certificate: hostRule.httpsCertificate, }; } diff --git a/lib/util/http/index.ts b/lib/util/http/index.ts index 8a671d984d3599..272fb96c5378c6 100644 --- a/lib/util/http/index.ts +++ b/lib/util/http/index.ts @@ -13,7 +13,7 @@ import { type AsyncResult, Result } from '../result'; import { resolveBaseUrl } from '../url'; import { applyAuthorization, removeAuthorization } from './auth'; import { hooks } from './hooks'; -import { applyHostRules } from './host-rules'; +import { applyHostRule, findMatchingRule } from './host-rules'; import { getQueue } from './queue'; import { extractRetryAfterHeaderSeconds, @@ -184,7 +184,8 @@ export class Http { applyDefaultHeaders(options); - options = applyHostRules(url, options); + const hostRule = findMatchingRule(url, options); + options = applyHostRule(url, options, hostRule); if (options.enabled === false) { logger.debug(`Host is disabled - rejecting request. HostUrl: ${url}`); throw new Error(HOST_DISABLED); @@ -499,7 +500,8 @@ export class Http { } applyDefaultHeaders(combinedOptions); - combinedOptions = applyHostRules(resolvedUrl, combinedOptions); + const hostRule = findMatchingRule(url, combinedOptions); + combinedOptions = applyHostRule(resolvedUrl, combinedOptions, hostRule); if (combinedOptions.enabled === false) { throw new Error(HOST_DISABLED); } From 186937cb5f733f0aab0761894bd4fa65a3b0ee27 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Thu, 30 Nov 2023 20:32:57 -0300 Subject: [PATCH 14/32] Correct `maxRetryAfter` usage --- lib/util/http/index.ts | 11 ++++------- lib/util/http/retry-after.ts | 6 ------ 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/lib/util/http/index.ts b/lib/util/http/index.ts index 272fb96c5378c6..c93553049ce124 100644 --- a/lib/util/http/index.ts +++ b/lib/util/http/index.ts @@ -15,11 +15,7 @@ import { applyAuthorization, removeAuthorization } from './auth'; import { hooks } from './hooks'; import { applyHostRule, findMatchingRule } from './host-rules'; import { getQueue } from './queue'; -import { - extractRetryAfterHeaderSeconds, - getMaxRetryAfter, - wrapWithRetry, -} from './retry-after'; +import { extractRetryAfterHeaderSeconds, wrapWithRetry } from './retry-after'; import { Throttle, getThrottle } from './throttle'; import type { GotJSONOptions, @@ -140,7 +136,7 @@ export class Http { context: { hostType }, retry: { limit: retryLimit, - maxRetryAfter: 0, + maxRetryAfter: 0, // Don't rely on `got` retry-after handling, just let it fail and then we'll handle it }, }); } @@ -234,11 +230,12 @@ export class Http { ? () => queue.add>(throttledTask) : throttledTask; + const { maxRetryAfter = 60 } = hostRule; resPromise = wrapWithRetry( url, queuedTask, extractRetryAfterHeaderSeconds, - getMaxRetryAfter(url), + maxRetryAfter, ); if (memCacheKey) { diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index 034eb5c2e6334a..bdcaced677147a 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -1,7 +1,6 @@ import { setTimeout } from 'timers/promises'; import { RequestError } from 'got'; import { logger } from '../../logger'; -import * as hostRules from '../host-rules'; import { parseUrl } from '../url'; import type { Task } from './types'; @@ -86,8 +85,3 @@ export function extractRetryAfterHeaderSeconds(err: unknown): number | null { return seconds; } - -export function getMaxRetryAfter(url: string): number { - const { maxRetryAfter = 60 } = hostRules.find({ url }); - return maxRetryAfter; -} From 75d23eef2a4b99e6936920d5c3a296726b1ad5d3 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Thu, 30 Nov 2023 20:38:44 -0300 Subject: [PATCH 15/32] Fix --- lib/util/http/retry-after.spec.ts | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/lib/util/http/retry-after.spec.ts b/lib/util/http/retry-after.spec.ts index 1db63900927ecf..2c7aa95927bc53 100644 --- a/lib/util/http/retry-after.spec.ts +++ b/lib/util/http/retry-after.spec.ts @@ -1,10 +1,6 @@ import { RequestError } from 'got'; import * as hostRules from '../host-rules'; -import { - extractRetryAfterHeaderSeconds, - getMaxRetryAfter, - wrapWithRetry, -} from './retry-after'; +import { extractRetryAfterHeaderSeconds, wrapWithRetry } from './retry-after'; function requestError( response: { @@ -141,21 +137,4 @@ describe('util/http/retry-after', () => { ).toBe(42); }); }); - - describe('getMaxRetryAfter', () => { - afterEach(() => { - hostRules.clear(); - }); - - it('returns default value of 60 when host rule is not set', () => { - expect(getMaxRetryAfter('http://example.com')).toBe(60); - }); - - it('returns maxRetryAfter', () => { - hostRules.add({ matchHost: 'foo.com', maxRetryAfter: 111 }); - hostRules.add({ matchHost: 'bar.com', maxRetryAfter: 222 }); - expect(getMaxRetryAfter('http://foo.com')).toBe(111); - expect(getMaxRetryAfter('http://bar.com')).toBe(222); - }); - }); }); From c0c219be1bf16fdd72cfc36474354cdcad9056c0 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Thu, 30 Nov 2023 20:39:19 -0300 Subject: [PATCH 16/32] Fix --- lib/util/http/retry-after.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/util/http/retry-after.spec.ts b/lib/util/http/retry-after.spec.ts index 2c7aa95927bc53..b1c0e9e8361a16 100644 --- a/lib/util/http/retry-after.spec.ts +++ b/lib/util/http/retry-after.spec.ts @@ -1,5 +1,4 @@ import { RequestError } from 'got'; -import * as hostRules from '../host-rules'; import { extractRetryAfterHeaderSeconds, wrapWithRetry } from './retry-after'; function requestError( From 2f94a23e75f45592bd2bb90b987cd7872b17be46 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Thu, 30 Nov 2023 20:48:49 -0300 Subject: [PATCH 17/32] Refactor --- lib/util/http/index.ts | 9 ++------ lib/util/http/retry-after.spec.ts | 35 ++++++++++++++++--------------- lib/util/http/retry-after.ts | 10 ++++----- 3 files changed, 25 insertions(+), 29 deletions(-) diff --git a/lib/util/http/index.ts b/lib/util/http/index.ts index c93553049ce124..c675e378f5692e 100644 --- a/lib/util/http/index.ts +++ b/lib/util/http/index.ts @@ -15,7 +15,7 @@ import { applyAuthorization, removeAuthorization } from './auth'; import { hooks } from './hooks'; import { applyHostRule, findMatchingRule } from './host-rules'; import { getQueue } from './queue'; -import { extractRetryAfterHeaderSeconds, wrapWithRetry } from './retry-after'; +import { getRetryAfter, wrapWithRetry } from './retry-after'; import { Throttle, getThrottle } from './throttle'; import type { GotJSONOptions, @@ -231,12 +231,7 @@ export class Http { : throttledTask; const { maxRetryAfter = 60 } = hostRule; - resPromise = wrapWithRetry( - url, - queuedTask, - extractRetryAfterHeaderSeconds, - maxRetryAfter, - ); + resPromise = wrapWithRetry(queuedTask, url, getRetryAfter, maxRetryAfter); if (memCacheKey) { memCache.set(memCacheKey, resPromise); diff --git a/lib/util/http/retry-after.spec.ts b/lib/util/http/retry-after.spec.ts index b1c0e9e8361a16..fa404529626c59 100644 --- a/lib/util/http/retry-after.spec.ts +++ b/lib/util/http/retry-after.spec.ts @@ -1,5 +1,5 @@ import { RequestError } from 'got'; -import { extractRetryAfterHeaderSeconds, wrapWithRetry } from './retry-after'; +import { getRetryAfter, wrapWithRetry } from './retry-after'; function requestError( response: { @@ -26,7 +26,12 @@ describe('util/http/retry-after', () => { it('works', async () => { const task = jest.fn(() => Promise.resolve(42)); - const res = await wrapWithRetry('http://example.com', task, () => null); + const res = await wrapWithRetry( + task, + 'http://example.com', + () => null, + 60, + ); expect(res).toBe(42); expect(task).toHaveBeenCalledTimes(1); }); @@ -35,7 +40,7 @@ describe('util/http/retry-after', () => { const task = jest.fn(() => Promise.reject(new Error('error'))); await expect( - wrapWithRetry('http://example.com', task, () => null), + wrapWithRetry(task, 'http://example.com', () => null, 60), ).rejects.toThrow('error'); expect(task).toHaveBeenCalledTimes(1); @@ -48,7 +53,7 @@ describe('util/http/retry-after', () => { .mockRejectedValueOnce(new Error('error-2')) .mockResolvedValueOnce(42); - const p = wrapWithRetry('http://example.com', task, () => 1); + const p = wrapWithRetry(task, 'http://example.com', () => 1, 60); await jest.advanceTimersByTimeAsync(2000); const res = await p; @@ -64,7 +69,7 @@ describe('util/http/retry-after', () => { .mockRejectedValueOnce('error-3') .mockRejectedValue('error-4'); - const p = wrapWithRetry('http://example.com', task, () => 1).catch( + const p = wrapWithRetry(task, 'http://example.com', () => 1, 60).catch( (err) => err, ); await jest.advanceTimersByTimeAsync(2000); @@ -76,7 +81,7 @@ describe('util/http/retry-after', () => { it('gives up when delay exceeds maxRetryAfter', async () => { const task = jest.fn().mockRejectedValue('error'); - const p = wrapWithRetry('http://example.com', task, () => 61).catch( + const p = wrapWithRetry(task, 'http://example.com', () => 61, 60).catch( (err) => err, ); @@ -85,34 +90,30 @@ describe('util/http/retry-after', () => { }); }); - describe('extractRetryAfterHeaderSeconds', () => { + describe('getRetryAfter', () => { it('returns null for non-RequestError', () => { - expect(extractRetryAfterHeaderSeconds(new Error())).toBeNull(); + expect(getRetryAfter(new Error())).toBeNull(); }); it('returns null for RequestError without response', () => { - expect(extractRetryAfterHeaderSeconds(requestError())).toBeNull(); + expect(getRetryAfter(requestError())).toBeNull(); }); it('returns null for status other than 429', () => { const err = new RequestError('request-error', {}, null as never); (err as any).response = { statusCode: 302 }; - expect( - extractRetryAfterHeaderSeconds(requestError({ statusCode: 302 })), - ).toBeNull(); + expect(getRetryAfter(requestError({ statusCode: 302 }))).toBeNull(); }); it('returns null missing "retry-after" header', () => { expect( - extractRetryAfterHeaderSeconds( - requestError({ statusCode: 429, headers: {} }), - ), + getRetryAfter(requestError({ statusCode: 429, headers: {} })), ).toBeNull(); }); it('returns null for non-integer "retry-after" header', () => { expect( - extractRetryAfterHeaderSeconds( + getRetryAfter( requestError({ statusCode: 429, headers: { @@ -125,7 +126,7 @@ describe('util/http/retry-after', () => { it('returns delay in seconds', () => { expect( - extractRetryAfterHeaderSeconds( + getRetryAfter( requestError({ statusCode: 429, headers: { diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index bdcaced677147a..fc11b0efb6b159 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -9,10 +9,10 @@ const hostBlocks = new Map>(); const maxRetries = 2; export async function wrapWithRetry( - url: string, task: Task, - getDelaySeconds: (err: unknown) => number | null, - maxRetryAfter = 60, + url: string, + getRetryAfter: (err: unknown) => number | null, + maxRetryAfter: number, ): Promise { const key = parseUrl(url)?.host ?? /* istanbul ignore next */ url; @@ -27,7 +27,7 @@ export async function wrapWithRetry( return await task(); } catch (err) { - const delaySeconds = getDelaySeconds(err); + const delaySeconds = getRetryAfter(err); if (delaySeconds === null) { throw err; } @@ -56,7 +56,7 @@ export async function wrapWithRetry( } } -export function extractRetryAfterHeaderSeconds(err: unknown): number | null { +export function getRetryAfter(err: unknown): number | null { if (!(err instanceof RequestError)) { return null; } From 3996c8a21e12aa7d16f5cbb5e79e30b4c9a889b7 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Fri, 1 Dec 2023 08:55:40 -0300 Subject: [PATCH 18/32] Fix tests --- lib/util/http/host-rules.spec.ts | 388 ++++++++++++------------------- 1 file changed, 154 insertions(+), 234 deletions(-) diff --git a/lib/util/http/host-rules.spec.ts b/lib/util/http/host-rules.spec.ts index bbd34eeac79e35..3c483a8699e3ec 100644 --- a/lib/util/http/host-rules.spec.ts +++ b/lib/util/http/host-rules.spec.ts @@ -53,75 +53,59 @@ describe('util/http/host-rules', () => { it('adds token', () => { const opts = { ...options }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "token", - } - `); - expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` - { - "context": { - "authType": undefined, - }, - "hostType": "github", - "token": "token", - } - `); + expect(hostRule).toEqual({ + token: 'token', + }); + expect(applyHostRule(url, opts, hostRule)).toEqual({ + context: { + authType: undefined, + }, + hostType: 'github', + token: 'token', + }); }); it('adds auth', () => { const opts = { hostType: 'gitea' }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "password": "password", - } - `); - expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` - { - "hostType": "gitea", - "password": "password", - "username": undefined, - } - `); + expect(hostRule).toEqual({ + password: 'password', + }); + expect(applyHostRule(url, opts, hostRule)).toEqual({ + hostType: 'gitea', + password: 'password', + username: undefined, + }); }); it('adds custom auth', () => { const opts = { hostType: 'npm' }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "authType": "Basic", - "timeout": 5000, - "token": "XXX", - } - `); - expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` - { - "context": { - "authType": "Basic", - }, - "hostType": "npm", - "timeout": 5000, - "token": "XXX", - } - `); + expect(hostRule).toEqual({ + authType: 'Basic', + timeout: 5000, + token: 'XXX', + }); + expect(applyHostRule(url, opts, hostRule)).toEqual({ + context: { + authType: 'Basic', + }, + hostType: 'npm', + timeout: 5000, + token: 'XXX', + }); }); it('skips', () => { const opts = { ...options, token: 'xxx' }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "token", - } - `); - expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` - { - "hostType": "github", - "token": "xxx", - } - `); + expect(hostRule).toEqual({ + token: 'token', + }); + expect(applyHostRule(url, opts, hostRule)).toEqual({ + hostType: 'github', + token: 'xxx', + }); }); it('uses http2', () => { @@ -129,19 +113,15 @@ describe('util/http/host-rules', () => { const opts = { ...options, token: 'xxx' }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "enableHttp2": true, - "token": "token", - } - `); - expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` - { - "hostType": "github", - "http2": true, - "token": "xxx", - } - `); + expect(hostRule).toEqual({ + enableHttp2: true, + token: 'token', + }); + expect(applyHostRule(url, opts, hostRule)).toEqual({ + hostType: 'github', + http2: true, + token: 'xxx', + }); }); it('uses dnsCache', () => { @@ -149,12 +129,10 @@ describe('util/http/host-rules', () => { const opts = { ...options, token: 'xxx' }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "dnsCache": true, - "token": "token", - } - `); + expect(hostRule).toEqual({ + dnsCache: true, + token: 'token', + }); expect(applyHostRule(url, opts, hostRule)).toMatchObject({ hostType: 'github', lookup: dnsLookup, @@ -167,12 +145,10 @@ describe('util/http/host-rules', () => { const opts = { ...options, token: 'xxx' }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "keepAlive": true, - "token": "token", - } - `); + expect(hostRule).toEqual({ + keepAlive: true, + token: 'token', + }); expect(applyHostRule(url, opts, hostRule).agent).toBeDefined(); }); @@ -183,34 +159,26 @@ describe('util/http/host-rules', () => { const opts = { ...options, token: 'xxx' }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "enableHttp2": true, - "token": "token", - } - `); - expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` - { - "hostType": "github", - "token": "xxx", - } - `); + expect(hostRule).toEqual({ + enableHttp2: true, + token: 'token', + }); + expect(applyHostRule(url, opts, hostRule)).toEqual({ + hostType: 'github', + token: 'xxx', + }); }); it('noAuth', () => { const opts = { ...options, noAuth: true }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "token", - } - `); - expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` - { - "hostType": "github", - "noAuth": true, - } - `); + expect(hostRule).toEqual({ + token: 'token', + }); + expect(applyHostRule(url, opts, hostRule)).toEqual({ + hostType: 'github', + noAuth: true, + }); }); it('certificateAuthority', () => { @@ -223,19 +191,15 @@ describe('util/http/host-rules', () => { const url = 'https://custom.datasource.ca/data/path'; const opts = { ...options, hostType: 'maven' }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "httpsCertificateAuthority": "ca-cert", - } - `); - expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` - { - "hostType": "maven", - "https": { - "certificateAuthority": "ca-cert", - }, - } - `); + expect(hostRule).toEqual({ + httpsCertificateAuthority: 'ca-cert', + }); + expect(applyHostRule(url, opts, hostRule)).toEqual({ + hostType: 'maven', + https: { + certificateAuthority: 'ca-cert', + }, + }); }); it('privateKey', () => { @@ -248,19 +212,15 @@ describe('util/http/host-rules', () => { const url = 'https://custom.datasource.key/data/path'; const opts = { ...options, hostType: 'maven' }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "httpsPrivateKey": "key", - } - `); - expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` - { - "hostType": "maven", - "https": { - "key": "key", - }, - } - `); + expect(hostRule).toEqual({ + httpsPrivateKey: 'key', + }); + expect(applyHostRule(url, opts, hostRule)).toEqual({ + hostType: 'maven', + https: { + key: 'key', + }, + }); }); it('certificate', () => { @@ -273,19 +233,15 @@ describe('util/http/host-rules', () => { const url = 'https://custom.datasource.cert/data/path'; const opts = { ...options, hostType: 'maven' }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "httpsCertificate": "cert", - } - `); - expect(applyHostRule(url, opts, hostRule)).toMatchInlineSnapshot(` - { - "hostType": "maven", - "https": { - "certificate": "cert", - }, - } - `); + expect(hostRule).toEqual({ + httpsCertificate: 'cert', + }); + expect(applyHostRule(url, opts, hostRule)).toEqual({ + hostType: 'maven', + https: { + certificate: 'cert', + }, + }); }); it('no fallback to github', () => { @@ -313,12 +269,10 @@ describe('util/http/host-rules', () => { opts = { ...options, hostType: 'github-releases' }; hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "password": "xxx", - "username": "some", - } - `); + expect(hostRule).toEqual({ + password: 'xxx', + username: 'some', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ hostType: 'github-releases', username: 'some', @@ -327,12 +281,10 @@ describe('util/http/host-rules', () => { opts = { ...options, hostType: 'github-tags' }; hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "password": "xxx2", - "username": "some2", - } - `); + expect(hostRule).toEqual({ + password: 'xxx2', + username: 'some2', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ hostType: 'github-tags', username: 'some2', @@ -341,11 +293,9 @@ describe('util/http/host-rules', () => { opts = { ...options, hostType: 'pod' }; hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "pod-token", - } - `); + expect(hostRule).toEqual({ + token: 'pod-token', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, @@ -356,11 +306,9 @@ describe('util/http/host-rules', () => { opts = { ...options, hostType: 'github-changelog' }; hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "changelogtoken", - } - `); + expect(hostRule).toEqual({ + token: 'changelogtoken', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, @@ -376,11 +324,9 @@ describe('util/http/host-rules', () => { opts = { ...options, hostType: 'github-tags' }; hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "token", - } - `); + expect(hostRule).toEqual({ + token: 'token', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, @@ -391,11 +337,9 @@ describe('util/http/host-rules', () => { opts = { ...options, hostType: 'github-changelog' }; hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "token", - } - `); + expect(hostRule).toEqual({ + token: 'token', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, @@ -406,11 +350,9 @@ describe('util/http/host-rules', () => { opts = { ...options, hostType: 'pod' }; hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "token", - } - `); + expect(hostRule).toEqual({ + token: 'token', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, @@ -439,11 +381,9 @@ describe('util/http/host-rules', () => { opts = { ...options, hostType: 'gitlab-tags' }; hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "tags-token", - } - `); + expect(hostRule).toEqual({ + token: 'tags-token', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, @@ -454,11 +394,9 @@ describe('util/http/host-rules', () => { opts = { ...options, hostType: 'gitlab-releases' }; hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "release-token", - } - `); + expect(hostRule).toEqual({ + token: 'release-token', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, @@ -469,11 +407,9 @@ describe('util/http/host-rules', () => { opts = { ...options, hostType: 'gitlab-packages' }; hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "package-token", - } - `); + expect(hostRule).toEqual({ + token: 'package-token', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, @@ -489,11 +425,9 @@ describe('util/http/host-rules', () => { opts = { ...options, hostType: 'gitlab-tags' }; hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "abc", - } - `); + expect(hostRule).toEqual({ + token: 'abc', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, @@ -504,11 +438,9 @@ describe('util/http/host-rules', () => { opts = { ...options, hostType: 'gitlab-releases' }; hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "abc", - } - `); + expect(hostRule).toEqual({ + token: 'abc', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, @@ -519,11 +451,9 @@ describe('util/http/host-rules', () => { opts = { ...options, hostType: 'gitlab-packages' }; hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "abc", - } - `); + expect(hostRule).toEqual({ + token: 'abc', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, @@ -534,11 +464,9 @@ describe('util/http/host-rules', () => { opts = { ...options, hostType: 'gitlab-changelog' }; hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "abc", - } - `); + expect(hostRule).toEqual({ + token: 'abc', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, @@ -556,12 +484,10 @@ describe('util/http/host-rules', () => { }); const opts = { ...options, hostType: 'bitbucket-tags' }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "password": "xxx", - "username": "some", - } - `); + expect(hostRule).toEqual({ + password: 'xxx', + username: 'some', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ hostType: 'bitbucket-tags', username: 'some', @@ -572,11 +498,9 @@ describe('util/http/host-rules', () => { it('fallback to bitbucket', () => { const opts = { ...options, hostType: 'bitbucket-tags' }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "cdef", - } - `); + expect(hostRule).toEqual({ + token: 'cdef', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, @@ -594,11 +518,9 @@ describe('util/http/host-rules', () => { const opts = { ...options, hostType: 'gitea-tags' }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "token": "abc", - } - `); + expect(hostRule).toEqual({ + token: 'abc', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ context: { authType: undefined, @@ -611,11 +533,9 @@ describe('util/http/host-rules', () => { it('fallback to gitea', () => { const opts = { ...options, hostType: 'gitea-tags' }; const hostRule = findMatchingRule(url, opts); - expect(hostRule).toMatchInlineSnapshot(` - { - "password": "password", - } - `); + expect(hostRule).toEqual({ + password: 'password', + }); expect(applyHostRule(url, opts, hostRule)).toEqual({ hostType: 'gitea-tags', password: 'password', From 53c8a4d4c2076dbaae2acc7f90aa29ec139cf020 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Fri, 1 Dec 2023 12:33:27 -0300 Subject: [PATCH 19/32] Handle date format --- lib/util/http/retry-after.spec.ts | 43 +++++++++++++++++++++++++------ lib/util/http/retry-after.ts | 32 +++++++++++++++++------ 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/lib/util/http/retry-after.spec.ts b/lib/util/http/retry-after.spec.ts index fa404529626c59..5291a7840a2c8e 100644 --- a/lib/util/http/retry-after.spec.ts +++ b/lib/util/http/retry-after.spec.ts @@ -15,15 +15,15 @@ function requestError( } describe('util/http/retry-after', () => { - describe('wrapWithRetry', () => { - beforeEach(() => { - jest.useFakeTimers({ advanceTimers: true }); - }); + beforeEach(() => { + jest.useFakeTimers(); + }); - afterEach(() => { - jest.useRealTimers(); - }); + afterEach(() => { + jest.useRealTimers(); + }); + describe('wrapWithRetry', () => { it('works', async () => { const task = jest.fn(() => Promise.resolve(42)); const res = await wrapWithRetry( @@ -124,7 +124,21 @@ describe('util/http/retry-after', () => { ).toBeNull(); }); - it('returns delay in seconds', () => { + it('returns delay in seconds from date', () => { + jest.setSystemTime(new Date('2020-01-01T00:00:00Z')); + expect( + getRetryAfter( + requestError({ + statusCode: 429, + headers: { + 'retry-after': 'Wed, 01 Jan 2020 00:00:42 GMT', + }, + }), + ), + ).toBe(42); + }); + + it('returns delay in seconds from number', () => { expect( getRetryAfter( requestError({ @@ -136,5 +150,18 @@ describe('util/http/retry-after', () => { ), ).toBe(42); }); + + it('returns null for invalid header value', () => { + expect( + getRetryAfter( + requestError({ + statusCode: 429, + headers: { + 'retry-after': 'invalid', + }, + }), + ), + ).toBeNull(); + }); }); }); diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index fc11b0efb6b159..ba8abad6f3957b 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -3,6 +3,8 @@ import { RequestError } from 'got'; import { logger } from '../../logger'; import { parseUrl } from '../url'; import type { Task } from './types'; +import { DateTime } from 'luxon'; +import { regEx } from '../regex'; const hostBlocks = new Map>(); @@ -69,19 +71,33 @@ export function getRetryAfter(err: unknown): number | null { return null; } - const retryAfter = err.response.headers['retry-after']; + const retryAfter = err.response.headers['retry-after']?.trim(); if (!retryAfter) { return null; } + const date = DateTime.fromHTTP(retryAfter); + if (date.isValid) { + const seconds = Math.floor(date.diffNow('seconds').seconds); + if (seconds < 0) { + logger.debug( + { url: err.response.url, retryAfter }, + 'Retry-After: date in the past', + ); + return null; + } + + return seconds; + } + const seconds = parseInt(retryAfter, 10); - if (Number.isNaN(seconds)) { - logger.debug( - { url: err.response.url, retryAfter }, - 'Retry-After: unsupported format', - ); - return null; + if (!Number.isNaN(seconds)) { + return seconds; } - return seconds; + logger.debug( + { url: err.response.url, retryAfter }, + 'Retry-After: unsupported format', + ); + return null; } From 598aad2386a7e92e69cff7e7ab98557538655647 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Fri, 1 Dec 2023 12:43:55 -0300 Subject: [PATCH 20/32] Fix lint --- lib/util/http/retry-after.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index ba8abad6f3957b..907065ec285d58 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -1,10 +1,9 @@ import { setTimeout } from 'timers/promises'; import { RequestError } from 'got'; +import { DateTime } from 'luxon'; import { logger } from '../../logger'; import { parseUrl } from '../url'; import type { Task } from './types'; -import { DateTime } from 'luxon'; -import { regEx } from '../regex'; const hostBlocks = new Map>(); From 919f570a0226620cde69ed8f32394df51943b201 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Fri, 1 Dec 2023 22:57:23 -0300 Subject: [PATCH 21/32] Delay accumulation --- lib/util/http/retry-after.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index 907065ec285d58..80bfbb51d9c238 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -5,7 +5,7 @@ import { logger } from '../../logger'; import { parseUrl } from '../url'; import type { Task } from './types'; -const hostBlocks = new Map>(); +const hostBlocks = new Map>(); const maxRetries = 2; @@ -51,7 +51,12 @@ export async function wrapWithRetry( `Retry-After: will retry ${url} after ${delaySeconds} seconds`, ); - hostBlocks.set(key, setTimeout(1000 * delaySeconds)); + const existingDelay = hostBlocks.get(key); + const newDelay = setTimeout(1000 * delaySeconds); + const delayPromise = existingDelay + ? Promise.all([existingDelay, newDelay]) + : newDelay; + hostBlocks.set(key, delayPromise); retries += 1; } } From e9fcc16e7e7d5ed3f796cd959dd9c4727bfe9bfb Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Fri, 1 Dec 2023 22:58:01 -0300 Subject: [PATCH 22/32] Refactor --- lib/util/http/retry-after.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index 80bfbb51d9c238..74f395f46de975 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -53,10 +53,10 @@ export async function wrapWithRetry( const existingDelay = hostBlocks.get(key); const newDelay = setTimeout(1000 * delaySeconds); - const delayPromise = existingDelay + const accumulatedDelay = existingDelay ? Promise.all([existingDelay, newDelay]) : newDelay; - hostBlocks.set(key, delayPromise); + hostBlocks.set(key, accumulatedDelay); retries += 1; } } From 125d1ea1b4d7a0328b57dc7485cf959cdaae186f Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sat, 2 Dec 2023 07:46:01 -0300 Subject: [PATCH 23/32] Fix --- lib/util/http/retry-after.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index 74f395f46de975..dddbe52824a7e2 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -51,12 +51,11 @@ export async function wrapWithRetry( `Retry-After: will retry ${url} after ${delaySeconds} seconds`, ); - const existingDelay = hostBlocks.get(key); - const newDelay = setTimeout(1000 * delaySeconds); - const accumulatedDelay = existingDelay - ? Promise.all([existingDelay, newDelay]) - : newDelay; - hostBlocks.set(key, accumulatedDelay); + const delay = Promise.all([ + hostBlocks.get(key), + setTimeout(1000 * delaySeconds), + ]); + hostBlocks.set(key, delay); retries += 1; } } From 38471ceb8bd5bb79eb674b56435aa2d424301ea4 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sat, 2 Dec 2023 12:27:46 -0300 Subject: [PATCH 24/32] Refactor --- lib/util/http/retry-after.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index dddbe52824a7e2..edb831a81eae6b 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -5,7 +5,7 @@ import { logger } from '../../logger'; import { parseUrl } from '../url'; import type { Task } from './types'; -const hostBlocks = new Map>(); +const hostDelays = new Map>(); const maxRetries = 2; @@ -20,11 +20,8 @@ export async function wrapWithRetry( let retries = 0; for (;;) { try { - const delayPromise = hostBlocks.get(key); - if (delayPromise) { - await delayPromise; - hostBlocks.delete(key); - } + await hostDelays.get(key); + hostDelays.delete(key); return await task(); } catch (err) { @@ -52,10 +49,10 @@ export async function wrapWithRetry( ); const delay = Promise.all([ - hostBlocks.get(key), + hostDelays.get(key), setTimeout(1000 * delaySeconds), ]); - hostBlocks.set(key, delay); + hostDelays.set(key, delay); retries += 1; } } From 02e378ffebb270c63b641fc41e1373bbc55b658a Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sat, 2 Dec 2023 17:01:44 -0300 Subject: [PATCH 25/32] Update lib/util/http/retry-after.ts Co-authored-by: Michael Kriese --- lib/util/http/retry-after.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index edb831a81eae6b..8a766dee48cc68 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -91,7 +91,7 @@ export function getRetryAfter(err: unknown): number | null { } const seconds = parseInt(retryAfter, 10); - if (!Number.isNaN(seconds)) { + if (!Number.isNaN(seconds) && seconds > 0) { return seconds; } From 1a47277ba9edd38d95801338d47d2a1363c93c37 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sun, 3 Dec 2023 20:48:36 -0300 Subject: [PATCH 26/32] Add comment and clarify status codes --- docs/usage/configuration-options.md | 2 +- lib/util/http/retry-after.ts | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 0e0bad658f230e..354228461aae88 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -1765,7 +1765,7 @@ Example config: ### maxRetryAfter -A remote host may return a `4xx` response with a `Retry-After` header value, which indicates that Renovate has been rate-limited. +A remote host may return a `429`/`403` response with a `Retry-After` header value, which indicates that Renovate has been rate-limited. Renovate may try to contact the host again after waiting a certain time, that's set by the host. By default, Renovate tries again after the `Retry-After` header value has passed, up to a maximum of 60 seconds. If the `Retry-After` value is more than 60 seconds, Renovate will abort the request instead of waiting. diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index 8a766dee48cc68..52c64109f79d4d 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -9,6 +9,14 @@ const hostDelays = new Map>(); const maxRetries = 2; +/** + * Given a task that returns a promise, retry the task if it fails with a + * 429 Too Many Requests or 403 Forbidden response, using the Retry-After + * header to determine the delay. + * + * For response codes other than 429 or 403, or if the Retry-After header + * is not present or invalid, the task is not retried, throwing the error. + */ export async function wrapWithRetry( task: Task, url: string, From 959b31cf57dc7c384429ce4cff918899ca42dc27 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Sun, 3 Dec 2023 20:49:31 -0300 Subject: [PATCH 27/32] Fix comment --- docs/usage/configuration-options.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 354228461aae88..6efde3f3018d80 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -1765,7 +1765,7 @@ Example config: ### maxRetryAfter -A remote host may return a `429`/`403` response with a `Retry-After` header value, which indicates that Renovate has been rate-limited. +A remote host may return a `429` or `403` response with a `Retry-After` header value, which indicates that Renovate has been rate-limited. Renovate may try to contact the host again after waiting a certain time, that's set by the host. By default, Renovate tries again after the `Retry-After` header value has passed, up to a maximum of 60 seconds. If the `Retry-After` value is more than 60 seconds, Renovate will abort the request instead of waiting. From b0fc1d1230417737420c69a2e8407d6e7e43f096 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Mon, 4 Dec 2023 13:05:08 -0300 Subject: [PATCH 28/32] 4xx --- docs/usage/configuration-options.md | 2 +- lib/util/http/retry-after.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 6efde3f3018d80..0e0bad658f230e 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -1765,7 +1765,7 @@ Example config: ### maxRetryAfter -A remote host may return a `429` or `403` response with a `Retry-After` header value, which indicates that Renovate has been rate-limited. +A remote host may return a `4xx` response with a `Retry-After` header value, which indicates that Renovate has been rate-limited. Renovate may try to contact the host again after waiting a certain time, that's set by the host. By default, Renovate tries again after the `Retry-After` header value has passed, up to a maximum of 60 seconds. If the `Retry-After` value is more than 60 seconds, Renovate will abort the request instead of waiting. diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index 52c64109f79d4d..517208a3a3853c 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -75,7 +75,7 @@ export function getRetryAfter(err: unknown): number | null { return null; } - if (err.response.statusCode !== 429 && err.response.statusCode !== 403) { + if (err.response.statusCode < 400 || err.response.statusCode >= 500) { return null; } From 240f63de6a6ad3319abe362e3d9cdf2fdcd550a8 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Mon, 4 Dec 2023 17:51:46 -0300 Subject: [PATCH 29/32] Fix coverage --- lib/util/http/retry-after.spec.ts | 2 +- lib/util/http/retry-after.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/util/http/retry-after.spec.ts b/lib/util/http/retry-after.spec.ts index 5291a7840a2c8e..59f7bbeee46106 100644 --- a/lib/util/http/retry-after.spec.ts +++ b/lib/util/http/retry-after.spec.ts @@ -28,7 +28,7 @@ describe('util/http/retry-after', () => { const task = jest.fn(() => Promise.resolve(42)); const res = await wrapWithRetry( task, - 'http://example.com', + 'foobar', () => null, 60, ); diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index 517208a3a3853c..9d1324ea09c1af 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -23,7 +23,7 @@ export async function wrapWithRetry( getRetryAfter: (err: unknown) => number | null, maxRetryAfter: number, ): Promise { - const key = parseUrl(url)?.host ?? /* istanbul ignore next */ url; + const key = parseUrl(url)?.host ?? url; let retries = 0; for (;;) { From 751fa9a1b0d98ad6d8bc0222de232b0295dc528b Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Mon, 4 Dec 2023 17:54:26 -0300 Subject: [PATCH 30/32] Fix lint --- lib/util/http/retry-after.spec.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/util/http/retry-after.spec.ts b/lib/util/http/retry-after.spec.ts index 59f7bbeee46106..9535cfabb3328d 100644 --- a/lib/util/http/retry-after.spec.ts +++ b/lib/util/http/retry-after.spec.ts @@ -26,12 +26,7 @@ describe('util/http/retry-after', () => { describe('wrapWithRetry', () => { it('works', async () => { const task = jest.fn(() => Promise.resolve(42)); - const res = await wrapWithRetry( - task, - 'foobar', - () => null, - 60, - ); + const res = await wrapWithRetry(task, 'foobar', () => null, 60); expect(res).toBe(42); expect(task).toHaveBeenCalledTimes(1); }); From 3356484ce6f682e812917e3e5047366207df5ac8 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Tue, 5 Dec 2023 14:29:33 -0300 Subject: [PATCH 31/32] Add logger --- lib/util/http/retry-after.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/util/http/retry-after.ts b/lib/util/http/retry-after.ts index 9d1324ea09c1af..6cde4ae33a1d85 100644 --- a/lib/util/http/retry-after.ts +++ b/lib/util/http/retry-after.ts @@ -76,6 +76,10 @@ export function getRetryAfter(err: unknown): number | null { } if (err.response.statusCode < 400 || err.response.statusCode >= 500) { + logger.warn( + { url: err.response.url }, + `Retry-After: unexpected status code ${err.response.statusCode}`, + ); return null; } From fc6fe79aa072b624b1bb7d182a42a1a4bf4165ff Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Wed, 6 Dec 2023 08:41:14 -0300 Subject: [PATCH 32/32] Update docs/usage/configuration-options.md Co-authored-by: Sebastian Poxhofer --- docs/usage/configuration-options.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 53fa79e0b26908..268c2584346bc0 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -1770,7 +1770,7 @@ Renovate may try to contact the host again after waiting a certain time, that's By default, Renovate tries again after the `Retry-After` header value has passed, up to a maximum of 60 seconds. If the `Retry-After` value is more than 60 seconds, Renovate will abort the request instead of waiting. -You can configure a different maximum value using `maxRetryAfter`: +You can configure a different maximum value in seconds using `maxRetryAfter`: ```json {