-
Notifications
You must be signed in to change notification settings - Fork 23
FFM-11788 Add maxStreamRetries
config option
#126
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
Merged
Merged
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
d266094
FFM-11788 Start adding max stream retry config
erdirowlands 0e7a0c6
FFM-11788 Add max stream retry to stream.ts
erdirowlands 5c89f2c
FFM-11788 Log if we stay in polling mode
erdirowlands fc480d8
FFM-11788 Add tests for streaming class
erdirowlands 2bea6aa
FFM-11788 Add tests for streaming class
erdirowlands 36346b8
FFM-11788 Default tests to infinite retries
erdirowlands 1e16259
FFM-11788 Update readme
erdirowlands d993822
FFM-11788 Change config name
erdirowlands 1ebf405
FFM-11788 Remove typo
erdirowlands 11080b9
FFM-11788 Update error log
erdirowlands a223f18
Merge branch 'main' into FFM-11788
erdirowlands 841b4af
FFM-11788 RC prep
erdirowlands 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 hidden or 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 hidden or 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,247 @@ | ||
import { Streamer } from '../stream' | ||
import type { Options } from '../types' | ||
import { Event } from '../types' | ||
import { getRandom } from '../utils' | ||
import type { Emitter } from 'mitt' | ||
import type Poller from "../poller"; | ||
|
||
jest.useFakeTimers() | ||
|
||
jest.mock('../utils.ts', () => ({ | ||
getRandom: jest.fn() | ||
})) | ||
|
||
const mockEventBus: Emitter = { | ||
emit: jest.fn(), | ||
on: jest.fn(), | ||
off: jest.fn(), | ||
all: new Map() | ||
} | ||
|
||
const mockXHR = { | ||
open: jest.fn(), | ||
setRequestHeader: jest.fn(), | ||
send: jest.fn(), | ||
abort: jest.fn(), | ||
status: 0, | ||
responseText: '', | ||
onload: null, | ||
onerror: null, | ||
onprogress: null, | ||
onabort: null, | ||
ontimeout: null | ||
} | ||
|
||
global.XMLHttpRequest = jest.fn(() => mockXHR) as unknown as jest.MockedClass<typeof XMLHttpRequest> | ||
|
||
const logError = jest.fn() | ||
const logDebug = jest.fn() | ||
|
||
const getStreamer = (overrides: Partial<Options> = {}, maxRetries: number = Infinity): Streamer => { | ||
const options: Options = { | ||
baseUrl: 'http://test', | ||
eventUrl: 'http://event', | ||
pollingInterval: 60000, | ||
debug: true, | ||
pollingEnabled: true, | ||
streamEnabled: true, | ||
...overrides | ||
} | ||
|
||
return new Streamer( | ||
mockEventBus, | ||
options, | ||
`${options.baseUrl}/stream`, | ||
'test-api-key', | ||
{ 'Test-Header': 'value' }, | ||
{ start: jest.fn(), stop: jest.fn(), isPolling: jest.fn() } as unknown as Poller, | ||
logDebug, | ||
logError, | ||
jest.fn(), | ||
maxRetries | ||
) | ||
} | ||
|
||
describe('Streamer', () => { | ||
beforeEach(() => { | ||
jest.clearAllMocks() | ||
}) | ||
|
||
it('should connect and emit CONNECTED event', () => { | ||
const streamer = getStreamer({}, 3) | ||
|
||
streamer.start() | ||
expect(mockXHR.open).toHaveBeenCalledWith('GET', 'http://test/stream') | ||
expect(mockXHR.send).toHaveBeenCalled() | ||
|
||
mockXHR.onprogress({} as ProgressEvent) | ||
expect(mockEventBus.emit).toHaveBeenCalledWith(Event.CONNECTED) | ||
}) | ||
|
||
it('should reconnect successfully after multiple failures', () => { | ||
const streamer = getStreamer({}, 5) | ||
|
||
streamer.start() | ||
expect(mockXHR.send).toHaveBeenCalled() | ||
|
||
for (let i = 0; i < 3; i++) { | ||
mockXHR.onerror({} as ProgressEvent) | ||
jest.advanceTimersByTime(getRandom(1000, 10000)) | ||
} | ||
|
||
// Simulate a successful connection on the next attempt | ||
mockXHR.onprogress({} as ProgressEvent) | ||
|
||
expect(mockEventBus.emit).toHaveBeenCalledWith(Event.CONNECTED) | ||
expect(mockXHR.send).toHaveBeenCalledTimes(4) // Should attempt to reconnect 3 times before succeeding | ||
}) | ||
|
||
it('should retry connecting on error and eventually fallback to polling', () => { | ||
const streamer = getStreamer() | ||
|
||
streamer.start() | ||
expect(mockXHR.send).toHaveBeenCalled() | ||
|
||
for (let i = 0; i < 3; i++) { | ||
mockXHR.onerror({} as ProgressEvent) | ||
jest.advanceTimersByTime(getRandom(1000, 10000)) | ||
} | ||
|
||
expect(mockEventBus.emit).toHaveBeenCalledWith(Event.DISCONNECTED) | ||
}) | ||
|
||
it('should not retry after max retries are exhausted', () => { | ||
const streamer = getStreamer({}, 3) | ||
|
||
streamer.start() | ||
expect(mockXHR.send).toHaveBeenCalled() | ||
|
||
for (let i = 0; i < 3; i++) { | ||
mockXHR.onerror({} as ProgressEvent) | ||
jest.advanceTimersByTime(getRandom(1000, 10000)) | ||
} | ||
|
||
mockXHR.onerror({} as ProgressEvent) | ||
expect(logError).toHaveBeenCalledWith('Streaming: Max streaming retries reached. Staying in polling mode.') | ||
expect(mockEventBus.emit).toHaveBeenCalledWith(Event.DISCONNECTED) | ||
expect(mockXHR.send).toHaveBeenCalledTimes(3) // Should not send after max retries | ||
}) | ||
|
||
it('should fallback to polling on stream failure', () => { | ||
const poller = { start: jest.fn(), stop: jest.fn(), isPolling: jest.fn() } as unknown as Poller | ||
const streamer = new Streamer( | ||
mockEventBus, | ||
{ baseUrl: 'http://test', eventUrl: 'http://event', pollingEnabled: true, streamEnabled: true, debug: true }, | ||
'http://test/stream', | ||
'test-api-key', | ||
{ 'Test-Header': 'value' }, | ||
poller, | ||
logDebug, | ||
logError, | ||
jest.fn(), | ||
Infinity | ||
) | ||
|
||
streamer.start() | ||
expect(mockXHR.send).toHaveBeenCalled() | ||
|
||
mockXHR.onerror({} as ProgressEvent) | ||
jest.advanceTimersByTime(getRandom(1000, 10000)) | ||
|
||
expect(poller.start).toHaveBeenCalled() | ||
expect(logDebug).toHaveBeenCalledWith('Streaming: Falling back to polling mode while stream recovers') | ||
}) | ||
|
||
it('should stop polling when close is called if in fallback polling mode', () => { | ||
const poller = { start: jest.fn(), stop: jest.fn(), isPolling: jest.fn() } as unknown as Poller | ||
;(poller.isPolling as jest.Mock) | ||
.mockImplementationOnce(() => false) | ||
.mockImplementationOnce(() => true) | ||
|
||
const streamer = new Streamer( | ||
mockEventBus, | ||
{ baseUrl: 'http://test', eventUrl: 'http://event', pollingEnabled: true, streamEnabled: true, debug: true }, | ||
'http://test/stream', | ||
'test-api-key', | ||
{ 'Test-Header': 'value' }, | ||
poller, | ||
logDebug, | ||
logError, | ||
jest.fn(), | ||
3 | ||
) | ||
|
||
streamer.start() | ||
expect(mockXHR.send).toHaveBeenCalled() | ||
|
||
// Simulate stream failure and fallback to polling | ||
mockXHR.onerror({} as ProgressEvent) | ||
jest.advanceTimersByTime(getRandom(1000, 10000)) | ||
|
||
// Ensure polling has started | ||
expect(poller.start).toHaveBeenCalled() | ||
|
||
// Now close the streamer | ||
streamer.close() | ||
|
||
expect(mockXHR.abort).toHaveBeenCalled() | ||
expect(poller.stop).toHaveBeenCalled() | ||
expect(mockEventBus.emit).toHaveBeenCalledWith(Event.STOPPED) | ||
}) | ||
|
||
it('should stop streaming but not call poller.stop if not in fallback polling mode when close is called', () => { | ||
const poller = { start: jest.fn(), stop: jest.fn(), isPolling: jest.fn().mockReturnValue(false) } as unknown as Poller | ||
const streamer = new Streamer( | ||
mockEventBus, | ||
{ baseUrl: 'http://test', eventUrl: 'http://event', pollingEnabled: true, streamEnabled: true, debug: true }, | ||
'http://test/stream', | ||
'test-api-key', | ||
{ 'Test-Header': 'value' }, | ||
poller, | ||
logDebug, | ||
logError, | ||
jest.fn(), | ||
3 | ||
) | ||
|
||
streamer.start() | ||
streamer.close() | ||
|
||
expect(mockXHR.abort).toHaveBeenCalled() | ||
expect(poller.stop).not.toHaveBeenCalled() | ||
expect(mockEventBus.emit).toHaveBeenCalledWith(Event.STOPPED) | ||
}) | ||
|
||
it('should retry indefinitely if maxRetries is set to Infinity', () => { | ||
const streamer = getStreamer() | ||
|
||
streamer.start() | ||
expect(mockXHR.send).toHaveBeenCalled() | ||
|
||
for (let i = 0; i < 100; i++) { | ||
mockXHR.onerror({} as ProgressEvent) | ||
jest.advanceTimersByTime(getRandom(1000, 10000)) | ||
} | ||
|
||
expect(logError).not.toHaveBeenCalledWith('Streaming: Max streaming retries reached. Staying in polling mode.') | ||
expect(mockXHR.send).toHaveBeenCalledTimes(101) | ||
}) | ||
|
||
it('should reconnect successfully after multiple failures', () => { | ||
const streamer = getStreamer({}, 5) | ||
|
||
streamer.start() | ||
expect(mockXHR.send).toHaveBeenCalled() | ||
|
||
for (let i = 0; i < 3; i++) { | ||
mockXHR.onerror({} as ProgressEvent) | ||
jest.advanceTimersByTime(getRandom(1000, 10000)) | ||
} | ||
|
||
// Simulate a successful connection on the next attempt | ||
mockXHR.onprogress({} as ProgressEvent) | ||
|
||
expect(mockEventBus.emit).toHaveBeenCalledWith(Event.CONNECTED) | ||
expect(mockXHR.send).toHaveBeenCalledTimes(4) // Should attempt to reconnect 3 times before succeeding | ||
}) | ||
}) |
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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
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.
Wondering would it be a good idea to indicate that polling will also be stopped in this
else
statement log so that users would know they're gonna stop receiving updates?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.
Thanks - good catch.
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.
Updated