Refactor fetch handling to avoid illegal invocation errors (#1990)

* refactor fetch handling

* avoid && taking precedence

* add a test

Author: skarimo

.generator/src/generator/templates/configuration.j2

@@ -188,7 +188,7 @@ export function createConfiguration(conf: ConfigurationParameters = {}): Configu
     conf.baseServer,
     conf.serverIndex || 0,
     conf.operationServerIndices || {},
-    conf.httpApi || new DefaultHttpLibrary({ fetch: conf.fetch }),
+    conf.httpApi || new DefaultHttpLibrary(),
     configureAuthMethods(authMethods),
     conf.httpConfig || {},
     conf.debug,
@@ -214,6 +214,7 @@ export function createConfiguration(conf: ConfigurationParameters = {}): Configu
   configuration.httpApi.maxRetries = configuration.maxRetries;
   configuration.httpApi.backoffBase =  configuration.backoffBase;
   configuration.httpApi.backoffMultiplier = configuration.backoffMultiplier;
+  configuration.httpApi.fetch = conf.fetch;
   return configuration;
 }
 

.generator/src/generator/templates/http/http.j2

@@ -266,6 +266,7 @@ export interface HttpLibrary {
   backoffBase?: number;
   backoffMultiplier?: number;
   debug?: boolean;
+  fetch?: any;
   zstdCompressorCallback?: ZstdCompressorCallback;
   send(request: RequestContext): Promise<ResponseContext>;
 }

.generator/src/generator/templates/http/isomorphic-fetch.j2

