From 52a9230af4b67136742a420683de54b73c198348 Mon Sep 17 00:00:00 2001 From: Thomas Raffray Date: Fri, 21 Nov 2025 16:38:20 +0100 Subject: [PATCH 1/5] fix(swift): remove unnecessary host reset in retry strategy --- .../Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift b/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift index 00e036e38e6..5eb068ada34 100644 --- a/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift +++ b/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift @@ -63,7 +63,6 @@ class AlgoliaRetryStrategy: RetryStrategy { } guard let error else { - self.hosts[hostIndex].reset() return } From d223662497ed7868b3f49ce37a7866ee3bc7a4b8 Mon Sep 17 00:00:00 2001 From: Thomas Raffray Date: Fri, 21 Nov 2025 16:43:45 +0100 Subject: [PATCH 2/5] fix(cts): test retry strat happy path --- scripts/cts/runCts.ts | 3 +- scripts/cts/testServer/error.ts | 45 ++++++++++++++++++++++++++++++ scripts/cts/testServer/index.ts | 3 +- tests/CTS/client/search/api.json | 47 ++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 2 deletions(-) diff --git a/scripts/cts/runCts.ts b/scripts/cts/runCts.ts index 4395a595896..dd08875deb1 100644 --- a/scripts/cts/runCts.ts +++ b/scripts/cts/runCts.ts @@ -8,7 +8,7 @@ import type { Language } from '../types.ts'; import { assertValidAccountCopyIndex } from './testServer/accountCopyIndex.ts'; import { printBenchmarkReport } from './testServer/benchmark.ts'; import { assertChunkWrapperValid } from './testServer/chunkWrapper.ts'; -import { assertValidErrors } from './testServer/error.ts'; +import { assertNeverCalledServerWasNotCalled, assertValidErrors } from './testServer/error.ts'; import { startTestServer } from './testServer/index.ts'; import { assertPushMockValid } from './testServer/pushMock.ts'; import { assertValidReplaceAllObjects } from './testServer/replaceAllObjects.ts'; @@ -158,6 +158,7 @@ export async function runCts( assertValidErrors(languages.length); assertValidTimeouts(languages.length); + assertNeverCalledServerWasNotCalled(languages.length); assertChunkWrapperValid(languages.length - skip('dart')); assertValidReplaceAllObjects(languages.length - skip('dart')); assertValidReplaceAllObjectsWithTransformation( diff --git a/scripts/cts/testServer/error.ts b/scripts/cts/testServer/error.ts index aec810a2fbb..406312cb2b5 100644 --- a/scripts/cts/testServer/error.ts +++ b/scripts/cts/testServer/error.ts @@ -6,6 +6,7 @@ import type express from 'express'; import { setupServer } from './index.ts'; const errorState: Record = {}; +const neverCalledState: Record = {}; export function assertValidErrors(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 @@ -26,6 +27,29 @@ export function assertValidErrors(expectedCount: number): void { } } +export function assertNeverCalledServerWasNotCalled(expectedCount: number): void { + // Verify that the fallback server was never called when the first host succeeds + if (Object.keys(neverCalledState).length !== expectedCount) { + throw new Error(`Expected ${expectedCount} language(s) to test the never-called server`); + } + + for (const [lang, callCount] of Object.entries(neverCalledState)) { + // python has sync and async tests + if (lang === 'python') { + expect(callCount).to.equal( + 0, + `Never-called server was called ${callCount} times for ${lang}, but should never be called`, + ); + continue; + } + + expect(callCount).to.equal( + 0, + `Never-called server was called ${callCount} times for ${lang}, but should never be called`, + ); + } +} + function addRoutes(app: express.Express): void { app.post('/1/test/error/:lang', (req, res) => { const lang = req.params.lang; @@ -58,3 +82,24 @@ export function errorServerRetriedOnce(): Promise { export function errorServerRetriedTwice(): Promise { return setupServer('errorRetriedTwice', 6673, addRoutes); } + +function addNeverCalledRoutes(app: express.Express): void { + app.get('/1/test/retry/:lang', (req, res) => { + const lang = req.params.lang; + if (!neverCalledState[lang]) { + neverCalledState[lang] = 0; + } + + neverCalledState[lang]++; + + // This should never be reached if the retry strategy correctly reuses successful hosts + res.status(500).json({ + message: 'This fallback server should never be called when the first host succeeds', + callCount: neverCalledState[lang], + }); + }); +} + +export function neverCalledServer(): Promise { + return setupServer('neverCalled', 6674, addNeverCalledRoutes); +} diff --git a/scripts/cts/testServer/index.ts b/scripts/cts/testServer/index.ts index de7521d77cd..da6010636d2 100644 --- a/scripts/cts/testServer/index.ts +++ b/scripts/cts/testServer/index.ts @@ -12,7 +12,7 @@ import { algoliaMockServer } from './algoliaMock.ts'; import { apiKeyServer } from './apiKey.ts'; import { benchmarkServer } from './benchmark.ts'; import { chunkWrapperServer } from './chunkWrapper.ts'; -import { errorServer, errorServerRetriedOnce, errorServerRetriedTwice } from './error.ts'; +import { errorServer, errorServerRetriedOnce, errorServerRetriedTwice, neverCalledServer } from './error.ts'; import { gzipServer } from './gzip.ts'; import { pushMockServer, pushMockServerRetriedOnce } from './pushMock.ts'; import { replaceAllObjectsServer } from './replaceAllObjects.ts'; @@ -31,6 +31,7 @@ export async function startTestServer(suites: Record): Promise errorServer(), errorServerRetriedOnce(), errorServerRetriedTwice(), + neverCalledServer(), gzipServer(), timeoutServerBis(), accountCopyIndexServer(), diff --git a/tests/CTS/client/search/api.json b/tests/CTS/client/search/api.json index 0dc964fb23d..cd598eac0fe 100644 --- a/tests/CTS/client/search/api.json +++ b/tests/CTS/client/search/api.json @@ -336,5 +336,52 @@ } } ] + }, + { + "testName": "does not retry on success", + "autoCreateClient": false, + "steps": [ + { + "type": "createClient", + "parameters": { + "appId": "test-app-id", + "apiKey": "test-api-key", + "customHosts": [ + { + "port": 6678 + }, + { + "port": 6674 + } + ] + } + }, + { + "type": "method", + "method": "customGet", + "parameters": { + "path": "1/test/retry/${{language}}" + }, + "expected": { + "type": "response", + "match": { + "message": "ok test server response" + } + } + }, + { + "type": "method", + "method": "customGet", + "parameters": { + "path": "1/test/retry/${{language}}" + }, + "expected": { + "type": "response", + "match": { + "message": "ok test server response" + } + } + } + ] } ] From e542b4fdc0e942b7c67aa15aa25e10167cfd69ad Mon Sep 17 00:00:00 2001 From: Thomas Raffray Date: Fri, 21 Nov 2025 21:17:41 +0100 Subject: [PATCH 3/5] fix(swift): add success server implementation and assertions --- scripts/cts/runCts.ts | 4 ++- scripts/cts/testServer/error.ts | 18 ++---------- scripts/cts/testServer/index.ts | 2 ++ scripts/cts/testServer/success.ts | 48 +++++++++++++++++++++++++++++++ tests/CTS/client/search/api.json | 10 +++---- 5 files changed, 60 insertions(+), 22 deletions(-) create mode 100644 scripts/cts/testServer/success.ts diff --git a/scripts/cts/runCts.ts b/scripts/cts/runCts.ts index dd08875deb1..613f57f14de 100644 --- a/scripts/cts/runCts.ts +++ b/scripts/cts/runCts.ts @@ -15,6 +15,7 @@ import { assertValidReplaceAllObjects } from './testServer/replaceAllObjects.ts' import { assertValidReplaceAllObjectsFailed } from './testServer/replaceAllObjectsFailed.ts'; import { assertValidReplaceAllObjectsScopes } from './testServer/replaceAllObjectsScopes.ts'; import { assertValidReplaceAllObjectsWithTransformation } from './testServer/replaceAllObjectsWithTransformation.ts'; +import { assertSuccessServerCalled } from './testServer/success.ts'; import { assertValidTimeouts } from './testServer/timeout.ts'; import { assertValidWaitForApiKey } from './testServer/waitFor.ts'; @@ -158,7 +159,8 @@ export async function runCts( assertValidErrors(languages.length); assertValidTimeouts(languages.length); - assertNeverCalledServerWasNotCalled(languages.length); + assertNeverCalledServerWasNotCalled(); + assertSuccessServerCalled(languages.length); assertChunkWrapperValid(languages.length - skip('dart')); assertValidReplaceAllObjects(languages.length - skip('dart')); assertValidReplaceAllObjectsWithTransformation( diff --git a/scripts/cts/testServer/error.ts b/scripts/cts/testServer/error.ts index 406312cb2b5..6e3322f19c8 100644 --- a/scripts/cts/testServer/error.ts +++ b/scripts/cts/testServer/error.ts @@ -27,22 +27,8 @@ export function assertValidErrors(expectedCount: number): void { } } -export function assertNeverCalledServerWasNotCalled(expectedCount: number): void { - // Verify that the fallback server was never called when the first host succeeds - if (Object.keys(neverCalledState).length !== expectedCount) { - throw new Error(`Expected ${expectedCount} language(s) to test the never-called server`); - } - +export function assertNeverCalledServerWasNotCalled(): void { for (const [lang, callCount] of Object.entries(neverCalledState)) { - // python has sync and async tests - if (lang === 'python') { - expect(callCount).to.equal( - 0, - `Never-called server was called ${callCount} times for ${lang}, but should never be called`, - ); - continue; - } - expect(callCount).to.equal( 0, `Never-called server was called ${callCount} times for ${lang}, but should never be called`, @@ -84,7 +70,7 @@ export function errorServerRetriedTwice(): Promise { } function addNeverCalledRoutes(app: express.Express): void { - app.get('/1/test/retry/:lang', (req, res) => { + app.get('/1/test/calling/:lang', (req, res) => { const lang = req.params.lang; if (!neverCalledState[lang]) { neverCalledState[lang] = 0; diff --git a/scripts/cts/testServer/index.ts b/scripts/cts/testServer/index.ts index da6010636d2..752d650a762 100644 --- a/scripts/cts/testServer/index.ts +++ b/scripts/cts/testServer/index.ts @@ -19,6 +19,7 @@ import { replaceAllObjectsServer } from './replaceAllObjects.ts'; import { replaceAllObjectsServerFailed } from './replaceAllObjectsFailed.ts'; import { replaceAllObjectsScopesServer } from './replaceAllObjectsScopes.ts'; import { replaceAllObjectsWithTransformationServer } from './replaceAllObjectsWithTransformation.ts'; +import { successServer } from './success.ts'; import { timeoutServer } from './timeout.ts'; import { timeoutServerBis } from './timeoutBis.ts'; import { waitForApiKeyServer } from './waitFor.ts'; @@ -32,6 +33,7 @@ export async function startTestServer(suites: Record): Promise errorServerRetriedOnce(), errorServerRetriedTwice(), neverCalledServer(), + successServer(), gzipServer(), timeoutServerBis(), accountCopyIndexServer(), diff --git a/scripts/cts/testServer/success.ts b/scripts/cts/testServer/success.ts new file mode 100644 index 00000000000..35500f2d9d3 --- /dev/null +++ b/scripts/cts/testServer/success.ts @@ -0,0 +1,48 @@ +import type { Server } from 'http'; + +import { expect } from 'chai'; +import type express from 'express'; + +import { setupServer } from './index.ts'; + +const successState: Record = {}; + +export function assertSuccessServerCalled(expectedCount: number): void { + // Verify that the success server was called the expected number of times per language + if (Object.keys(successState).length !== expectedCount) { + throw new Error(`Expected ${expectedCount} language(s) to test the success server`); + } + + for (const [lang, callCount] of Object.entries(successState)) { + // python has sync and async tests, each making 2 requests + if (lang === 'python') { + expect(callCount).to.equal( + 4, + `Success server was called ${callCount} times for ${lang}, expected 4 (2 sync + 2 async)`, + ); + continue; + } + + // Each test makes 2 consecutive requests, both should hit this server + expect(callCount).to.equal(2, `Success server was called ${callCount} times for ${lang}, expected 2`); + } +} + +function addRoutes(app: express.Express): void { + app.get('/1/test/calling/:lang', (req, res) => { + const lang = req.params.lang; + if (!successState[lang]) { + successState[lang] = 0; + } + + successState[lang]++; + + res.status(200).json({ + message: 'success server response', + }); + }); +} + +export function successServer(): Promise { + return setupServer('success', 6675, addRoutes); +} diff --git a/tests/CTS/client/search/api.json b/tests/CTS/client/search/api.json index cd598eac0fe..89d284a8425 100644 --- a/tests/CTS/client/search/api.json +++ b/tests/CTS/client/search/api.json @@ -348,7 +348,7 @@ "apiKey": "test-api-key", "customHosts": [ { - "port": 6678 + "port": 6675 }, { "port": 6674 @@ -360,12 +360,12 @@ "type": "method", "method": "customGet", "parameters": { - "path": "1/test/retry/${{language}}" + "path": "1/test/calling/${{language}}" }, "expected": { "type": "response", "match": { - "message": "ok test server response" + "message": "success server response" } } }, @@ -373,12 +373,12 @@ "type": "method", "method": "customGet", "parameters": { - "path": "1/test/retry/${{language}}" + "path": "1/test/calling/${{language}}" }, "expected": { "type": "response", "match": { - "message": "ok test server response" + "message": "success server response" } } } From 2f99a2fd822dca4a3c5cabd9f7968895fdd5f604 Mon Sep 17 00:00:00 2001 From: Thomas Raffray Date: Fri, 21 Nov 2025 21:40:40 +0100 Subject: [PATCH 4/5] fix(swift): testing that the test works as expected --- .../Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift b/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift index 5eb068ada34..00e036e38e6 100644 --- a/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift +++ b/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift @@ -63,6 +63,7 @@ class AlgoliaRetryStrategy: RetryStrategy { } guard let error else { + self.hosts[hostIndex].reset() return } From b6858be5ece6160393811d74c82654aa67650d86 Mon Sep 17 00:00:00 2001 From: Thomas Raffray Date: Fri, 21 Nov 2025 22:11:12 +0100 Subject: [PATCH 5/5] fix(swift): match other lang --- .../Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift b/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift index 00e036e38e6..cda8a7a702f 100644 --- a/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift +++ b/clients/algoliasearch-client-swift/Sources/Core/Networking/RetryStrategy/AlgoliaRetryStrategy.swift @@ -42,7 +42,6 @@ class AlgoliaRetryStrategy: RetryStrategy { } var hostIterator = self.hosts - .sorted { $0.lastUpdated.compare($1.lastUpdated) == .orderedAscending } .filter { $0.supports(callType) && $0.isUp } .makeIterator()