fix: Fix retry-handler.js when retry-after header is a Date (#4084)

Co-authored-by: Carlos Fuentes <[email protected]>

Author: fgiova

lib/handler/retry-handler.js

@@ -11,8 +11,8 @@ const {
 } = require('../core/util')
 
 function calculateRetryAfterHeader (retryAfter) {
-  const current = Date.now()
-  return new Date(retryAfter).getTime() - current
+  const retryTime = new Date(retryAfter).getTime()
+  return isNaN(retryTime) ? 0 : retryTime - Date.now()
 }
 
 class RetryHandler {
@@ -124,7 +124,7 @@ class RetryHandler {
     if (retryAfterHeader) {
       retryAfterHeader = Number(retryAfterHeader)
       retryAfterHeader = Number.isNaN(retryAfterHeader)
-        ? calculateRetryAfterHeader(retryAfterHeader)
+        ? calculateRetryAfterHeader(headers['retry-after'])
         : retryAfterHeader * 1e3 // Retry-After is in seconds
     }
 

test/interceptors/retry.js

@@ -191,19 +191,19 @@ test('Should use retry-after header for retries (date)', async t => {
   server.on('request', (req, res) => {
     switch (counter) {
       case 0:
+        checkpoint = Date.now()
         res.writeHead(429, {
           'retry-after': new Date(
-            new Date().setSeconds(new Date().getSeconds() + 1)
+            checkpoint + 2000
           ).toUTCString()
         })
         res.end('rate limit')
-        checkpoint = Date.now()
         counter++
         return
       case 1:
         res.writeHead(200)
         res.end('hello world!')
-        t.ok(Date.now() - checkpoint >= 1)
+        t.ok(Date.now() - checkpoint >= 1000)
         counter++
         return
       default:

test/retry-handler.js

@@ -364,19 +364,19 @@ test('Should use retry-after header for retries (date)', async t => {
   server.on('request', (req, res) => {
     switch (counter) {
       case 0:
+        checkpoint = Date.now()
         res.writeHead(429, {
           'retry-after': new Date(
-            new Date().setSeconds(new Date().getSeconds() + 1)
+            checkpoint + 2000
           ).toUTCString()
         })
         res.end('rate limit')
-        checkpoint = Date.now()
         counter++
         return
       case 1:
         res.writeHead(200)
         res.end('hello world!')
-        t.ok(Date.now() - checkpoint >= 1)
+        t.ok(Date.now() - checkpoint >= 1000)
         counter++
         return
       default:
@@ -1547,3 +1547,89 @@ test('Should throw RequestRetryError when Content-Range mismatch', async t => {
 
   await t.completed
 })
+
+test('Should use retry-after header for retries (date) but date format is wrong', async t => {
+  t = tspl(t, { plan: 3 })
+
+  let counter = 0
+  const chunks = []
+  const server = createServer()
+  let checkpoint
+  const dispatchOptions = {
+    method: 'PUT',
+    path: '/',
+    headers: {
+      'content-type': 'application/json'
+    },
+    retryOptions: {
+      minTimeout: 1000
+    }
+  }
+
+  server.on('request', (req, res) => {
+    switch (counter) {
+      case 0:
+        checkpoint = Date.now()
+        res.writeHead(429, {
+          'retry-after': 'this is not a date'
+        })
+        res.end('rate limit')
+        counter++
+        return
+      case 1:
+        res.writeHead(200)
+        res.end('hello world!')
+        t.ok(Date.now() - checkpoint >= 1000)
+        counter++
+        return
+      default:
+        t.fail('unexpected request')
+    }
+  })
+
+  server.listen(0, () => {
+    const client = new Client(`http://localhost:${server.address().port}`)
+    const handler = new RetryHandler(dispatchOptions, {
+      dispatch: client.dispatch.bind(client),
+      handler: {
+        onConnect () {
+          t.ok(true, 'pass')
+        },
+        onHeaders (status, _rawHeaders, resume, _statusMessage) {
+          t.strictEqual(status, 200)
+          return true
+        },
+        onData (chunk) {
+          chunks.push(chunk)
+          return true
+        },
+        onComplete () {
+          t.strictEqual(Buffer.concat(chunks).toString('utf-8'), 'hello world!')
+        },
+        onError (err) {
+          t.ifError(err)
+        }
+      }
+    })
+
+    after(async () => {
+      await client.close()
+      server.close()
+
+      await once(server, 'close')
+    })
+
+    client.dispatch(
+      {
+        method: 'PUT',
+        path: '/',
+        headers: {
+          'content-type': 'application/json'
+        }
+      },
+      handler
+    )
+  })
+
+  await t.completed
+})