Skip to content

Commit 5c4d8cc

Browse files
authored
FFM-7412 - Reduce complexity of SSE code (#70)
FFM-7412 - Reduce complexity of SSE code What Remove eventsource dependency to reduce code complexity and create an easier to maintain implementation Why We're seeing the eventsource/retry packages spin and cause a lot of retries to happen at once, causing an adverse effect on system resources. Testing Manual testing with custom proxies to simulate network failures.
1 parent acd91c4 commit 5c4d8cc

File tree

8 files changed

+147
-88
lines changed

8 files changed

+147
-88
lines changed

package-lock.json

Lines changed: 2 additions & 32 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@harnessio/ff-nodejs-server-sdk",
3-
"version": "1.2.15",
3+
"version": "1.2.16",
44
"description": "Feature flags SDK for NodeJS environments",
55
"main": "dist/cjs/index.js",
66
"module": "dist/esm/index.mjs",
@@ -41,8 +41,8 @@
4141
"versioning": "node -p \"'export const VERSION = ' + JSON.stringify(require('./package.json').version) + ';'\" > src/version.ts",
4242
"prebuild": "npm run clean; npm run versioning; npm run lint",
4343
"dts": "tsc ./src/index.ts --declaration --emitDeclarationOnly --esModuleInterop --downlevelIteration --outdir ./dist",
44-
"build:esm": "esbuild ./src/index.ts --tsconfig=tsconfig.modules.json --minify --bundle --sourcemap --target=node12.22.5 --platform=node --format=esm --outfile=./dist/esm/index.mjs --external:keyv --external:keyv-file --external:eventsource --external:axios",
45-
"build:cjs": "esbuild ./src/index.ts --tsconfig=tsconfig.release.json --minify --bundle --sourcemap --target=node12.22.5 --platform=node --format=cjs --outfile=./dist/cjs/index.js --external:keyv --external:keyv-file --external:eventsource --external:axios",
44+
"build:esm": "esbuild ./src/index.ts --tsconfig=tsconfig.modules.json --minify --bundle --sourcemap --target=node12.22.5 --platform=node --format=esm --outfile=./dist/esm/index.mjs --external:keyv --external:keyv-file --external:axios",
45+
"build:cjs": "esbuild ./src/index.ts --tsconfig=tsconfig.release.json --minify --bundle --sourcemap --target=node12.22.5 --platform=node --format=cjs --outfile=./dist/cjs/index.js --external:keyv --external:keyv-file --external:axios",
4646
"build": "npm run build:esm; npm run build:cjs; npm run dts",
4747
"build:watch": "tsc -w -p tsconfig.release.json",
4848
"prepare": "npm run build",
@@ -63,7 +63,6 @@
6363
"dependencies": {
6464
"axios": "^0.22.0",
6565
"axios-retry": "^3.2.0",
66-
"@harnessio/eventsource": "^2.1.4",
6766
"jwt-decode": "^3.1.2",
6867
"keyv": "^4.0.3",
6968
"keyv-file": "^0.2.0",

src/__tests__/client.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ jest.mock('../openapi/api');
66

77
describe('Client', () => {
88
beforeEach(() => {
9-
jest.resetAllMocks()
10-
})
9+
jest.resetAllMocks();
10+
});
1111

1212
it('should close the client when the close method is called', async () => {
1313
// given
@@ -28,7 +28,7 @@ describe('Client', () => {
2828
});
2929

3030
it('should warn if poll interval is set below the default', async () => {
31-
jest.spyOn(PollingProcessor.prototype, 'start').mockReturnValue(undefined)
31+
jest.spyOn(PollingProcessor.prototype, 'start').mockReturnValue(undefined);
3232
const warnSpy = jest.spyOn(console, 'warn').mockReturnValue(undefined);
3333

3434
new Client('some key', {
@@ -55,7 +55,7 @@ describe('Client', () => {
5555
});
5656

5757
it('should warn if events sync interval is set below the default', async () => {
58-
jest.spyOn(PollingProcessor.prototype, 'start').mockReturnValue(undefined)
58+
jest.spyOn(PollingProcessor.prototype, 'start').mockReturnValue(undefined);
5959
const warnSpy = jest.spyOn(console, 'warn');
6060

6161
new Client('some key', {

src/client.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,16 @@ export default class Client {
6161

6262
if (options.pollInterval < defaultOptions.pollInterval) {
6363
this.options.pollInterval = defaultOptions.pollInterval;
64-
this.log.warn(`Polling interval cannot be lower than ${defaultOptions.pollInterval} ms`);
64+
this.log.warn(
65+
`Polling interval cannot be lower than ${defaultOptions.pollInterval} ms`,
66+
);
6567
}
6668

6769
if (options.eventsSyncInterval < defaultOptions.eventsSyncInterval) {
6870
this.options.eventsSyncInterval = defaultOptions.eventsSyncInterval;
69-
this.log.warn(`Events sync interval cannot be lower than ${defaultOptions.eventsSyncInterval} ms`);
71+
this.log.warn(
72+
`Events sync interval cannot be lower than ${defaultOptions.eventsSyncInterval} ms`,
73+
);
7074
}
7175

7276
this.configuration = new Configuration({
@@ -105,15 +109,19 @@ export default class Client {
105109

106110
this.eventBus.on(StreamEvent.RETRYING, () => {
107111
this.failure = true;
108-
this.log.error('Issue with streaming: falling back to polling while the SDK attempts to reconnect');
112+
this.log.error(
113+
'Issue with streaming: falling back to polling while the SDK attempts to reconnect',
114+
);
109115
if (!this.closing) {
110116
this.pollProcessor.start();
111117
}
112118
});
113119

114120
this.eventBus.on(StreamEvent.ERROR, () => {
115121
this.failure = true;
116-
this.log.error('Unrecoverable issue with streaming: falling back to polling');
122+
this.log.error(
123+
'Unrecoverable issue with streaming: falling back to polling',
124+
);
117125
if (!this.closing) {
118126
this.pollProcessor.start();
119127
}

src/evaluator.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ export class Evaluator {
206206
target: Target,
207207
): Promise<boolean> {
208208
for (const clause of clauses) {
209-
if ((await this.evaluateClause(clause, target))) {
209+
if (await this.evaluateClause(clause, target)) {
210210
// If any clause returns true we return - rules being treated as OR rather than AND
211211
return true;
212212
}

src/polling.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export class PollingProcessor {
2020
private eventBus: EventEmitter;
2121
private timeout: NodeJS.Timeout;
2222
private log: ConsoleLog;
23+
private lastPollTime = 0;
2324

2425
constructor(
2526
environment: string,
@@ -52,6 +53,17 @@ export class PollingProcessor {
5253
this.timeout = setTimeout(() => this.poll(), sleepFor);
5354
};
5455

56+
if (this.lastPollTime > Date.now() - this.options.pollInterval) {
57+
this.log.info(
58+
`Last poll was ${Math.round(
59+
(Date.now() - this.lastPollTime) / 1000,
60+
)} seconds ago, skipping flag refresh`,
61+
);
62+
pollAgain();
63+
return;
64+
}
65+
66+
this.lastPollTime = Date.now();
5567
Promise.all([this.retrieveFlags(), this.retrieveSegments()])
5668
.then(() => {
5769
// when first fetch is successful then poller is ready

0 commit comments

Comments
 (0)