chore(cts): assert the retry strategy error (#3405)

millotp · Fluf22 · web-flow · commit e974ffcb9ef0 · 2024-07-24T10:10:18.000+02:00
Co-authored-by: Thomas Raffray &lt;Fluf22@users.noreply.github.com&gt;
diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml
@@ -271,7 +271,7 @@ jobs:
           path: clients-javascript.zip
 
   client_gen:
-    timeout-minutes: 20
+    timeout-minutes: 10
     runs-on: ubuntu-22.04
     needs:
       - setup
diff --git a/clients/algoliasearch-client-csharp/algoliasearch/Http/AlgoliaHttpRequester.cs b/clients/algoliasearch-client-csharp/algoliasearch/Http/AlgoliaHttpRequester.cs
@@ -105,7 +105,7 @@ public async Task<AlgoliaHttpResponse> SendRequestAsync(Request request, TimeSpa
         _logger.LogWarning(ex, "Timeout while sending request");
       }
 
-      return new AlgoliaHttpResponse { IsTimedOut = true, Error = ex.ToString() };
+      return new AlgoliaHttpResponse { IsTimedOut = true, Error = ex.Message };
     }
     catch (HttpRequestException ex)
     {
diff --git a/clients/algoliasearch-client-swift/Sources/Core/Networking/HTTP/AlgoliaError.swift b/clients/algoliasearch-client-swift/Sources/Core/Networking/HTTP/AlgoliaError.swift
@@ -10,7 +10,7 @@ import Foundation
 public enum AlgoliaError: Error, LocalizedError {
     case requestError(Error)
     case httpError(HTTPError)
-    case noReachableHosts(intermediateErrors: [Error])
+    case noReachableHosts(intermediateErrors: [Error], exposeIntermediateErrors: Bool)
     case missingData
     case decodingFailure(Error)
     case runtimeError(String)
@@ -24,8 +24,8 @@ public enum AlgoliaError: Error, LocalizedError {
             "Request failed: \(error.localizedDescription)"
         case let .httpError(error):
             "HTTP error: \(error)"
-        case let .noReachableHosts(errors):
-            "All hosts are unreachable. Intermediate errors: \(errors.map(\.localizedDescription).joined(separator: ", "))"
+        case let .noReachableHosts(errors, exposeIntermediateErrors):
+            "All hosts are unreachable. " + (exposeIntermediateErrors ? "Intermediate errors:\n- \(errors.map(\.localizedDescription).joined(separator: "\n- "))" : "You can use 'exposeIntermediateErrors: true' in the config to investigate.")
         case .missingData:
             "Missing response data"
         case .decodingFailure:
diff --git a/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift b/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift
@@ -40,13 +40,16 @@ class AlgoliaRetryStrategy: RetryStrategy {
             if !self.hosts.contains(where: { $0.supports(callType) && $0.isUp }) {
                 self.hosts.resetAll(for: callType)
             }
+            
+            var hostIterator = self.hosts
+                .sorted { $0.lastUpdated.compare($1.lastUpdated) == .orderedAscending }
+                .filter { $0.supports(callType) && $0.isUp }
+                .makeIterator()
 
             return HostIterator { [weak self] in
                 guard let retryStrategy = self else { return nil }
                 return retryStrategy.queue.sync {
-                    retryStrategy.hosts
-                        .sorted { $0.lastUpdated.compare($1.lastUpdated) == .orderedAscending }
-                        .first { $0.supports(callType) && $0.isUp }
+                    hostIterator.next()
                 }
             }
         }
diff --git a/clients/algoliasearch-client-swift/Sources/Core/Networking/Transporter.swift b/clients/algoliasearch-client-swift/Sources/Core/Networking/Transporter.swift
@@ -15,14 +15,17 @@ open class Transporter {
     let configuration: BaseConfiguration
     let retryStrategy: RetryStrategy
     let requestBuilder: RequestBuilder
+    let exposeIntermediateErrors: Bool
 
     public init(
         configuration: BaseConfiguration,
         retryStrategy: RetryStrategy? = nil,
-        requestBuilder: RequestBuilder? = nil
+        requestBuilder: RequestBuilder? = nil,
+        exposeIntermediateErrors: Bool = false
     ) {
         self.configuration = configuration
         self.retryStrategy = retryStrategy ?? AlgoliaRetryStrategy(configuration: configuration)
+        self.exposeIntermediateErrors = exposeIntermediateErrors
 
         guard let requestBuilder else {
             let sessionConfiguration: URLSessionConfiguration = .default
@@ -139,6 +142,6 @@ open class Transporter {
             }
         }
 
-        throw AlgoliaError.noReachableHosts(intermediateErrors: intermediateErrors)
+        throw AlgoliaError.noReachableHosts(intermediateErrors: intermediateErrors, exposeIntermediateErrors: self.exposeIntermediateErrors)
     }
 }
diff --git a/generators/src/main/java/com/algolia/codegen/cts/tests/ClientTestData.java b/generators/src/main/java/com/algolia/codegen/cts/tests/ClientTestData.java
@@ -22,6 +22,6 @@ class Step {
 class Expected {
 
   public String type;
-  public String error;
+  public Object error;
   public Object match;
 }
diff --git a/generators/src/main/java/com/algolia/codegen/cts/tests/TestsClient.java b/generators/src/main/java/com/algolia/codegen/cts/tests/TestsClient.java
@@ -163,10 +163,17 @@ public void run(Map<String, CodegenModel> models, Map<String, CodegenOperation>
               }
               if (step.expected.error != null) {
                 stepOut.put("isError", true);
-                stepOut.put("expectedError", step.expected.error);
+                if (step.expected.error instanceof Map errorMap) {
+                  stepOut.put("expectedError", errorMap.getOrDefault(language, "<missing error for " + language + ">"));
+                } else {
+                  stepOut.put("expectedError", step.expected.error);
+                }
                 if (language.equals("go") && step.method != null) {
                   // hack for go that use PascalCase, but just in the operationID
-                  stepOut.put("expectedError", step.expected.error.replace(step.method, Helpers.toPascalCase(step.method)));
+                  stepOut.put(
+                    "expectedError",
+                    ((String) stepOut.get("expectedError")).replace(step.method, Helpers.toPascalCase(step.method))
+                  );
                 }
               } else if (step.expected.match != null) {
                 Map<String, Object> matchMap = new HashMap<>();
diff --git a/scripts/cli/index.ts b/scripts/cli/index.ts
@@ -160,7 +160,12 @@ ctsCommand
   .description('Start the test servers in standalone mode')
   .action(async () => {
     setVerbose(true);
-    await startTestServer();
+    await startTestServer({
+      benchmark: true,
+      client: true,
+      requests: true,
+      e2e: true,
+    });
   });
 
 program
diff --git a/scripts/cts/runCts.ts b/scripts/cts/runCts.ts
@@ -5,7 +5,7 @@ import { getTestOutputFolder } from '../config.js';
 import { createSpinner } from '../spinners.js';
 import type { Language } from '../types.js';
 
-import { startBenchmarkServer, startTestServer } from './testServer';
+import { startTestServer } from './testServer';
 import { printBenchmarkReport } from './testServer/benchmark.js';
 import { assertChunkWrapperValid } from './testServer/chunkWrapper.js';
 import { assertValidReplaceAllObjects } from './testServer/replaceAllObjects.js';
@@ -142,38 +142,30 @@ export async function runCts(
   clients: string[],
   suites: Record<CTSType, boolean>,
 ): Promise<void> {
-  const useTestServer = suites.client && (clients.includes('search') || clients.includes('all'));
-  const useBenchmarkServer =
+  const withBenchmarkServer =
     suites.benchmark && (clients.includes('search') || clients.includes('all'));
-
-  let closeTestServer: () => Promise<void> = async () => {};
-  if (useTestServer) {
-    closeTestServer = await startTestServer();
-  }
-
-  let closeBenchmarkServer: () => Promise<void> = async () => {};
-  if (useBenchmarkServer) {
-    closeBenchmarkServer = await startBenchmarkServer();
-  }
+  const withClientServer = suites.client && (clients.includes('search') || clients.includes('all'));
+  const closeTestServer = await startTestServer({
+    ...suites,
+    benchmark: withBenchmarkServer,
+    client: withClientServer,
+  });
 
   for (const lang of languages) {
     await runCtsOne(lang, suites);
   }
 
-  if (useTestServer) {
-    await closeTestServer();
+  await closeTestServer();
 
+  if (withClientServer) {
     const skip = (lang: Language): number => (languages.includes(lang) ? 1 : 0);
 
     assertValidTimeouts(languages.length);
     assertChunkWrapperValid(languages.length - skip('dart') - skip('scala'));
     assertValidReplaceAllObjects(languages.length - skip('dart') - skip('scala'));
     assertValidWaitForApiKey(languages.length - skip('dart') - skip('scala'));
   }
-
-  if (useBenchmarkServer) {
-    await closeBenchmarkServer();
-
+  if (withBenchmarkServer) {
     printBenchmarkReport();
   }
 }
diff --git a/scripts/cts/testServer/index.ts b/scripts/cts/testServer/index.ts
@@ -4,6 +4,7 @@ import express from 'express';
 import type { Express } from 'express';
 
 import { createSpinner } from '../../spinners';
+import type { CTSType } from '../runCts';
 
 import { benchmarkServer } from './benchmark';
 import { chunkWrapperServer } from './chunkWrapper';
@@ -13,15 +14,28 @@ import { timeoutServer } from './timeout';
 import { timeoutServerBis } from './timeoutBis';
 import { waitForApiKeyServer } from './waitForApiKey';
 
-export async function startTestServer(): Promise<() => Promise<void>> {
-  const servers = await Promise.all([
-    timeoutServer(),
-    gzipServer(),
-    timeoutServerBis(),
-    replaceAllObjectsServer(),
-    chunkWrapperServer(),
-    waitForApiKeyServer(),
-  ]);
+export async function startTestServer(
+  suites: Record<CTSType, boolean>,
+): Promise<() => Promise<void>> {
+  const toStart: Array<Promise<Server>> = [];
+  if (suites.client) {
+    toStart.push(
+      timeoutServer(),
+      gzipServer(),
+      timeoutServerBis(),
+      replaceAllObjectsServer(),
+      chunkWrapperServer(),
+      waitForApiKeyServer(),
+    );
+  }
+  if (suites.benchmark) {
+    toStart.push(benchmarkServer());
+  }
+  if (toStart.length === 0) {
+    return async () => {};
+  }
+
+  const servers = await Promise.all(toStart);
 
   return async () => {
     await Promise.all(
@@ -35,16 +49,6 @@ export async function startTestServer(): Promise<() => Promise<void>> {
   };
 }
 
-export async function startBenchmarkServer(): Promise<() => Promise<void>> {
-  const server = await benchmarkServer();
-
-  return async () => {
-    await new Promise<void>((resolve) => {
-      server.close(() => resolve());
-    });
-  };
-}
-
 export async function setupServer(
   name: string,
   port: number,
diff --git a/scripts/cts/testServer/timeout.ts b/scripts/cts/testServer/timeout.ts
@@ -5,7 +5,8 @@ import type express from 'express';
 
 import { setupServer } from '.';
 
-const timeoutState: Record<string, { timestamp: number[]; duration: number[] }> = {};
+const timeoutState: Record<string, { timestamp: number[]; duration: number[]; hangCount: number }> =
+  {};
 
 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
@@ -15,6 +16,7 @@ export function assertValidTimeouts(expectedCount: number): void {
   }
 
   for (const [lang, state] of Object.entries(timeoutState)) {
+    expect(state.hangCount).to.equal(1);
     expect(state.timestamp.length).to.equal(3);
     expect(state.duration.length).to.equal(3);
     expect(state.timestamp[1] - state.timestamp[0]).to.be.closeTo(state.duration[0], 100);
@@ -23,7 +25,7 @@ export function assertValidTimeouts(expectedCount: number): void {
     // languages are not consistent yet for the delay between requests
     switch (lang) {
       case 'javascript':
-        expect(state.duration[0] * 4).to.be.closeTo(state.duration[1], 200);
+        expect(state.duration[0] * 4).to.be.closeTo(state.duration[1], 300);
         break;
       case 'php':
         expect(state.duration[0] * 2).to.be.closeTo(state.duration[1], 200);
@@ -51,6 +53,7 @@ export function retryHandler(after: number, message: string): express.RequestHan
       timeoutState[lang] = {
         timestamp: [],
         duration: [],
+        hangCount: 0,
       };
     }
 
@@ -70,6 +73,21 @@ export function retryHandler(after: number, message: string): express.RequestHan
 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 test server response'));
+
+  app.get('/1/test/hang/:lang', (req) => {
+    const lang = req.params.lang;
+    if (!timeoutState[lang]) {
+      timeoutState[lang] = {
+        timestamp: [],
+        duration: [],
+        hangCount: 0,
+      };
+    }
+
+    timeoutState[lang].hangCount++;
+
+    // no response, just hang
+  });
 }
 
 export function timeoutServer(): Promise<Server> {
diff --git a/templates/dart/tests/client/client.mustache b/templates/dart/tests/client/client.mustache
@@ -22,7 +22,7 @@ void main() {
     {{/autoCreateClient}}    
     {{#steps}}    
     {{#isError}}
-    expectError(
+    await expectError(
       '{{{expectedError}}}',
       () async {
         {{#dynamicTemplate}}{{/dynamicTemplate}}
diff --git a/templates/go/tests/client/method.mustache b/templates/go/tests/client/method.mustache
@@ -1 +1 @@
-{{^useEchoRequester}}{{^isBenchmark}}res, err := {{/isBenchmark}}{{#isBenchmark}}_, err = {{/isBenchmark}}{{/useEchoRequester}}{{#useEchoRequester}}_, err = {{/useEchoRequester}}{{> tests/method}}
+res, err = {{> tests/method}}
diff --git a/templates/go/tests/client/tests.mustache b/templates/go/tests/client/tests.mustache
@@ -2,6 +2,8 @@
 // {{{testName}}}
 func Test{{#lambda.titlecase}}{{clientPrefix}}{{testType}}{{/lambda.titlecase}}{{testIndex}}(t *testing.T) {
   var err error
+  var res any
+  _ = res
   {{#autoCreateClient}}
   client, echo := create{{#lambda.titlecase}}{{clientPrefix}}{{/lambda.titlecase}}Client(t)
   {{/autoCreateClient}}
diff --git a/templates/swift/tests/Package.mustache b/templates/swift/tests/Package.mustache
@@ -36,7 +36,6 @@ let package = Package(
     .testTarget(
       name: "requests",
       dependencies: [
-        .product(name: "DotEnv", package: "DotEnv"),
         .target(name: "Utils"),
       ] + libraries
     ),
@@ -50,7 +49,6 @@ let package = Package(
     .testTarget(
       name: "benchmark",
       dependencies: [
-        .product(name: "DotEnv", package: "DotEnv"),
         .target(name: "Utils"),
       ] + libraries
     )
diff --git a/tests/CTS/client/search/api.json b/tests/CTS/client/search/api.json
@@ -87,6 +87,47 @@
       }
     ]
   },
+  {
+    "testName": "tests the retry strategy error",
+    "autoCreateClient": false,
+    "steps": [
+      {
+        "type": "createClient",
+        "parameters": {
+          "appId": "test-app-id",
+          "apiKey": "test-api-key",
+          "customHosts": [
+            {
+              "host": "${{localhost}}",
+              "port": 6676
+            }
+          ]
+        }
+      },
+      {
+        "type": "method",
+        "method": "customGet",
+        "parameters": {
+          "path": "1/test/hang/${{language}}"
+        },
+        "expected": {
+          "error": {
+            "csharp": "RetryStrategy failed to connect to Algolia. Reason: The operation has timed out.",
+            "dart": "UnreachableHostsException{errors: [AlgoliaTimeoutException{error: DioException [receive timeout]: The request took longer than 0:00:05.000000 to receive data. It was aborted. To get rid of this exception, try raising the RequestOptions.receiveTimeout above the duration of 0:00:05.000000 or improve the response time of the server.}]}",
+            "go": "failed to do request: all hosts have been contacted unsuccessfully, it can either be a server or a network error or wrong appID/key credentials were used. You can use 'ExposeIntermediateNetworkErrors: true' in the config to investigate.",
+            "java": "Error(s) while processing the retry strategy\\nCaused by: java.net.SocketTimeoutException: timeout",
+            "javascript": "Unreachable hosts - your application id may be incorrect. If the error persists, please reach out to the Algolia Support team: https://alg.li/support.",
+            "kotlin": "Error(s) while processing the retry strategy",
+            "php": "Impossible to connect, please check your Algolia Application Id.",
+            "python": "Unreachable hosts",
+            "ruby": "Unreachable hosts. Last error for ${{localhost}}: Net::ReadTimeout with #<TCPSocket:(closed)>",
+            "scala": "Error(s) while processing the retry strategy",
+            "swift": "All hosts are unreachable. You can use 'exposeIntermediateErrors: true' in the config to investigate."
+          }
+        }
+      }
+    ]
+  },
   {
     "testName": "test the compression strategy",
     "autoCreateClient": false,
diff --git a/tests/output/csharp/src/RetryStrategyTests.cs b/tests/output/csharp/src/RetryStrategyTests.cs
diff --git a/tests/output/dart/lib/src/expect.dart b/tests/output/dart/lib/src/expect.dart
diff --git a/tests/output/scala/src/test/scala/algoliasearch/package.scala b/tests/output/scala/src/test/scala/algoliasearch/package.scala

Original file line number	Diff line number	Diff line change
`@@ -105,7 +105,7 @@ public async Task<AlgoliaHttpResponse> SendRequestAsync(Request request, TimeSpa`
`105`	`105`	`_logger.LogWarning(ex, "Timeout while sending request");`
`106`	`106`	`}`
`107`	`107`
`108`		`- return new AlgoliaHttpResponse { IsTimedOut = true, Error = ex.ToString() };`
	`108`	`+ return new AlgoliaHttpResponse { IsTimedOut = true, Error = ex.Message };`
`109`	`109`	`}`
`110`	`110`	`catch (HttpRequestException ex)`
`111`	`111`	`{`
Original file line number	Diff line number	Diff line change
`@@ -40,13 +40,16 @@ class AlgoliaRetryStrategy: RetryStrategy {`
`40`	`40`	`if !self.hosts.contains(where: { $0.supports(callType) && $0.isUp }) {`
`41`	`41`	`self.hosts.resetAll(for: callType)`
`42`	`42`	`}`
	`43`	`+`
	`44`	`+ var hostIterator = self.hosts`
	`45`	`+ .sorted { $0.lastUpdated.compare($1.lastUpdated) == .orderedAscending }`
	`46`	`+ .filter { $0.supports(callType) && $0.isUp }`
	`47`	`+ .makeIterator()`
`43`	`48`
`44`	`49`	`return HostIterator { [weak self] in`
`45`	`50`	`guard let retryStrategy = self else { return nil }`
`46`	`51`	`return retryStrategy.queue.sync {`
`47`		`- retryStrategy.hosts`
`48`		`- .sorted { $0.lastUpdated.compare($1.lastUpdated) == .orderedAscending }`
`49`		`- .first { $0.supports(callType) && $0.isUp }`
	`52`	`+ hostIterator.next()`
`50`	`53`	`}`
`51`	`54`	`}`
`52`	`55`	`}`
Original file line number	Diff line number	Diff line change
`@@ -15,14 +15,17 @@ open class Transporter {`
`15`	`15`	`let configuration: BaseConfiguration`
`16`	`16`	`let retryStrategy: RetryStrategy`
`17`	`17`	`let requestBuilder: RequestBuilder`
	`18`	`+ let exposeIntermediateErrors: Bool`
`18`	`19`
`19`	`20`	`public init(`
`20`	`21`	`configuration: BaseConfiguration,`
`21`	`22`	`retryStrategy: RetryStrategy? = nil,`
`22`		`- requestBuilder: RequestBuilder? = nil`
	`23`	`+ requestBuilder: RequestBuilder? = nil,`
	`24`	`+ exposeIntermediateErrors: Bool = false`
`23`	`25`	`) {`
`24`	`26`	`self.configuration = configuration`
`25`	`27`	`self.retryStrategy = retryStrategy ?? AlgoliaRetryStrategy(configuration: configuration)`
	`28`	`+ self.exposeIntermediateErrors = exposeIntermediateErrors`
`26`	`29`
`27`	`30`	`guard let requestBuilder else {`
`28`	`31`	`let sessionConfiguration: URLSessionConfiguration = .default`
`@@ -139,6 +142,6 @@ open class Transporter {`
`139`	`142`	`}`
`140`	`143`	`}`
`141`	`144`
`142`		`- throw AlgoliaError.noReachableHosts(intermediateErrors: intermediateErrors)`
	`145`	`+ throw AlgoliaError.noReachableHosts(intermediateErrors: intermediateErrors, exposeIntermediateErrors: self.exposeIntermediateErrors)`
`143`	`146`	`}`
`144`	`147`	`}`
Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,6 @@ class Step {`
`22`	`22`	`class Expected {`
`23`	`23`
`24`	`24`	`public String type;`
`25`		`- public String error;`
	`25`	`+ public Object error;`
`26`	`26`	`public Object match;`
`27`	`27`	`}`