diff --git a/README.md b/README.md index e6b8bf6..ff26789 100644 --- a/README.md +++ b/README.md @@ -94,6 +94,10 @@ with defaults set to max of 3 retries and initial Delay as 100ms

createFetch([proxyAuthOptions])function

Return the appropriate Fetch function depending on proxy settings.

+
parseRetryAfterHeader(header)number
+

Parse the Retry-After header +Spec: https://tools.ietf.org/html/rfc7231#section-7.1.3

+
## Typedefs @@ -141,7 +145,6 @@ This provides a wrapper for fetch that facilitates proxy auth authorization. * [ProxyFetch](#ProxyFetch) * [new ProxyFetch(authOptions)](#new_ProxyFetch_new) - * [.proxyAgent()](#ProxyFetch+proxyAgent) ⇒ http.Agent * [.fetch(resource, options)](#ProxyFetch+fetch) ⇒ Promise.<Response> @@ -154,13 +157,6 @@ Initialize this class with Proxy auth options | --- | --- | --- | | authOptions | [ProxyAuthOptions](#ProxyAuthOptions) | the auth options to connect with | - - -### proxyFetch.proxyAgent() ⇒ http.Agent -Returns the http.Agent used for this proxy - -**Kind**: instance method of [ProxyFetch](#ProxyFetch) -**Returns**: http.Agent - a http.Agent for basic auth proxy ### proxyFetch.fetch(resource, options) ⇒ Promise.<Response> @@ -186,6 +182,19 @@ Return the appropriate Fetch function depending on proxy settings. | --- | --- | --- | | [proxyAuthOptions] | [ProxyAuthOptions](#ProxyAuthOptions) | the proxy auth options | + + +## parseRetryAfterHeader(header) ⇒ number +Parse the Retry-After header +Spec: [https://tools.ietf.org/html/rfc7231#section-7.1.3](https://tools.ietf.org/html/rfc7231#section-7.1.3) + +**Kind**: global function +**Returns**: number - Number of milliseconds to sleep until the next call to getEventsFromJournal + +| Param | Type | Description | +| --- | --- | --- | +| header | string | Retry-After header value | + ## RetryOptions : object diff --git a/src/HttpExponentialBackoff.js b/src/HttpExponentialBackoff.js index 0d7cb72..188367f 100644 --- a/src/HttpExponentialBackoff.js +++ b/src/HttpExponentialBackoff.js @@ -13,7 +13,7 @@ const DEFAULT_MAX_RETRIES = 3 const DEFAULY_INITIAL_DELAY_MS = 100 const loggerNamespace = '@adobe/aio-lib-core-networking:HttpExponentialBackoff' const logger = require('@adobe/aio-lib-core-logging')(loggerNamespace, { level: process.env.LOG_LEVEL }) -const { createFetch } = require('./utils') +const { createFetch, parseRetryAfterHeader } = require('./utils') /* global Request, Response, ProxyAuthOptions */ // for linter @@ -89,7 +89,7 @@ class HttpExponentialBackoff { __getRetryOptions (retries, initialDelayInMillis) { return { retryOn: this.__getRetryOn(retries), - retryDelay: this.__getRetryDelay(initialDelayInMillis) + retryDelay: this.__getRetryDelayWithRetryAfterHeader(initialDelayInMillis) } } @@ -103,7 +103,7 @@ class HttpExponentialBackoff { */ __getRetryOn (retries) { return function (attempt, error, response) { - if (attempt < retries && (error !== null || (response.status > 499 && response.status < 600))) { + if (attempt < retries && (error !== null || (response.status > 499 && response.status < 600) || response.status === 429)) { const msg = `Retrying after attempt ${attempt + 1}. failed: ${error || response.statusText}` logger.debug(msg) return true @@ -127,6 +127,27 @@ class HttpExponentialBackoff { return timeToWait } } + + /** + * Retry Delay returns a function that either: + * - return the value of Retry-After header, if present + * - implements exponential backoff otherwise + * + * @param {number} initialDelayInMillis The multiplying factor and the initial delay. Eg. 100 would mean the retries would be spaced at 100, 200, 400, .. ms + * @returns {Function} retryDelayFunction {function(*=, *, *): number} + * @private + */ + __getRetryDelayWithRetryAfterHeader (initialDelayInMillis) { + return (attempt, error, response) => { + const retryAfter = response.headers.get('Retry-After') + const timeToWait = parseRetryAfterHeader(retryAfter) + if (!isNaN(timeToWait)) { + logger.debug(`Request will be retried after ${timeToWait} ms`) + return timeToWait + } + return this.__getRetryDelay(initialDelayInMillis)(attempt, error, response) + } + } } module.exports = HttpExponentialBackoff diff --git a/src/utils.js b/src/utils.js index 3784a96..666337f 100644 --- a/src/utils.js +++ b/src/utils.js @@ -82,7 +82,33 @@ function urlToHttpOptions (aUrl) { return options } +/** + * Parse the Retry-After header + * Spec: {@link https://tools.ietf.org/html/rfc7231#section-7.1.3} + * + * @param {string} header Retry-After header value + * @returns {number} Number of milliseconds to sleep until the next call to getEventsFromJournal + */ +function parseRetryAfterHeader (header) { + if (header == null) { + return NaN + } + if (header.match(/^[0-9]+$/)) { + const delta = parseInt(header, 10) * 1000 + return delta <= 0 ? NaN : delta + } + if (header.match(/^-[0-9]+$/)) { + return NaN + } + const dateMs = Date.parse(header) + const delta = dateMs - Date.now() + return isNaN(delta) || delta <= 0 + ? NaN + : delta +} + module.exports = { urlToHttpOptions, - createFetch + createFetch, + parseRetryAfterHeader } diff --git a/test/HttpExponentialBackoff.test.js b/test/HttpExponentialBackoff.test.js index f8b4967..763e01a 100644 --- a/test/HttpExponentialBackoff.test.js +++ b/test/HttpExponentialBackoff.test.js @@ -12,6 +12,7 @@ governing permissions and limitations under the License. const HttpExponentialBackoff = require('../src/HttpExponentialBackoff') const fetchClient = new HttpExponentialBackoff() const fetchMock = require('node-fetch') +const { parseRetryAfterHeader } = require('../src/utils') jest.mock('node-fetch') /** @@ -25,7 +26,7 @@ jest.mock('node-fetch') */ function __testRetryOnHelper (retries, low = 499, high = 600) { return jest.fn().mockImplementation(function (attempt, error, response) { - if (attempt < retries && (error !== null || (response.status > low && response.status < high))) { + if (attempt < retries && (error !== null || (response.status > low && response.status < high) || response.status === 429)) { return true } return false @@ -41,6 +42,11 @@ function __testRetryOnHelper (retries, low = 499, high = 600) { */ function __testRetryDelayHelper (initialDelay) { return jest.fn().mockImplementation(function (attempt, error, response) { + const retryAfter = response.headers.get('Retry-After') + const timeToWait = parseRetryAfterHeader(retryAfter) + if (!isNaN(timeToWait)) { + return timeToWait + } return attempt * initialDelay// 1000, 2000, 4000 }) } @@ -107,6 +113,19 @@ test('test exponentialBackoff with no retries on 4xx errors and default retry st retrySpy.mockRestore() }) +test('test exponentialBackoff with 3 retries on 429 errors and default retry strategy', async () => { + const mockDefaultFn = __testRetryOnHelper(3) + const retrySpy = jest.spyOn(fetchClient, '__getRetryOn').mockImplementation((retries) => mockDefaultFn) + fetchMock.mockResponse('429 Too many requests', { + status: 429 + }) + const result = await fetchClient.exponentialBackoff('https://abc1.com/', { method: 'GET' }, { initialDelayInMillis: 10 }) + expect(result.status).toBe(429) + expect(retrySpy).toHaveBeenCalledWith(3) + expect(mockDefaultFn).toHaveBeenCalledTimes(4) + retrySpy.mockRestore() +}) + test('test exponentialBackoff with 3 retries on 5xx errors and default retry strategy', async () => { const mockDefaultFn = __testRetryOnHelper(3) const retrySpy = jest.spyOn(fetchClient, '__getRetryOn').mockImplementation((retries) => { @@ -122,6 +141,20 @@ test('test exponentialBackoff with 3 retries on 5xx errors and default retry str retrySpy.mockRestore() }) +test('test exponentialBackoff with 3 retries on errors with default retry strategy and date in Retry-After header', async () => { + const spy = jest.spyOn(global.Date, 'now').mockImplementation(() => new Date('Mon, 13 Feb 2023 23:59:59 GMT')) + const header = 'Tue, 14 Feb 2023 00:00:00 GMT' + fetchMock.mockResponse('503 Service Unavailable', { + status: 503, + headers: { + 'Retry-After': header + } + }) + const result = await fetchClient.exponentialBackoff('https://abc2.com/', { method: 'GET' }, { maxRetries: 2 }) + expect(result.status).toBe(503) + expect(spy).toHaveBeenCalledTimes(2) +}) + test('test exponential backoff with success in first attempt and custom retryOptions', async () => { const mockDefaultFn = __testRetryOnHelper(2) const retrySpy = jest.spyOn(fetchClient, '__getRetryOn').mockImplementation((retries) => { @@ -215,7 +248,8 @@ test('test exponentialBackoff with default 3 retries on 5xx errors and custom re test('test exponentialBackoff with 3 retries on 5xx errors and custom retryDelay', async () => { const mockDefaultFn1 = __testRetryDelayHelper(100) fetchMock.mockResponse('503 Service Unavailable', { - status: 503 + status: 503, + headers: {} }) const result = await fetchClient.exponentialBackoff('https://abc2.com/', { method: 'GET' }, { maxRetries: 2 }, undefined, mockDefaultFn1) expect(result.status).toBe(503) diff --git a/test/utils.test.js b/test/utils.test.js index 06dc37d..f383a3a 100644 --- a/test/utils.test.js +++ b/test/utils.test.js @@ -9,7 +9,7 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ -const { urlToHttpOptions, createFetch } = require('../src/utils') +const { urlToHttpOptions, createFetch, parseRetryAfterHeader } = require('../src/utils') const { ProxyFetch } = require('../src/index') const { getProxyForUrl } = require('proxy-from-env') @@ -21,6 +21,7 @@ jest.mock('node-fetch') test('exports', () => { expect(typeof urlToHttpOptions).toEqual('function') expect(typeof createFetch).toEqual('function') + expect(typeof parseRetryAfterHeader).toEqual('function') }) test('url test (undefined)', () => { @@ -125,3 +126,38 @@ describe('createFetch', () => { expect(response.status).toEqual(result.status) }) }) + +describe('parseRetryAfterHeader', () => { + test('null retry after', () => { + const header = 'null' + expect(parseRetryAfterHeader(header)).toEqual(NaN) + }) + test('positive integer retry-after header', () => { + const header = '23' + expect(parseRetryAfterHeader(header)).toEqual(23000) + }) + test('negative integer retry-after header', () => { + const header = '-23' + expect(parseRetryAfterHeader(header)).toEqual(NaN) + }) + test('retry-after header is 0', () => { + const header = '0' + expect(parseRetryAfterHeader(header)).toEqual(NaN) + }) + test('date retry-after header', () => { + const spy = jest.spyOn(global.Date, 'now').mockImplementation(() => new Date('Mon, 13 Feb 2023 23:59:59 GMT')) + const header = 'Tue, 14 Feb 2023 00:00:00 GMT' + expect(parseRetryAfterHeader(header)).toEqual(1000) + expect(spy).toHaveBeenCalled() + }) + test('date retry-after header older than now', () => { + const spy = jest.spyOn(global.Date, 'now').mockImplementation(() => new Date('Tue, 14 Feb 2023 00:00:00 GMT')) + const header = 'Mon, 13 Feb 2023 23:59:59 GMT' + expect(parseRetryAfterHeader(header)).toEqual(NaN) + expect(spy).toHaveBeenCalled() + }) + test('invalid retry-after header', () => { + const header = 'not::a::date' + expect(parseRetryAfterHeader(header)).toEqual(NaN) + }) +}) diff --git a/types.d.ts b/types.d.ts index f8a8b20..62945a8 100644 --- a/types.d.ts +++ b/types.d.ts @@ -45,11 +45,6 @@ declare type ProxyAuthOptions = { */ declare class ProxyFetch { constructor(authOptions: ProxyAuthOptions); - /** - * Returns the http.Agent used for this proxy - * @returns a http.Agent for basic auth proxy - */ - proxyAgent(): http.Agent; /** * Fetch function, using the configured NTLM Auth options. * @param resource - the url or Request object to fetch from @@ -66,3 +61,11 @@ declare class ProxyFetch { */ declare function createFetch(proxyAuthOptions?: ProxyAuthOptions): (...params: any[]) => any; +/** + * Parse the Retry-After header + * Spec: {@link https://tools.ietf.org/html/rfc7231#section-7.1.3} + * @param header - Retry-After header value + * @returns Number of milliseconds to sleep until the next call to getEventsFromJournal + */ +declare function parseRetryAfterHeader(header: string): number; +