feat(cts): test the timeouts of the retry stragety (#3264)

Author: millotp

scripts/cts/runCts.ts

@@ -5,7 +5,7 @@ import { createSpinner } from '../spinners.js';
 import type { Language } from '../types.js';
 
 import { startTestServer } from './testServer';
-import { getTimeoutCounter } from './testServer/timeout.js';
+import { assertValidTimeouts } from './testServer/timeout.js';
 
 async function runCtsOne(language: string): Promise<void> {
   const spinner = createSpinner(`running cts for '${language}'`);
@@ -86,17 +86,9 @@ export async function runCts(languages: Language[], clients: string[]): Promise<
   if (useTestServer) {
     await close();
 
-    if (languages.length !== getTimeoutCounter()) {
-      throw new Error(
-        `Expected ${languages.length} timeout(s), got ${getTimeoutCounter()} instead.`,
-      );
-    }
+    assertValidTimeouts(languages.length);
 
     // uncomment this once all languages are supported
-    // if (languages.length !== numberOfSuccessfulReplaceAllObjectsCalls()) {
-    //   throw new Error(
-    //     `Expected ${languages.length} call to replaceAllObjects, got ${numberOfSuccessfulReplaceAllObjectsCalls()} instead.`,
-    //   );
-    // }
+    // assertValidReplaceAllObjects(languages.length);
   }
 }

scripts/cts/testServer/gzip.ts

@@ -4,12 +4,12 @@ import zlib from 'zlib';
 import crc32 from 'crc/crc32';
 import type { Express } from 'express';
 
+import { retryHandler } from './timeout';
+
 import { setupServer } from '.';
 
 function addRoutes(app: Express): void {
-  app.get('/1/test/retry', (req, res) => {
-    res.json({ message: 'ok test server response' });
-  });
+  app.get('/1/test/retry/:lang', retryHandler(0, 'ok test server response'));
 
   app.post('/1/test/gzip', (req, res) => {
     let rawBody = Buffer.from([]);

scripts/cts/testServer/index.ts

@@ -8,9 +8,15 @@ import { createSpinner } from '../../spinners';
 import { gzipServer } from './gzip';
 import { replaceAllObjectsServer } from './replaceAllObjects';
 import { timeoutServer } from './timeout';
+import { timeoutServerBis } from './timeoutBis';
 
 export async function startTestServer(): Promise<() => Promise<void>> {
-  const servers = await Promise.all([timeoutServer(), gzipServer(), replaceAllObjectsServer()]);
+  const servers = await Promise.all([
+    timeoutServer(),
+    timeoutServerBis(),
+    gzipServer(),
+    replaceAllObjectsServer(),
+  ]);
 
   return async () => {
     await Promise.all(

scripts/cts/testServer/replaceAllObjects.ts

@@ -17,8 +17,11 @@ const raoState: Record<
   }
 > = {};
 
-export function numberOfSuccessfulReplaceAllObjectsCalls(): number {
-  return Object.values(raoState).filter((s) => s.successful).length;
+export function assertValidReplaceAllObjects(expectedCount: number): void {
+  const count = Object.values(raoState).filter((s) => s.successful).length;
+  if (count !== expectedCount) {
+    throw new Error(`Expected ${expectedCount} call to replaceAllObjects, got ${count} instead.`);
+  }
 }
 
 function addRoutes(app: Express): void {

scripts/cts/testServer/timeout.ts

@@ -4,24 +4,89 @@ import type express from 'express';
 
 import { setupServer } from '.';
 
-let timeoutCounter = 0;
+const timeoutState: Record<string, { timestamp: number[]; duration: number[] }> = {};
 
-export function getTimeoutCounter(): number {
-  return timeoutCounter;
+function aboutEqual(a: number, b: number, epsilon = 100): boolean {
+  return Math.abs(a - b) <= epsilon;
+}
+
+export function assertValidTimeouts(expectedCount: number): void {
+  // assert that the retry strategy uses the correct timings, by checking the time between each request, and how long each request took before being timed out
+  // there should be no delay between requests, only an increase in timeout.
+  if (Object.keys(timeoutState).length !== expectedCount) {
+    throw new Error(`Expected ${expectedCount} timeout(s)`);
+  }
+
+  for (const [lang, state] of Object.entries(timeoutState)) {
+    if (state.timestamp.length !== 3 || state.duration.length !== 3) {
+      throw new Error(`Expected 3 requests for ${lang}, got ${state.timestamp.length}`);
+    }
+
+    let delay = state.timestamp[1] - state.timestamp[0];
+    if (!aboutEqual(delay, state.duration[0])) {
+      throw new Error(`Expected no delay between requests for ${lang}, got ${delay}ms`);
+    }
+
+    delay = state.timestamp[2] - state.timestamp[1];
+    if (!aboutEqual(delay, state.duration[1])) {
+      throw new Error(`Expected no delay between requests for ${lang}, got ${delay}ms`);
+    }
+
+    // languages are not consistent yet for the delay between requests
+    switch (lang) {
+      case 'JavaScript':
+        if (!aboutEqual(state.duration[0] * 4, state.duration[1], 200)) {
+          throw new Error(`Expected increasing delay between requests for ${lang}`);
+        }
+        break;
+      case 'PHP':
+        if (!aboutEqual(state.duration[0] * 2, state.duration[1], 200)) {
+          throw new Error(`Expected increasing delay between requests for ${lang}`);
+        }
+        break;
+      default:
+        // the delay should be the same, because the `retryCount` is per host instead of global
+        if (!aboutEqual(state.duration[0], state.duration[1])) {
+          throw new Error(`Expected the same delay between requests for ${lang}`);
+        }
+        break;
+    }
+  }
+}
+
+export function retryHandler(after: number, message: string): express.RequestHandler {
+  return (req, res) => {
+    const timeout = setTimeout(() => {
+      res.json({ message });
+    }, after);
+
+    const lang = req.params.lang;
+    const startTime = Date.now();
+    if (!timeoutState[lang]) {
+      timeoutState[lang] = {
+        timestamp: [],
+        duration: [],
+      };
+    }
+
+    timeoutState[lang].timestamp.push(startTime);
+
+    req.on('close', () => {
+      const endTime = Date.now();
+      const duration = endTime - startTime;
+      timeoutState[lang].duration.push(duration);
+
+      clearTimeout(timeout);
+      res.end();
+    });
+  };
 }
 
 function addRoutes(app: express.Express): void {
   // this endpoint is also defined in the gzip server but without the timeout
-  app.get('/1/test/retry', (req, res) => {
-    // this is safe because js is single threaded
-    timeoutCounter++;
-    // wait for 7.5 seconds, the default read timeout is 5 seconds + 2s of connection timeout
-    setTimeout(() => {
-      res.json({ message: 'timeout test server response' });
-    }, 7500);
-  });
+  app.get('/1/test/retry/:lang', retryHandler(20000, 'timeout test server response'));
 }
 
 export function timeoutServer(): Promise<Server> {
-  return setupServer('timeout', 6677, addRoutes);
+  return setupServer('timeout', 6676, addRoutes);
 }

scripts/cts/testServer/timeoutBis.ts

@@ -0,0 +1,16 @@
+import type { Server } from 'http';
+
+import type express from 'express';
+
+import { retryHandler } from './timeout';
+
+import { setupServer } from '.';
+
+function addRoutes(app: express.Express): void {
+  // this endpoint is also defined in the gzip server but without the timeout
+  app.get('/1/test/retry/:lang', retryHandler(20000, 'timeout bis test server response'));
+}
+
+export function timeoutServerBis(): Promise<Server> {
+  return setupServer('timeoutBis', 6677, addRoutes);
+}

templates/javascript/tests/client/suite.mustache

@@ -51,7 +51,7 @@ describe('{{testType}}', () => {
         {{/testResponse}}
       {{/isError}}
     {{/steps}}
-  });
+  }, 15000);
 
   {{/tests}}
 })

tests/CTS/client/search/api.json

@@ -59,6 +59,10 @@
           "appId": "test-app-id",
           "apiKey": "test-api-key",
           "customHosts": [
+            {
+              "host": "${{localhost}}",
+              "port": 6676
+            },
             {
               "host": "${{localhost}}",
               "port": 6677
@@ -75,7 +79,7 @@
         "object": "$client",
         "path": "customGet",
         "parameters": {
-          "path": "1/test/retry"
+          "path": "1/test/retry/${{languageCased}}"
         },
         "expected": {
           "type": "response",