Merge pull request #13 from octotravel/feat-fetch-retry-corrections

Feat fetch retry corrections
octotravel · Apr 24, 2024 · beaf77c · beaf77c
2 parents c27acd9 + 0a4657c
commit beaf77c
Show file tree

Hide file tree

Showing 5 changed files with 105 additions and 76 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@octocloud/backend",
-  "version": "1.0.19",
+  "version": "1.0.20",
   "license": "ISC",
   "author": "",
   "exports": {
@@ -31,7 +31,7 @@
   },
   "types": "./dist/index.d.ts",
   "dependencies": {
-    "@octocloud/core": "^1.0.50",
+    "@octocloud/core": "^1.0.51",
     "@octocloud/types": "^1.5.24",
     "date-fns": "^2.29.3",
     "date-fns-tz": "^1.3.7",

diff --git a/src/api/Client.ts b/src/api/Client.ts
@@ -1,5 +1,5 @@
 import { v5 } from 'uuid';
-import { OctoBackend, BaseConfig, SubRequestContext, BackendParams, Logger, fetchRetry } from '@octocloud/core';
+import { BaseConfig, SubRequestContext, BackendParams, Logger, fetchRetry } from '@octocloud/core';
 import { BeforeRequest } from './../index';
 import { OctoApiErrorHandler } from './ErrorHandler';
 
@@ -48,12 +48,10 @@ export abstract class APIClient {
   };
 
   public readonly fetch = async (url: string, method: RequestMethod, params: ApiClientParams): Promise<Response> => {
-    this.logger.log(`${new Date().toISOString()} ${method} ${url}`);
-
     const request = await this.createRequest(url, method, params);
     const req = await this.beforeRequest({ request });
     const subRequestContext = new SubRequestContext({
-      request: req.clone(),
+      request: req,
       requestId: params.ctx.getRequestId(),
       accountId: params.ctx.getAccountId(),
     });
@@ -66,7 +64,7 @@ export abstract class APIClient {
       await this.errorHandler.handleError(res, subRequestData, params.ctx);
     }
 
-    return res.clone();
+    return res;
   };
 
   private readonly createRequest = async (

diff --git a/src/api/ErrorHandler.ts b/src/api/ErrorHandler.ts
@@ -46,7 +46,7 @@ export class OctoApiErrorHandler {
       body,
       error: body?.error ?? null,
       requestId: requestContext.getRequestId(),
-      subRequestId: subRequestData.id,
+      subRequestId: subRequestData.getId(),
     };
 
     const error = this.errorMapper(body, errorParams, status);

diff --git a/src/api/__tests__/Api.test.ts b/src/api/__tests__/Api.test.ts
@@ -1,4 +1,4 @@
-import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import { API } from '../Api';
 import { BaseConfig, Environment, Logger, NullLogger, RequestContext, RequestMethod } from '@octocloud/core';
 import { BeforeRequest } from '../..';
@@ -8,6 +8,8 @@ describe('API', () => {
   let beforeRequest: BeforeRequest;
   let baseConfig: BaseConfig;
   let logger: Logger;
+  let request: Request;
+  let response: Response;
   let requestContext: RequestContext;
 
   beforeEach(() => {
@@ -18,19 +20,22 @@ describe('API', () => {
     beforeRequest = async ({ request }) => {
       return await Promise.resolve(request);
     };
+
     baseConfig = new BaseConfig({
       environment: Environment.TEST,
       productionURL: '',
       stagingURL: '',
     });
     logger = new NullLogger();
     api = new API(beforeRequest, baseConfig, logger);
+    request = new Request('https://octo.ventrata.com', {
+      headers: {
+        'Content-Type': 'application/json',
+      },
+    });
+    response = new Response('{}', { status: 200 });
     requestContext = new RequestContext({
-      request: new Request('https://octo.ventrata.com', {
-        headers: {
-          'Content-Type': 'application/json',
-        },
-      }),
+      request,
     });
 
     requestContext.setConnection({
@@ -41,6 +46,29 @@ describe('API', () => {
       accountId: '2635034e-3094-428b-b8f0-9d0cc0960c0c',
       name: 'testConnection',
     });
+    requestContext.setAccountId('124aef9e-bd6e-4f4f-9537-c41969889d86');
+    requestContext.setResponse(response);
+  });
+
+  afterEach(async () => {
+    // request and response should be properly consumed
+    const requestData = requestContext.getRequestData();
+
+    await request.text();
+    await response.text();
+
+    await requestData.getRequest().text();
+    await requestData.getResponse().text();
+
+    for (const subRequestData of requestData.getSubRequests()) {
+      await subRequestData.getRequest().text();
+      await subRequestData.getResponse().text();
+
+      for (const subRequestRetryData of subRequestData.getRetries()) {
+        await subRequestRetryData.getRequest().text();
+        await subRequestRetryData.getResponse().text();
+      }
+    }
   });
 
   describe('fetch', () => {
@@ -49,9 +77,9 @@ describe('API', () => {
       await api.fetch('https://octo.ventrata.com', RequestMethod.Get, {
         ctx: requestContext,
       });
-      const subrequest = requestContext.getSubrequests()[0];
-      expect(subrequest.response.status).toBe(200);
-      expect(subrequest.retries.length).toBe(0);
+      const subrequest = requestContext.getSubRequests()[0];
+      expect(subrequest.getResponse().status).toBe(200);
+      expect(subrequest.getRetries().length).toBe(0);
     });
 
     it('should succeed at second retry', async () => {
@@ -62,10 +90,10 @@ describe('API', () => {
       await api.fetch('https://octo.ventrata.com', RequestMethod.Get, {
         ctx: requestContext,
       });
-      const subrequest = requestContext.getSubrequests()[0];
-      expect(subrequest.response.status).toBe(503);
-      expect(subrequest.retries[0].response.status).toBe(200);
-      expect(subrequest.retries.length).toBe(1);
+      const subrequest = requestContext.getSubRequests()[0];
+      expect(subrequest.getResponse().status).toBe(503);
+      expect(subrequest.getRetries().length).toBe(1);
+      expect(subrequest.getRetries()[0].getResponse().status).toBe(200);
     });
 
     it('should succeed at third retry', async () => {
@@ -77,11 +105,11 @@ describe('API', () => {
       await api.fetch('https://octo.ventrata.com', RequestMethod.Get, {
         ctx: requestContext,
       });
-      const subrequest = requestContext.getSubrequests()[0];
-      expect(subrequest.response.status).toBe(503);
-      expect(subrequest.retries[0].response.status).toBe(502);
-      expect(subrequest.retries[1].response.status).toBe(200);
-      expect(subrequest.retries.length).toBe(2);
+      const subrequest = requestContext.getSubRequests()[0];
+      expect(subrequest.getResponse().status).toBe(503);
+      expect(subrequest.getRetries().length).toBe(2);
+      expect(subrequest.getRetries()[0].getResponse().status).toBe(502);
+      expect(subrequest.getRetries()[1].getResponse().status).toBe(200);
     });
 
     it('should fail after three retries', async () => {
@@ -93,11 +121,11 @@ describe('API', () => {
       await api.fetch('https://octo.ventrata.com', RequestMethod.Get, {
         ctx: requestContext,
       });
-      const subrequest = requestContext.getSubrequests()[0];
-      expect(subrequest.response.status).toBe(503);
-      expect(subrequest.retries[0].response.status).toBe(502);
-      expect(subrequest.retries[1].response.status).toBe(500);
-      expect(subrequest.retries.length).toBe(2);
+      const subrequest = requestContext.getSubRequests()[0];
+      expect(subrequest.getResponse().status).toBe(503);
+      expect(subrequest.getRetries().length).toBe(2);
+      expect(subrequest.getRetries()[0].getResponse().status).toBe(502);
+      expect(subrequest.getRetries()[1].getResponse().status).toBe(500);
     });
   });
 });