Skip to content

Commit 54cb1f1

Browse files
LaunchDarklyReleaseBoteli-darklyzmdavisLaunchDarklyCIbwoskow-ld
authored
prepare 3.8.2 release (#78)
* initial move of code from js-client-sdk-private * changelog note * rm obsolete comment * add npm audit helper * update babel, jest, rollup * fix rollup config * fix ES build, dependency cleanup * add Releaser metadata * Update babel config to work in `test` without `useBuiltIns` * copyedits * fix misnamed directory * use spread operator instead of Object.assign * add issue templates * add babel-eslint * add event capacity config property * re-add deprecation warning on samplingInterval * better config validation * remove rollup-plugins-node-resolve * use newer Rollup node-resolve plugin * rm rollup-plugin-includepaths (unused) * npm audit fix (handlebars dependency from jest) * comment * copyedit * use new test helpers + misc test cleanup * clean up stream testing logic * fix hash parameter * linter * clearer way to model the config option defaults/types * test improvements * change internal param name * comment * fix default logger logic * simpler way to enforce minimum values * implement diagnostic events in common JS package (#11) * add support for function type in config options * add support for function type in config options (#13) * add wrapper metadata options and fix custom header logic * lint * lint * remove image-loading logic from common code, replace it with an abstraction * add validation for options.streaming * typo * rm unused params * typo in comment * misc fixes to merged code from external PR * add event payload ID header * npm audit fix * change exact dependencies to best-compatible * standardize linting * disallow "window" and "document" * improve diag event tests + debug logging * misc cleanup * fix updating secure mode hash with identify() * don't omit streamInits.failed when it's false * clean up init state logic, prevent unhandled rejections * lint * less strict matching of json content-type header * remove unsafe usage of hasOwnProperty * console logger must tolerate console object not always existing * fix double initialization of diagnostics manager * fix TypeScript declaration for track() and add more TS compilation tests (#27) * remove startsWith usage (#28) * prevent poll caused by a stream ping from overwriting later poll for another user (#29) * upgrade jest dependency and transitive yargs-parser dependency (#30) * Add null to LDEvaluationDetail.reason type (#31) * Revert "Add null to LDEvaluationDetail.reason type (#31)" This reverts commit bcb1573. * Revert "Add null to LDEvaluationDetail.reason type (#31)" This reverts commit bcb1573. * nullable evaluation reason (#32) * adding alias event functionality (#33) * set stream read timeout * Add prepare script (#34) * add a missing typescript verification (#36) * Removed the guides link * Correct doc link (#36) * Fix typo in LDClient.on jsdoc (#37) * add inlineUsersInEvents option in TypeScript (#37) * Filter private attributes on debug event users. Send variation for debug events. * update uuid package version (#39) * use Releaser v2 config + newer CI image * First half, add the type, create the new options, add the new util method, and add tests * Second half, call the tranform util method before calling any HTTP requests * Update the transform to work on a copy of headers instead of mutating it * add comments about removing custom event warning logic in the future * revert updating of UUID dependency (#43) * Revert "update uuid package version (#39)" This reverts commit 3b2ff6c. * update package-lock.json * better error handling for local storage operations (#44) * better error handling for local storage operations * lint * fix obsolete comments * add basic logger similar to server-side Node SDK (#45) * fix exports and add validation of custom logger (#46) * remove typedoc.js file that interferes with Releaser's docs build * update typescript version * add maintenance branch * backport sc-142333 fix * make URL path concatenation work right whether base URL has a trailing slash or not (#61) * make URL path concatenation work right whether base URL has a trailing slash or not * lint * Implement application tags for 3.x. (#62) * don't include deleted flags in allFlags (#66) * Backport changes to seen request cache. (#74) * Backport changes for inspectors V2 and tag length enforcement. (#76) * Fix onflags invocation. (#78) * Implement jitter and backoff. (#79) * Merge: Remove flatMap usage. (#83) Co-authored-by: Mateusz Sikora <mateusz.sikora@ramp.network> Co-authored-by: Eli Bishop <eli@launchdarkly.com> Co-authored-by: Zach Davis <zach@launchdarkly.com> Co-authored-by: LaunchDarklyCI <dev@launchdarkly.com> Co-authored-by: Ben Woskow <bwoskow@launchdarkly.com> Co-authored-by: Ben Woskow <48036130+bwoskow-ld@users.noreply.github.com> Co-authored-by: Michael Siadak <mike.siadak@gmail.com> Co-authored-by: Jeff Wen <sinchangwen@gmail.com> Co-authored-by: Andrey Krasnov <34657799+Doesntmeananything@users.noreply.github.com> Co-authored-by: Gavin Whelan <gwhelan@launchdarkly.com> Co-authored-by: LaunchDarklyReleaseBot <launchdarklyreleasebot@launchdarkly.com> Co-authored-by: Louis Chan <lchan@launchdarkly.com> Co-authored-by: Louis Chan <91093020+louis-launchdarkly@users.noreply.github.com> Co-authored-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Co-authored-by: Mateusz Sikora <mateusz.sikora@ramp.network>
1 parent c98721b commit 54cb1f1

File tree

4 files changed

+92
-7
lines changed

4 files changed

+92
-7
lines changed

src/Stream.js

+45-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import * as messages from './messages';
22
import { appendUrlPath, base64URLEncode, objectHasOwnProperty } from './utils';
33
import { getLDHeaders, transformHeaders } from './headers';
4+
import { isHttpErrorRecoverable } from './errors';
45

56
// The underlying event source implementation is abstracted via the platform object, which should
67
// have these three properties:
@@ -16,6 +17,8 @@ import { getLDHeaders, transformHeaders } from './headers';
1617
// interval between heartbeats from the LaunchDarkly streaming server. If this amount of time elapses
1718
// with no new data, the connection will be cycled.
1819
const streamReadTimeoutMillis = 5 * 60 * 1000; // 5 minutes
20+
const maxRetryDelay = 30 * 1000; // Maximum retry delay 30 seconds.
21+
const jitterRatio = 0.5; // Delay should be 50%-100% of calculated time.
1922

2023
export default function Stream(platform, config, environment, diagnosticsAccumulator) {
2124
const baseUrl = config.streamUrl;
@@ -24,7 +27,7 @@ export default function Stream(platform, config, environment, diagnosticsAccumul
2427
const evalUrlPrefix = appendUrlPath(baseUrl, '/eval/' + environment);
2528
const useReport = config.useReport;
2629
const withReasons = config.evaluationReasons;
27-
const streamReconnectDelay = config.streamReconnectDelay;
30+
const baseReconnectDelay = config.streamReconnectDelay;
2831
const headers = getLDHeaders(platform, config);
2932
let firstConnectionErrorLogged = false;
3033
let es = null;
@@ -33,6 +36,22 @@ export default function Stream(platform, config, environment, diagnosticsAccumul
3336
let user = null;
3437
let hash = null;
3538
let handlers = null;
39+
let retryCount = 0;
40+
41+
function backoff() {
42+
const delay = baseReconnectDelay * Math.pow(2, retryCount);
43+
return delay > maxRetryDelay ? maxRetryDelay : delay;
44+
}
45+
46+
function jitter(computedDelayMillis) {
47+
return computedDelayMillis - Math.trunc(Math.random() * jitterRatio * computedDelayMillis);
48+
}
49+
50+
function getNextRetryDelay() {
51+
const delay = jitter(backoff());
52+
retryCount += 1;
53+
return delay;
54+
}
3655

3756
stream.connect = function(newUser, newHash, newHandlers) {
3857
user = newUser;
@@ -63,13 +82,31 @@ export default function Stream(platform, config, environment, diagnosticsAccumul
6382
};
6483

6584
function handleError(err) {
85+
// The event source may not produce a status. But the LaunchDarkly
86+
// polyfill can. If we can get the status, then we should stop retrying
87+
// on certain error codes.
88+
if (err.status && typeof err.status === 'number' && !isHttpErrorRecoverable(err.status)) {
89+
// If we encounter an unrecoverable condition, then we do not want to
90+
// retry anymore.
91+
closeConnection();
92+
logger.error(messages.unrecoverableStreamError(err));
93+
// Ensure any pending retry attempts are not done.
94+
if (reconnectTimeoutReference) {
95+
clearTimeout(reconnectTimeoutReference);
96+
reconnectTimeoutReference = null;
97+
}
98+
return;
99+
}
100+
101+
const delay = getNextRetryDelay();
102+
66103
if (!firstConnectionErrorLogged) {
67-
logger.warn(messages.streamError(err, streamReconnectDelay));
104+
logger.warn(messages.streamError(err, delay));
68105
firstConnectionErrorLogged = true;
69106
}
70107
logConnectionResult(false);
71108
closeConnection();
72-
tryConnect(streamReconnectDelay);
109+
tryConnect(delay);
73110
}
74111

75112
function tryConnect(delay) {
@@ -123,6 +160,11 @@ export default function Stream(platform, config, environment, diagnosticsAccumul
123160
}
124161

125162
es.onerror = handleError;
163+
164+
es.onopen = () => {
165+
// If the connection is a success, then reset the retryCount.
166+
retryCount = 0;
167+
};
126168
}
127169
}
128170

src/__tests__/Stream-test.js

+40-2
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ describe('Stream', () => {
170170
const nAttempts = 5;
171171
for (let i = 0; i < nAttempts; i++) {
172172
es.mockError('test error');
173+
await sleepAsync(10);
173174
const created1 = await platform.testing.expectStream();
174175
const es1 = created1.eventSource;
175176

@@ -185,6 +186,40 @@ describe('Stream', () => {
185186
}
186187
});
187188

189+
it.each([401, 403])('does not reconnect after an unrecoverable error', async status => {
190+
const config = { ...defaultConfig, streamReconnectDelay: 1, useReport: false };
191+
const stream = new Stream(platform, config, envName);
192+
stream.connect(user);
193+
194+
const created = await platform.testing.expectStream();
195+
const es = created.eventSource;
196+
197+
expect(es.readyState).toBe(EventSource.CONNECTING);
198+
es.mockOpen();
199+
expect(es.readyState).toBe(EventSource.OPEN);
200+
201+
es.mockError({ status });
202+
await sleepAsync(10);
203+
expect(platform.testing.eventSourcesCreated.length()).toEqual(0);
204+
});
205+
206+
it.each([400, 408, 429])('does reconnect after a recoverable error', async status => {
207+
const config = { ...defaultConfig, streamReconnectDelay: 1, useReport: false };
208+
const stream = new Stream(platform, config, envName);
209+
stream.connect(user);
210+
211+
const created = await platform.testing.expectStream();
212+
const es = created.eventSource;
213+
214+
expect(es.readyState).toBe(EventSource.CONNECTING);
215+
es.mockOpen();
216+
expect(es.readyState).toBe(EventSource.OPEN);
217+
218+
es.mockError({ status });
219+
await sleepAsync(10);
220+
expect(platform.testing.eventSourcesCreated.length()).toEqual(1);
221+
});
222+
188223
it('logs a warning for only the first failed connection attempt', async () => {
189224
const config = { ...defaultConfig, streamReconnectDelay: 1 };
190225
const stream = new Stream(platform, config, envName);
@@ -197,6 +232,7 @@ describe('Stream', () => {
197232
const nAttempts = 5;
198233
for (let i = 0; i < nAttempts; i++) {
199234
es.mockError('test error');
235+
await sleepAsync(10);
200236
const created1 = await platform.testing.expectStream();
201237
es = created1.eventSource;
202238
es.mockOpen();
@@ -221,6 +257,7 @@ describe('Stream', () => {
221257
const nAttempts = 5;
222258
for (let i = 0; i < nAttempts; i++) {
223259
es.mockError('test error #1');
260+
await sleepAsync(10);
224261
const created1 = await platform.testing.expectStream();
225262
es = created1.eventSource;
226263
es.mockOpen();
@@ -232,15 +269,16 @@ describe('Stream', () => {
232269

233270
for (let i = 0; i < nAttempts; i++) {
234271
es.mockError('test error #2');
272+
await sleepAsync(10);
235273
const created1 = await platform.testing.expectStream();
236274
es = created1.eventSource;
237275
es.mockOpen();
238276
}
239277

240278
// make sure there is just a single logged message rather than five (one per attempt)
241279
expect(logger.output.warn).toEqual([
242-
messages.streamError('test error #1', 1),
243-
messages.streamError('test error #2', 1),
280+
expect.stringContaining('test error #1'),
281+
expect.stringContaining('test error #2'),
244282
]);
245283
});
246284

src/headers.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ export function getLDHeaders(platform, options) {
1818
if (tagKeys.length) {
1919
h['x-launchdarkly-tags'] = tagKeys
2020
.sort()
21-
.flatMap(
21+
.map(
2222
key => (Array.isArray(tags[key]) ? tags[key].sort().map(value => `${key}/${value}`) : [`${key}/${tags[key]}`])
2323
)
24+
.reduce((flattened, item) => flattened.concat(item), [])
2425
.join(' ');
2526
}
2627
return h;

src/messages.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,16 @@ export const streamError = function(err, streamReconnectDelay) {
123123
return (
124124
'Error on stream connection: ' +
125125
errorString(err) +
126-
', will continue retrying every ' +
126+
', will continue retrying after ' +
127127
streamReconnectDelay +
128128
' milliseconds.'
129129
);
130130
};
131131

132+
export const unrecoverableStreamError = function(err) {
133+
return `Error on stream connection ${errorString(err)}, giving up permanently`;
134+
};
135+
132136
export const unknownOption = name => 'Ignoring unknown config option "' + name + '"';
133137

134138
export const wrongOptionType = (name, expectedType, actualType) =>

0 commit comments

Comments
 (0)