Skip to content

Commit 2173013

Browse files
authored
fix: Improve remote evaluation fetch retry logic (#37)
1 parent b480a77 commit 2173013

File tree

4 files changed

+59
-7
lines changed

4 files changed

+59
-7
lines changed

__tests__/client.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { AnalyticsConnector } from '@amplitude/analytics-connector';
2+
import { FetchError } from '@amplitude/experiment-core';
23
import AsyncStorage from '@react-native-async-storage/async-storage';
34

45
import { Exposure } from '../lib/typescript';
@@ -61,6 +62,7 @@ class TestHttpClient implements HttpClient {
6162
return { status: this.status, body: this.body } as SimpleResponse;
6263
}
6364
}
65+
6466
/**
6567
* Basic test that fetching variants for a user succeeds.
6668
*/
@@ -957,3 +959,45 @@ describe('start', () => {
957959
expect(fetchSpy).toBeCalledTimes(0);
958960
});
959961
});
962+
963+
describe('fetch retry with different response codes', () => {
964+
beforeEach(() => {
965+
jest.clearAllMocks();
966+
});
967+
test.each([
968+
[300, 'Fetch Exception 300', 1],
969+
[400, 'Fetch Exception 400', 0],
970+
[429, 'Fetch Exception 429', 1],
971+
[500, 'Fetch Exception 500', 1],
972+
[0, 'Other Exception', 1],
973+
])(
974+
'responseCode=%p, errorMessage=%p, retryCalled=%p',
975+
async (responseCode, errorMessage, retryCalled) => {
976+
const client = new ExperimentClient(API_KEY, {
977+
retryFetchOnFailure: true,
978+
});
979+
980+
jest
981+
.spyOn(ExperimentClient.prototype as any, 'doFetch')
982+
.mockImplementation(async () => {
983+
return new Promise<ExperimentClient>((_resolve, reject) => {
984+
if (responseCode === 0) {
985+
reject(new Error(errorMessage));
986+
} else {
987+
reject(new FetchError(responseCode, errorMessage));
988+
}
989+
});
990+
});
991+
const retryMock = jest.spyOn(
992+
ExperimentClient.prototype as any,
993+
'startRetries',
994+
);
995+
try {
996+
await client.fetch({ user_id: 'test_user' });
997+
} catch (e) {
998+
// catch error
999+
}
1000+
expect(retryMock).toHaveBeenCalledTimes(retryCalled);
1001+
},
1002+
);
1003+
});

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
},
4949
"dependencies": {
5050
"@amplitude/analytics-connector": "^1.4.7",
51-
"@amplitude/experiment-core": "^0.7.0",
51+
"@amplitude/experiment-core": "^0.7.2",
5252
"@react-native-async-storage/async-storage": "^1.17.6",
5353
"unfetch": "^4.2.0"
5454
},

src/experimentClient.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
EvaluationApi,
99
EvaluationEngine,
1010
EvaluationFlag,
11+
FetchError,
1112
FlagApi,
1213
Poller,
1314
SdkFlagApi,
@@ -607,7 +608,7 @@ export class ExperimentClient implements Client {
607608

608609
this.debug(`[Experiment] Fetch all: retry=${retry}`);
609610

610-
// Proactively cancel retries if active in order to avoid unecessary API
611+
// Proactively cancel retries if active in order to avoid unnecessary API
611612
// requests. A new failure will restart the retries.
612613
if (retry) {
613614
this.stopRetries();
@@ -618,7 +619,7 @@ export class ExperimentClient implements Client {
618619
await this.storeVariants(variants, options);
619620
return variants;
620621
} catch (e) {
621-
if (retry) {
622+
if (retry && this.shouldRetryFetch(e)) {
622623
this.startRetries(user, options);
623624
}
624625
throw e;
@@ -780,6 +781,13 @@ export class ExperimentClient implements Client {
780781
console.debug(message, ...optionalParams);
781782
}
782783
}
784+
785+
private shouldRetryFetch(e: Error): boolean {
786+
if (e instanceof FetchError) {
787+
return e.statusCode < 400 || e.statusCode >= 500 || e.statusCode === 429;
788+
}
789+
return true;
790+
}
783791
}
784792

785793
type SourceVariant = {

yarn.lock

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@
1414
dependencies:
1515
"@amplitude/ua-parser-js" "^0.7.31"
1616

17-
"@amplitude/experiment-core@^0.7.0":
18-
version "0.7.0"
19-
resolved "https://registry.yarnpkg.com/@amplitude/experiment-core/-/experiment-core-0.7.0.tgz#258d95d691461acf3b6d1bea430f5ba231986342"
20-
integrity sha512-/W+BFGc2qix4aZ9V4VEYDcot8Vk/YirRcqY9Y0hlB27NPnhke0Id5mgPl/puKy7wqbXC6yTuda7rVpAtGBFMOA==
17+
"@amplitude/experiment-core@^0.7.2":
18+
version "0.7.2"
19+
resolved "https://registry.yarnpkg.com/@amplitude/experiment-core/-/experiment-core-0.7.2.tgz#f94219d68d86322e8d580c8fbe0672dcd29f86bb"
20+
integrity sha512-Wc2NWvgQ+bLJLeF0A9wBSPIaw0XuqqgkPKsoNFQrmS7r5Djd56um75In05tqmVntPJZRvGKU46pAp8o5tdf4mA==
2121
dependencies:
2222
js-base64 "^3.7.5"
2323

0 commit comments

Comments
 (0)