@@ -12,17 +12,7 @@ export class IsomorphicFetchHttpLibrary implements HttpLibrary {
   public maxRetries!: number ;
   public backoffBase!: number ;
   public backoffMultiplier!: number;
-  #fetch: any;
-
-  constructor({ fetch: customFetch }: { fetch?: any }) {
-    this.#fetch =
-      customFetch ||
-      // On non-node environments, use native fetch if available.
-      // `cross-fetch` incorrectly assumes all browsers have XHR available.
-      // See https://github.com/lquixada/cross-fetch/issues/78
-      // TODO: Remove once once above issue is resolved.
-      (!isNode && typeof fetch === "function" ? fetch : crossFetch);
-  }
+  public fetch: any;
 
   public send(request: RequestContext): Promise<ResponseContext> {
     if (this.debug) {
@@ -79,7 +69,14 @@ export class IsomorphicFetchHttpLibrary implements HttpLibrary {
       signal: request.getHttpConfig().signal,
     }
     try {
-      const resp  = await this.#fetch(request.getUrl(),fetchOptions);
+      const fetchFunction = this.fetch ||
+        // On non-node environments, use native fetch if available.
+        // `cross-fetch` incorrectly assumes all browsers have XHR available.
+        // See https://github.com/lquixada/cross-fetch/issues/78
+        // TODO: Remove once once above issue is resolved.
+        ((!isNode && typeof fetch === "function") ? fetch : crossFetch);
+
+      const resp  = await fetchFunction(request.getUrl(),fetchOptions);
       const responseHeaders: { [name: string]: string } = {};
         resp.headers.forEach((value: string, name: string) => {
           responseHeaders[name] = value;

packages/datadog-api-client-common/configuration.ts

@@ -213,7 +213,7 @@ export function createConfiguration(
     conf.baseServer,
     conf.serverIndex || 0,
     conf.operationServerIndices || {},
-    conf.httpApi || new DefaultHttpLibrary({ fetch: conf.fetch }),
+    conf.httpApi || new DefaultHttpLibrary(),
     configureAuthMethods(authMethods),
     conf.httpConfig || {},
     conf.debug,
@@ -312,6 +312,7 @@ export function createConfiguration(
   configuration.httpApi.maxRetries = configuration.maxRetries;
   configuration.httpApi.backoffBase = configuration.backoffBase;
   configuration.httpApi.backoffMultiplier = configuration.backoffMultiplier;
+  configuration.httpApi.fetch = conf.fetch;
   return configuration;
 }
 

packages/datadog-api-client-common/http/http.ts

@@ -274,6 +274,7 @@ export interface HttpLibrary {
   backoffBase?: number;
   backoffMultiplier?: number;
   debug?: boolean;
+  fetch?: any;
   zstdCompressorCallback?: ZstdCompressorCallback;
   send(request: RequestContext): Promise<ResponseContext>;
 }

packages/datadog-api-client-common/http/isomorphic-fetch.ts

@@ -17,17 +17,7 @@ export class IsomorphicFetchHttpLibrary implements HttpLibrary {
   public maxRetries!: number;
   public backoffBase!: number;
   public backoffMultiplier!: number;
-  #fetch: any;
-
-  constructor({ fetch: customFetch }: { fetch?: any }) {
-    this.#fetch =
-      customFetch ||
-      // On non-node environments, use native fetch if available.
-      // `cross-fetch` incorrectly assumes all browsers have XHR available.
-      // See https://github.com/lquixada/cross-fetch/issues/78
-      // TODO: Remove once once above issue is resolved.
-      (!isNode && typeof fetch === "function" ? fetch : crossFetch);
-  }
+  public fetch: any;
 
   public send(request: RequestContext): Promise<ResponseContext> {
     if (this.debug) {
@@ -84,7 +74,15 @@ export class IsomorphicFetchHttpLibrary implements HttpLibrary {
       signal: request.getHttpConfig().signal,
     };
     try {
-      const resp = await this.#fetch(request.getUrl(), fetchOptions);
+      const fetchFunction =
+        this.fetch ||
+        // On non-node environments, use native fetch if available.
+        // `cross-fetch` incorrectly assumes all browsers have XHR available.
+        // See https://github.com/lquixada/cross-fetch/issues/78
+        // TODO: Remove once once above issue is resolved.
+        (!isNode && typeof fetch === "function" ? fetch : crossFetch);
+
+      const resp = await fetchFunction(request.getUrl(), fetchOptions);
       const responseHeaders: { [name: string]: string } = {};
       resp.headers.forEach((value: string, name: string) => {
         responseHeaders[name] = value;

tests/api/retry.test.ts

@@ -9,7 +9,7 @@ import {
 describe("IsomorphicFetchHttpLibrary Retry Test", () => {
 
     const fakeRequestContext = new RequestContext("https://retry.test.com",HttpMethod.GET);
-    const httpLibrary = new IsomorphicFetchHttpLibrary({fetch: null});
+    const httpLibrary = new IsomorphicFetchHttpLibrary();
     httpLibrary['sleep'] = jest.fn(() => Promise.resolve());
     httpLibrary.enableRetry = true;
     httpLibrary.maxRetries = 3;

tests/api/server.test.ts

@@ -3,7 +3,8 @@ import { createConfiguration } from "../../packages/datadog-api-client-common/co
 import {
   HttpMethod,
 } from "../../packages/datadog-api-client-common/http/http";
-import { DashboardsApiRequestFactory } from '../../packages/datadog-api-client-v1/apis/DashboardsApi';
+import { DashboardsApiRequestFactory, DashboardsApi } from '../../packages/datadog-api-client-v1/apis/DashboardsApi';
+import { logger } from "../../logger";
 
 test('TestServerClone', () => {
     const newServer = server1.clone();
@@ -53,4 +54,17 @@ test ('TestBackoffBaseNoLessThanTwo',() => {
     }; 
    expect(()=> createConfiguration(configOpts)).toThrow();
   }
+);
+
+test('TestCustomFetch', async () => {
+    // Mock logger to prevent writing to console during test
+    logger.error = jest.fn();
+    // Mock fetch
+    const _fetch = jest.fn(() => Promise.resolve());
+    const config = createConfiguration({fetch: _fetch});
+    const api = new DashboardsApi(config);
+    await expect(api.deleteDashboard({dashboardId: "id"})).rejects.toThrow();
+
+    expect(_fetch).toHaveBeenCalledTimes(1);
+  }
 );