Skip to content

Commit 52055ba

Browse files
authored
fix: Make it more clear what is happening when an event source is connecting. (#518)
The event source was treating the initial connection as a re-connection. This results in confusing messaging. This also changes the retrying event, which we do not expose. When we convert our polyfill to typescript we should use it instead and allow passing a connection method. The reason this is being used is theoretically an issue with fetch not streaming in RN, so it uses an XHR instead.
1 parent 84d68e6 commit 52055ba

File tree

2 files changed

+35
-18
lines changed

2 files changed

+35
-18
lines changed

packages/sdk/react-native/src/fromExternal/react-native-sse/EventSource.test.ts

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import EventSource, { backoff, jitter } from './EventSource';
66
describe('EventSource', () => {
77
const uri = 'https://mock.events.uri';
88
let eventSource: EventSource<EventName>;
9+
let mockXhr: any;
910

1011
beforeAll(() => {
1112
jest.useFakeTimers();
@@ -17,9 +18,17 @@ describe('EventSource', () => {
1718
.mockImplementationOnce(() => 0.888)
1819
.mockImplementationOnce(() => 0.999);
1920

21+
mockXhr = {
22+
open: jest.fn(),
23+
send: jest.fn(),
24+
setRequestHeader: jest.fn(),
25+
abort: jest.fn(),
26+
};
27+
28+
jest.spyOn(window, 'XMLHttpRequest').mockImplementation(() => mockXhr as XMLHttpRequest);
29+
2030
eventSource = new EventSource<EventName>(uri, { logger });
2131
eventSource.onclose = jest.fn();
22-
eventSource.open = jest.fn();
2332
eventSource.onretrying = jest.fn();
2433
});
2534

@@ -66,18 +75,17 @@ describe('EventSource', () => {
6675
expect(delay1).toEqual(1001);
6776
});
6877

69-
test('tryConnect force no delay', () => {
70-
// @ts-ignore
71-
eventSource.tryConnect(true);
78+
test('initial connection', () => {
7279
jest.runAllTimers();
7380

74-
expect(logger.debug).toHaveBeenCalledWith(expect.stringMatching(/new connection in 0 ms/i));
75-
expect(eventSource.onretrying).toHaveBeenCalledWith({ type: 'retry', delayMillis: 0 });
76-
expect(eventSource.open).toHaveBeenCalledTimes(1);
77-
expect(eventSource.onclose).toHaveBeenCalledTimes(1);
81+
expect(logger.debug).toHaveBeenCalledWith(expect.stringMatching(/\[EventSource\] opening new connection./));
82+
expect(mockXhr.open).toHaveBeenCalledTimes(1);
83+
expect(eventSource.onclose).toHaveBeenCalledTimes(0);
7884
});
7985

8086
test('tryConnect with delay', () => {
87+
jest.runAllTimers();
88+
// This forces it to reconnect.
8189
// @ts-ignore
8290
eventSource.tryConnect();
8391
jest.runAllTimers();
@@ -87,7 +95,8 @@ describe('EventSource', () => {
8795
expect.stringMatching(/new connection in 556 ms/i),
8896
);
8997
expect(eventSource.onretrying).toHaveBeenCalledWith({ type: 'retry', delayMillis: 556 });
90-
expect(eventSource.open).toHaveBeenCalledTimes(1);
98+
// Initial connection + forced reconnect.
99+
expect(mockXhr.open).toHaveBeenCalledTimes(2);
91100
expect(eventSource.onclose).toHaveBeenCalledTimes(1);
92101
});
93102
});

packages/sdk/react-native/src/fromExternal/react-native-sse/EventSource.ts

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export default class EventSource<E extends string = never> {
5757
private body: any;
5858
private url: string;
5959
private xhr: XMLHttpRequest = new XMLHttpRequest();
60-
private pollTimer: any;
60+
private connectTimer: any;
6161
private retryAndHandleError?: (err: any) => boolean;
6262
private initialRetryDelayMillis: number = 1000;
6363
private retryCount: number = 0;
@@ -88,17 +88,25 @@ export default class EventSource<E extends string = never> {
8888
return delay;
8989
}
9090

91-
private tryConnect(forceNoDelay: boolean = false) {
92-
let delay = forceNoDelay ? 0 : this.getNextRetryDelay();
93-
this.logger?.debug(`[EventSource] Will open new connection in ${delay} ms.`);
94-
this.dispatch('retry', { type: 'retry', delayMillis: delay });
95-
this.pollTimer = setTimeout(() => {
96-
this.close();
91+
private tryConnect(initialConnection: boolean = false) {
92+
let delay = initialConnection ? 0 : this.getNextRetryDelay();
93+
if(initialConnection) {
94+
this.logger?.debug(`[EventSource] opening new connection.`)
95+
} else {
96+
this.logger?.debug(`[EventSource] Will open new connection in ${delay} ms.`);
97+
this.dispatch('retry', { type: 'retry', delayMillis: delay });
98+
}
99+
100+
this.connectTimer = setTimeout(() => {
101+
if(!initialConnection) {
102+
this.close();
103+
}
104+
97105
this.open();
98106
}, delay);
99107
}
100108

101-
open() {
109+
private open() {
102110
try {
103111
this.lastIndexProcessed = 0;
104112
this.status = this.CONNECTING;
@@ -329,7 +337,7 @@ export default class EventSource<E extends string = never> {
329337

330338
close() {
331339
this.status = this.CLOSED;
332-
clearTimeout(this.pollTimer);
340+
clearTimeout(this.connectTimer);
333341
if (this.xhr) {
334342
this.xhr.abort();
335343
}

0 commit comments

Comments
 (0)