Merge pull request #40 from octotravel/feat/fixes

Feat/fixes

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@octocloud/core",
-  "version": "1.0.65",
+  "version": "1.0.66",
   "license": "ISC",
   "author": "",
   "exports": {

src/models/HeaderParser.ts

@@ -0,0 +1,25 @@
+export class HeaderParser {
+  public static getRetryAfterInSeconds(response: Response): number {
+    if (!response?.headers) {
+      return 0;
+    }
+
+    const retryAfterHeader = response.headers.get('retry-after');
+    if (retryAfterHeader === null) {
+      return 0;
+    }
+
+    const retryAfterHeaderNumberValue = Number(retryAfterHeader);
+    if (Number.isFinite(retryAfterHeaderNumberValue)) {
+      return retryAfterHeaderNumberValue || 1;
+    }
+
+    const retryDateMS = Date.parse(retryAfterHeader);
+    if (Number.isNaN(retryDateMS)) {
+      return 0;
+    }
+
+    const deltaMS = retryDateMS - Date.now();
+    return deltaMS > 0 ? Math.ceil(deltaMS / 1000) : 1;
+  }
+}

src/models/RequestContext.ts

@@ -1,10 +1,10 @@
+import { DataGenerationService } from '../services/DataGenerationService';
 import { BaseConnection } from '../types/Connection';
+import { AlertData } from './AlertData';
+import { Environment } from './Config';
+import { HttpError } from './Error';
 import { ConnectionMetaData, RequestData, RequestMetaData } from './RequestData';
 import { SubRequestData } from './SubRequestData';
-import { DataGenerationService } from '../services/DataGenerationService';
-import { HttpError } from './Error';
-import { Environment } from './Config';
-import { AlertData } from './AlertData';
 
 export class RequestContext {
   private readonly dataGenerationService = new DataGenerationService();
@@ -159,7 +159,7 @@ export class RequestContext {
 
   public getError = (): Error | null => this.error;
 
-  public setError = (error: Error): void => {
+  public setError = (error: Error | null): void => {
     this.error = error;
   };
 

src/models/__tests__/HeaderParser.test.ts

@@ -0,0 +1,87 @@
+import { describe, it, expect, vi } from 'vitest';
+import { HeaderParser } from '../HeaderParser';
+
+describe('HeaderParser.getRetryAfterInSeconds', () => {
+  it('should return 0 if response is null or undefined', () => {
+    expect(HeaderParser.getRetryAfterInSeconds(null as any)).toBe(0);
+    expect(HeaderParser.getRetryAfterInSeconds(undefined as any)).toBe(0);
+  });
+
+  it('should return 0 if response.headers is null or undefined', () => {
+    const response = { headers: null } as any;
+    expect(HeaderParser.getRetryAfterInSeconds(response)).toBe(0);
+  });
+
+  it('should return 0 if the "retry-after" header is not present', () => {
+    const response = {
+      headers: {
+        get: vi.fn().mockReturnValue(null),
+      },
+    } as unknown as Response;
+
+    expect(HeaderParser.getRetryAfterInSeconds(response)).toBe(0);
+    expect(response.headers.get).toHaveBeenCalledWith('retry-after');
+  });
+
+  it('should return the numeric value if "retry-after" header is a valid number', () => {
+    const response = {
+      headers: {
+        get: vi.fn().mockReturnValue('10'),
+      },
+    } as unknown as Response;
+
+    expect(HeaderParser.getRetryAfterInSeconds(response)).toBe(10);
+    expect(response.headers.get).toHaveBeenCalledWith('retry-after');
+  });
+
+  it('should return 1 if "retry-after" header is 0 or NaN when parsed as a number', () => {
+    const responseWithZero = {
+      headers: {
+        get: vi.fn().mockReturnValue('0'),
+      },
+    } as unknown as Response;
+
+    const responseWithNaN = {
+      headers: {
+        get: vi.fn().mockReturnValue('not-a-number'),
+      },
+    } as unknown as Response;
+
+    expect(HeaderParser.getRetryAfterInSeconds(responseWithZero)).toBe(1);
+    expect(HeaderParser.getRetryAfterInSeconds(responseWithNaN)).toBe(0);
+  });
+
+  it('should return the difference in seconds if "retry-after" is a valid date string in the future', () => {
+    const futureDate = new Date(Date.now() + 5000).toUTCString(); // 5 seconds in the future
+    const response = {
+      headers: {
+        get: vi.fn().mockReturnValue(futureDate),
+      },
+    } as unknown as Response;
+
+    expect(HeaderParser.getRetryAfterInSeconds(response)).toBeGreaterThanOrEqual(4);
+    expect(HeaderParser.getRetryAfterInSeconds(response)).toBeLessThanOrEqual(5);
+  });
+
+  it('should return 1 if "retry-after" is a valid date string in the past', () => {
+    const pastDate = new Date(Date.now() - 5000).toUTCString(); // 5 seconds in the past
+    const response = {
+      headers: {
+        get: vi.fn().mockReturnValue(pastDate),
+      },
+    } as unknown as Response;
+
+    expect(HeaderParser.getRetryAfterInSeconds(response)).toBe(1);
+  });
+
+  it('should return 0 if "retry-after" is an invalid date string', () => {
+    const invalidDate = 'invalid-date';
+    const response = {
+      headers: {
+        get: vi.fn().mockReturnValue(invalidDate),
+      },
+    } as unknown as Response;
+
+    expect(HeaderParser.getRetryAfterInSeconds(response)).toBe(0);
+  });
+});

src/util/__tests__/fetchRetry.test.ts

@@ -691,13 +691,31 @@ describe('fetchRetry', () => {
         it('should succeed at third retry with successful response', async () => {
           global.fetch = vi
             .fn()
-            .mockReturnValueOnce(Promise.resolve(new Response('503 Service Unavailable', { status: 503 })))
-            .mockReturnValueOnce(Promise.resolve(new Response('502 Bad Gateway', { status: 502 })))
+            .mockReturnValueOnce(
+              Promise.resolve(
+                new Response('503 Service Unavailable', { status: 503, headers: { 'Retry-After': '1' } }),
+              ),
+            )
+            .mockReturnValueOnce(
+              Promise.resolve(new Response('502 Bad Gateway', { status: 502, headers: { 'Retry-After': '2' } })),
+            )
             .mockReturnValue(Promise.resolve(new Response('{}', { status: 200 })));
           response = await fetchRetry(input, init, { retryDelayMultiplierInMs: RETRY_DELAY_MULTIPLIER_IN_MS });
           expect(response.status).toBe(200);
         });
 
+        it('should fail without any retries based on retry-again header', async () => {
+          global.fetch = vi
+            .fn()
+            .mockReturnValueOnce(
+              Promise.resolve(
+                new Response('500 Internal Server Error', { status: 500, headers: { 'Retry-After': '100' } }),
+              ),
+            );
+          response = await fetchRetry(input, init, { retryDelayMultiplierInMs: RETRY_DELAY_MULTIPLIER_IN_MS });
+          expect(response.status).toBe(500);
+        });
+
         it('should fail after three retries with server error response', async () => {
           global.fetch = vi
             .fn()

src/util/fetchRetry.ts

@@ -1,5 +1,6 @@
 import { SubRequestContext } from '../models/SubRequestContext';
 import { SubRequestRetryContext } from '../models/SubRequestRetryContext';
+import { HeaderParser } from '../models/HeaderParser';
 
 const DEFAULT_MAX_RETRY_ATTEMPTS = 3;
 const DEFAULT_RETRY_DELAY_MULTIPLIER_IN_MS = 1000;
@@ -92,11 +93,13 @@ export async function fetchRetry(
 
   currentRetryAttempt++;
 
+  const retryAfterInSeconds = HeaderParser.getRetryAfterInSeconds(res);
   const status = res.status;
 
   if (
     ((status >= 500 && status < 599) || status === 429 || (await shouldForceRetry(status, res.clone()))) &&
-    currentRetryAttempt < maxRetryAttempts
+    currentRetryAttempt < maxRetryAttempts &&
+    retryAfterInSeconds <= currentRetryAttempt
   ) {
     return await fetchRetry(request, undefined, {
       subRequestContext,