-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Implement RN SDK EventSource jitter backoff. #359
Merged
yusinto
merged 36 commits into
main
from
yus/sc-225912/implement-rn-eventsource-jitter-backoff
Feb 1, 2024
Merged
Changes from all commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
2c7ac96
feat: Implement common support for auto environment attributes.
yusinto 53d4358
feat: Implement common client side support for auto environment attri…
yusinto dc3f3af
chore: Fixed server sdk tests due to mock api changes for auto env.
yusinto e552def
feat: React-native support for auto-env attributes.
yusinto 2028f4c
chore: Fix broken common tests due to mocks api changes.
yusinto e8989f1
Merge branch 'yus/client-sdk-auto-env' into yus/rn-sdk-auto-env
yusinto 23a11dc
Merge branch 'main' into yus/client-sdk-auto-env
yusinto 1156f25
Merge branch 'yus/client-sdk-auto-env' into yus/rn-sdk-auto-env
yusinto adc1de1
fix: Remove sdk data as fallback for auto env. Strip falsy values and…
yusinto 04bb98f
fix: Respect customer provided ld_application and ld_device contexts.
yusinto 75bb880
fix: Add mandatory autoEnvAttributes argument to LDClient constructor.
yusinto 5243f3f
chore: Move AutoEnvAttributes enum to common.
yusinto 22b97bd
fix: Make all device.os properties optional.
yusinto d365184
fix: Added mandatory AutoEnvAttributes constructor arg.
yusinto b47a5e2
chore: Log warning if auto env attributes are not added because they …
yusinto d1f0310
Merge branch 'yus/client-sdk-auto-env' into yus/rn-sdk-auto-env
yusinto ebc670a
chore: Remove unused import in platform mock.
yusinto b5ce097
chore: Fix duplicated test name.
yusinto ccb8419
fix: Implemented separate namespaces for anon and contexts. Fixed bug…
yusinto 125f82b
Merge branch 'yus/client-sdk-auto-env' into yus/rn-sdk-auto-env
yusinto 4e7f1c1
fix: Hardcode @types/node version to avoid crypto typings but in v20.…
yusinto caa1f14
chore: Add comment to explain harcoding of @types.node.
yusinto 6ef3a87
Merge branch 'yus/client-sdk-auto-env' into yus/rn-sdk-auto-env
yusinto 4375509
Merge branch 'yus/client-sdk-auto-env' into yus/rn-sdk-auto-env
yusinto ef15c18
Merge branch 'yus/rn-sdk-auto-env' of github.com:launchdarkly/js-core…
yusinto 189d379
fix: Import AutoEnvAttributes from the rn sdk instead of client common.
yusinto 31643eb
fix: Add tsconfig jsx react setting.
yusinto 1788a21
chore: Remove hardcoded @types/node version.
yusinto f851823
Merge branch 'yus/client-sdk-auto-env' into yus/rn-sdk-auto-env
yusinto d9dc2e5
chore: Deleted redundant react-native-sse files and folder.
yusinto f6ca89c
fix: Added rn sdk streaming jitter backoff.
yusinto 50c884c
chore: Added backoff jitter tests.
yusinto aaa7f94
chore: Use Math.min instead of logical operators for backoff.
yusinto 05f52b6
chore: Fixed minor type import.
yusinto eb2a682
Merge branch 'main' into yus/sc-225912/implement-rn-eventsource-jitte…
yusinto b50c904
chore: Remove trailing commas.
yusinto File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
90 changes: 90 additions & 0 deletions
90
packages/sdk/react-native/src/fromExternal/react-native-sse/EventSource.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
import { type EventName } from '@launchdarkly/js-client-sdk-common'; | ||
import { logger } from '@launchdarkly/private-js-mocks'; | ||
|
||
import EventSource, { backoff, jitter } from './EventSource'; | ||
|
||
describe('EventSource', () => { | ||
const uri = 'https://mock.events.uri'; | ||
let eventSource: EventSource<EventName>; | ||
|
||
beforeAll(() => { | ||
jest.useFakeTimers(); | ||
}); | ||
|
||
beforeEach(() => { | ||
jest | ||
.spyOn(Math, 'random') | ||
.mockImplementationOnce(() => 0.888) | ||
.mockImplementationOnce(() => 0.999); | ||
|
||
eventSource = new EventSource<EventName>(uri, { logger }); | ||
eventSource.open = jest.fn(); | ||
eventSource.onretrying = jest.fn(); | ||
}); | ||
|
||
afterEach(() => { | ||
// GOTCHA: Math.random must be reset separately because of a source-map type error | ||
// https://medium.com/orchestrated/updating-react-to-version-17-471bfbe6bfcd | ||
jest.spyOn(Math, 'random').mockRestore(); | ||
|
||
jest.resetAllMocks(); | ||
}); | ||
|
||
test('backoff exponentially', () => { | ||
const delay0 = backoff(1000, 0); | ||
const delay1 = backoff(1000, 1); | ||
const delay2 = backoff(1000, 2); | ||
|
||
expect(delay0).toEqual(1000); | ||
expect(delay1).toEqual(2000); | ||
expect(delay2).toEqual(4000); | ||
}); | ||
|
||
test('backoff returns max delay', () => { | ||
const delay = backoff(1000, 5); | ||
expect(delay).toEqual(30000); | ||
}); | ||
|
||
test('jitter', () => { | ||
const delay0 = jitter(1000); | ||
const delay1 = jitter(2000); | ||
|
||
expect(delay0).toEqual(556); | ||
expect(delay1).toEqual(1001); | ||
}); | ||
|
||
test('getNextRetryDelay', () => { | ||
// @ts-ignore | ||
const delay0 = eventSource.getNextRetryDelay(); | ||
// @ts-ignore | ||
const delay1 = eventSource.getNextRetryDelay(); | ||
|
||
// @ts-ignore | ||
expect(eventSource.retryCount).toEqual(2); | ||
expect(delay0).toEqual(556); | ||
expect(delay1).toEqual(1001); | ||
}); | ||
|
||
test('tryConnect force no delay', () => { | ||
// @ts-ignore | ||
eventSource.tryConnect(true); | ||
jest.runAllTimers(); | ||
|
||
expect(logger.debug).toHaveBeenCalledWith(expect.stringMatching(/new connection in 0 ms/i)); | ||
expect(eventSource.onretrying).toHaveBeenCalledWith({ type: 'retry', delayMillis: 0 }); | ||
expect(eventSource.open).toHaveBeenCalledTimes(2); | ||
}); | ||
|
||
test('tryConnect with delay', () => { | ||
// @ts-ignore | ||
eventSource.tryConnect(); | ||
jest.runAllTimers(); | ||
|
||
expect(logger.debug).toHaveBeenNthCalledWith( | ||
2, | ||
expect.stringMatching(/new connection in 556 ms/i), | ||
); | ||
expect(eventSource.onretrying).toHaveBeenCalledWith({ type: 'retry', delayMillis: 556 }); | ||
expect(eventSource.open).toHaveBeenCalledTimes(2); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,16 +12,27 @@ const XMLReadyStateMap = ['UNSENT', 'OPENED', 'HEADERS_RECEIVED', 'LOADING', 'DO | |
|
||
const defaultOptions: EventSourceOptions = { | ||
body: undefined, | ||
debug: false, | ||
headers: {}, | ||
method: 'GET', | ||
pollingInterval: 5000, | ||
timeout: 0, | ||
timeoutBeforeConnection: 0, | ||
withCredentials: false, | ||
retryAndHandleError: undefined, | ||
initialRetryDelayMillis: 1000, | ||
logger: undefined, | ||
}; | ||
|
||
const maxRetryDelay = 30 * 1000; // Maximum retry delay 30 seconds. | ||
const jitterRatio = 0.5; // Delay should be 50%-100% of calculated time. | ||
|
||
export function backoff(base: number, retryCount: number) { | ||
const delay = base * Math.pow(2, retryCount); | ||
return Math.min(delay, maxRetryDelay); | ||
} | ||
|
||
export function jitter(computedDelayMillis: number) { | ||
return computedDelayMillis - Math.trunc(Math.random() * jitterRatio * computedDelayMillis); | ||
} | ||
|
||
export default class EventSource<E extends string = never> { | ||
ERROR = -1; | ||
CONNECTING = 0; | ||
|
@@ -41,16 +52,16 @@ export default class EventSource<E extends string = never> { | |
|
||
private method: string; | ||
private timeout: number; | ||
private timeoutBeforeConnection: number; | ||
private withCredentials: boolean; | ||
private headers: Record<string, any>; | ||
private body: any; | ||
private debug: boolean; | ||
private url: string; | ||
private xhr: XMLHttpRequest = new XMLHttpRequest(); | ||
private pollTimer: any; | ||
private pollingInterval: number; | ||
private retryAndHandleError?: (err: any) => boolean; | ||
private initialRetryDelayMillis: number = 1000; | ||
private retryCount: number = 0; | ||
private logger?: any; | ||
|
||
constructor(url: string, options?: EventSourceOptions) { | ||
const opts = { | ||
|
@@ -61,25 +72,29 @@ export default class EventSource<E extends string = never> { | |
this.url = url; | ||
this.method = opts.method!; | ||
this.timeout = opts.timeout!; | ||
this.timeoutBeforeConnection = opts.timeoutBeforeConnection!; | ||
this.withCredentials = opts.withCredentials!; | ||
this.headers = opts.headers!; | ||
this.body = opts.body; | ||
this.debug = opts.debug!; | ||
this.pollingInterval = opts.pollingInterval!; | ||
this.retryAndHandleError = opts.retryAndHandleError; | ||
this.initialRetryDelayMillis = opts.initialRetryDelayMillis!; | ||
this.logger = opts.logger; | ||
|
||
this.pollAgain(this.timeoutBeforeConnection, true); | ||
this.tryConnect(true); | ||
} | ||
|
||
private pollAgain(time: number, allowZero: boolean) { | ||
if (time > 0 || allowZero) { | ||
this.logDebug(`[EventSource] Will open new connection in ${time} ms.`); | ||
this.dispatch('retry', { type: 'retry' }); | ||
this.pollTimer = setTimeout(() => { | ||
this.open(); | ||
}, time); | ||
} | ||
private getNextRetryDelay() { | ||
const delay = jitter(backoff(this.initialRetryDelayMillis, this.retryCount)); | ||
this.retryCount += 1; | ||
return delay; | ||
} | ||
|
||
private tryConnect(forceNoDelay: boolean = false) { | ||
let delay = forceNoDelay ? 0 : this.getNextRetryDelay(); | ||
this.logger?.debug(`[EventSource] Will open new connection in ${delay} ms.`); | ||
this.dispatch('retry', { type: 'retry', delayMillis: delay }); | ||
this.pollTimer = setTimeout(() => { | ||
this.open(); | ||
}, delay); | ||
} | ||
|
||
open() { | ||
|
@@ -113,7 +128,7 @@ export default class EventSource<E extends string = never> { | |
return; | ||
} | ||
|
||
this.logDebug( | ||
this.logger?.debug( | ||
`[EventSource][onreadystatechange] ReadyState: ${ | ||
XMLReadyStateMap[this.xhr.readyState] || 'Unknown' | ||
}(${this.xhr.readyState}), status: ${this.xhr.status}`, | ||
|
@@ -128,16 +143,18 @@ export default class EventSource<E extends string = never> { | |
|
||
if (this.xhr.status >= 200 && this.xhr.status < 400) { | ||
if (this.status === this.CONNECTING) { | ||
this.retryCount = 0; | ||
this.status = this.OPEN; | ||
this.dispatch('open', { type: 'open' }); | ||
this.logDebug('[EventSource][onreadystatechange][OPEN] Connection opened.'); | ||
this.logger?.debug('[EventSource][onreadystatechange][OPEN] Connection opened.'); | ||
} | ||
|
||
// retry from server gets set here | ||
this.handleEvent(this.xhr.responseText || ''); | ||
|
||
if (this.xhr.readyState === XMLHttpRequest.DONE) { | ||
this.logDebug('[EventSource][onreadystatechange][DONE] Operation done.'); | ||
this.pollAgain(this.pollingInterval, false); | ||
this.logger?.debug('[EventSource][onreadystatechange][DONE] Operation done.'); | ||
this.tryConnect(); | ||
} | ||
} else if (this.xhr.status !== 0) { | ||
this.status = this.ERROR; | ||
|
@@ -149,20 +166,20 @@ export default class EventSource<E extends string = never> { | |
}); | ||
|
||
if (this.xhr.readyState === XMLHttpRequest.DONE) { | ||
this.logDebug('[EventSource][onreadystatechange][ERROR] Response status error.'); | ||
this.logger?.debug('[EventSource][onreadystatechange][ERROR] Response status error.'); | ||
|
||
if (!this.retryAndHandleError) { | ||
// default implementation | ||
this.pollAgain(this.pollingInterval, false); | ||
// by default just try and reconnect if there's an error. | ||
this.tryConnect(); | ||
} else { | ||
// custom retry logic | ||
// custom retry logic taking into account status codes. | ||
const shouldRetry = this.retryAndHandleError({ | ||
status: this.xhr.status, | ||
message: this.xhr.responseText, | ||
}); | ||
|
||
if (shouldRetry) { | ||
this.pollAgain(this.pollingInterval, true); | ||
this.tryConnect(); | ||
} | ||
} | ||
} | ||
|
@@ -207,13 +224,6 @@ export default class EventSource<E extends string = never> { | |
} | ||
} | ||
|
||
private logDebug(...msg: string[]) { | ||
if (this.debug) { | ||
// eslint-disable-next-line no-console | ||
console.debug(...msg); | ||
} | ||
} | ||
|
||
private handleEvent(response: string) { | ||
const parts = response.slice(this.lastIndexProcessed).split('\n'); | ||
|
||
|
@@ -234,7 +244,8 @@ export default class EventSource<E extends string = never> { | |
} else if (line.indexOf('retry') === 0) { | ||
retry = parseInt(line.replace(/retry:?\s*/, ''), 10); | ||
if (!Number.isNaN(retry)) { | ||
this.pollingInterval = retry; | ||
// GOTCHA: Ignore the server retry recommendation. Use our own custom getNextRetryDelay logic. | ||
// this.pollingInterval = retry; | ||
Comment on lines
+247
to
+248
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The original source code uses the server recommended retry seconds but we don't need that for our purpose. |
||
} | ||
} else if (line.indexOf('data') === 0) { | ||
data.push(line.replace(/data:?\s*/, '')); | ||
|
@@ -307,7 +318,7 @@ export default class EventSource<E extends string = never> { | |
this.onerror(data); | ||
break; | ||
case 'retry': | ||
this.onretrying({ delayMillis: this.pollingInterval }); | ||
this.onretrying(data); | ||
break; | ||
default: | ||
break; | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically we would only reset this after having been active for 1 minute.