Refactor failed request callback to be agnostic of Bottleneck queue

src/HardenedFetch.ts

@@ -30,7 +30,7 @@ export class HardenedFetch {
       minTime: this.options.minRequestTime,
     })
 
-    this.queue.on('failed', handleFailed.bind(null, this.options))
+    this.queue.on('failed', (error, info) => handleFailed(this.options, error, info.retryCount))
   }
 
   fetch(url: string, init: RequestInit = {}, timeout: number = 30_000) {

src/handleFailed.spec.ts

@@ -2,15 +2,18 @@ import assert from 'node:assert'
 import { describe, it } from 'node:test'
 import createHttpError from 'http-errors'
 import { handleFailed } from './handleFailed.js'
-import type Bottleneck from 'bottleneck'
+import type { Options } from './options.js'
 
-const options = {
+const options: Options = {
+  // Throttle options
+  maxConcurrency: 0,
+  minRequestTime: 100,
   // Retry options
   maxRetries: 3,
   doNotRetry: [404],
   // Rate limit options
-  rateLimitHeader: 'Retry-After' as const,
-  resetFormat: 'seconds' as const,
+  rateLimitHeader: 'Retry-After',
+  resetFormat: 'seconds',
 }
 
 const createError = (status: number, headers = {}) => {
@@ -32,66 +35,54 @@ const createTimeout = () => {
 describe('Handle Failed', () => {
   describe('Retries', () => {
     it('exponentially increases the wait time as retry count increases', () => {
-      const info1 = { retryCount: 0 } as Bottleneck.EventInfoRetryable
-      const info2 = { retryCount: 1 } as Bottleneck.EventInfoRetryable
-      const info3 = { retryCount: 2 } as Bottleneck.EventInfoRetryable
-
       const httpError = createError(500)
-      assert.equal(handleFailed(options, httpError, info1), 1000)
-      assert.equal(handleFailed(options, httpError, info2), 4000)
-      assert.equal(handleFailed(options, httpError, info3), 9000)
+      assert.equal(handleFailed(options, httpError, 0), 1000)
+      assert.equal(handleFailed(options, httpError, 1), 4000)
+      assert.equal(handleFailed(options, httpError, 2), 9000)
 
       const timeoutError = createTimeout()
-      assert.equal(handleFailed(options, timeoutError, info1), 1000)
-      assert.equal(handleFailed(options, timeoutError, info2), 4000)
-      assert.equal(handleFailed(options, timeoutError, info3), 9000)
+      assert.equal(handleFailed(options, timeoutError, 0), 1000)
+      assert.equal(handleFailed(options, timeoutError, 1), 4000)
+      assert.equal(handleFailed(options, timeoutError, 2), 9000)
     })
 
     describe('when retry count reaches the limit', () => {
       it('returns nothing', () => {
         const httpError = createError(500)
         const timeoutError = createTimeout()
-        const info = { retryCount: 3 } as Bottleneck.EventInfoRetryable
 
-        assert.equal(handleFailed(options, httpError, info), undefined)
-        assert.equal(handleFailed(options, timeoutError, info), undefined)
+        assert.equal(handleFailed(options, httpError, 3), undefined)
+        assert.equal(handleFailed(options, timeoutError, 3), undefined)
       })
     })
 
     describe('when response status is flagged as do not retry', () => {
       it('returns nothing', () => {
         const error = createError(404)
-        const info = { retryCount: 3 } as Bottleneck.EventInfoRetryable
 
-        assert.equal(handleFailed(options, error, info), undefined)
+        assert.equal(handleFailed(options, error, 3), undefined)
       })
     })
   })
 
   describe('Rate limiting', () => {
     it('adds 1 second to the reset time', () => {
       const error = createError(429, { 'Retry-After': '12' })
-      const info = { retryCount: 0 } as Bottleneck.EventInfoRetryable
-
-      assert.equal(handleFailed(options, error, info), 13000)
+      assert.equal(handleFailed(options, error, 0), 13000)
     })
 
     describe('when no reset header is found', () => {
       it('returns default retry value', () => {
         const error = createError(429)
-        const info = { retryCount: 0 } as Bottleneck.EventInfoRetryable
-
-        assert.equal(handleFailed(options, error, info), 1000)
+        assert.equal(handleFailed(options, error, 0), 1000)
       })
     })
   })
 
   describe('with an unsupported error', () => {
     it('returns nothing', () => {
       const error = new Error()
-      const info = { retryCount: 0 } as Bottleneck.EventInfoRetryable
-
-      assert.equal(handleFailed(options, error, info), undefined)
+      assert.equal(handleFailed(options, error, 0), undefined)
     })
   })
 })

src/handleFailed.ts

@@ -1,37 +1,32 @@
 import { getResetHeader } from './utils.js'
 import { isHttpError } from 'http-errors'
-import type Bottleneck from 'bottleneck'
-import type { RateLimitOptions, RetryOptions } from './options.d.ts'
+import type { Options } from './options.d.ts'
 
 const backoff = (retries: number) => Math.pow(retries + 1, 2) * 1000
 
-export function handleFailed(
-  options: RetryOptions & RateLimitOptions,
-  error: Error,
-  info: Bottleneck.EventInfoRetryable
-): number | void {
-  const retry = info.retryCount < options.maxRetries
-
-  if (retry) {
-    if (isHttpError(error) && error?.response instanceof Response) {
-      const response = error.response
+export function handleFailed(options: Options, error: Error, retries: number): number | void {
+  if (retries >= options.maxRetries) {
+    return
+  }
 
-      if (response.status === 429) {
-        const wait = getResetHeader(response, options.rateLimitHeader, options.resetFormat)
+  if (isHttpError(error) && error?.response instanceof Response) {
+    if (options.doNotRetry?.includes(error.status)) {
+      return
+    }
 
-        if (wait) {
-          // Add extra 1 second to account for sub second differences
-          return wait + 1000
-        }
-      }
+    if (error.status === 429) {
+      const wait = getResetHeader(error.response, options.rateLimitHeader, options.resetFormat)
 
-      if (!options.doNotRetry?.includes(response.status)) {
-        return backoff(info.retryCount)
+      if (wait) {
+        // Add extra 1 second to account for sub second differences
+        return wait + 1000
       }
     }
 
-    if (error.name === 'TimeoutError') {
-      return backoff(info.retryCount)
-    }
+    return backoff(retries)
+  }
+
+  if (error.name === 'TimeoutError') {
+    return backoff(retries)
   }
 }