feat: add retry-after header support (#22)

Co-authored-by: Nicola Dardanis <ndardanis@adobe.com>
Co-authored-by: Shazron Abdullah <36107+shazron@users.noreply.github.com>
Co-authored-by: Sangeetha Krishnan <sangeetha5491@gmail.com>

README.md

@@ -94,6 +94,10 @@ with defaults set to max of 3 retries and initial Delay as 100ms</p>
 <dt><a href="#createFetch">createFetch([proxyAuthOptions])</a> ⇒ <code>function</code></dt>
 <dd><p>Return the appropriate Fetch function depending on proxy settings.</p>
 </dd>
+<dt><a href="#parseRetryAfterHeader">parseRetryAfterHeader(header)</a> ⇒ <code>number</code></dt>
+<dd><p>Parse the Retry-After header
+Spec: <a href="https://tools.ietf.org/html/rfc7231#section-7.1.3">https://tools.ietf.org/html/rfc7231#section-7.1.3</a></p>
+</dd>
 </dl>
 
 ## 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) ⇒ <code>http.Agent</code>
     * [.fetch(resource, options)](#ProxyFetch+fetch) ⇒ <code>Promise.&lt;Response&gt;</code>
 
 <a name="new_ProxyFetch_new"></a>
@@ -154,13 +157,6 @@ Initialize this class with Proxy auth options
 | --- | --- | --- |
 | authOptions | [<code>ProxyAuthOptions</code>](#ProxyAuthOptions) | the auth options to connect with |
 
-<a name="ProxyFetch+proxyAgent"></a>
-
-### proxyFetch.proxyAgent() ⇒ <code>http.Agent</code>
-Returns the http.Agent used for this proxy
-
-**Kind**: instance method of [<code>ProxyFetch</code>](#ProxyFetch)  
-**Returns**: <code>http.Agent</code> - a http.Agent for basic auth proxy  
 <a name="ProxyFetch+fetch"></a>
 
 ### proxyFetch.fetch(resource, options) ⇒ <code>Promise.&lt;Response&gt;</code>
@@ -186,6 +182,19 @@ Return the appropriate Fetch function depending on proxy settings.
 | --- | --- | --- |
 | [proxyAuthOptions] | [<code>ProxyAuthOptions</code>](#ProxyAuthOptions) | the proxy auth options |
 
+<a name="parseRetryAfterHeader"></a>
+
+## parseRetryAfterHeader(header) ⇒ <code>number</code>
+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**: <code>number</code> - Number of milliseconds to sleep until the next call to getEventsFromJournal  
+
+| Param | Type | Description |
+| --- | --- | --- |
+| header | <code>string</code> | Retry-After header value |
+
 <a name="RetryOptions"></a>
 
 ## RetryOptions : <code>object</code>

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

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
 }

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)

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)
+  })
+})

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;
+