Skip to content

Commit 29aefb7

Browse files
committed
fix: error responses are not propagated in RN eventsource
1 parent 6604149 commit 29aefb7

3 files changed

Lines changed: 213 additions & 11 deletions

File tree

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

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,4 +223,149 @@ describe('EventSource', () => {
223223

224224
expect(onopen).toHaveBeenCalledTimes(1);
225225
});
226+
227+
test('calls retryAndHandleError with parsed response headers on an error response', () => {
228+
const retryAndHandleError = jest.fn(() => false);
229+
230+
mockXhr.getAllResponseHeaders = jest.fn(
231+
() => 'X-Ld-Fd-Fallback: true\r\nX-Ld-Fd-Fallback-Ttl: 60\r\nContent-Type: text/event-stream',
232+
);
233+
mockXhr.responseText = 'error body';
234+
235+
const es = new EventSource<EventName>(uri, { logger, retryAndHandleError });
236+
es.onerror = jest.fn();
237+
238+
jest.runAllTimers();
239+
240+
mockXhr.readyState = 4;
241+
mockXhr.status = 500;
242+
mockXhr.onreadystatechange();
243+
244+
expect(retryAndHandleError).toHaveBeenCalledTimes(1);
245+
expect(retryAndHandleError).toHaveBeenCalledWith(
246+
expect.objectContaining({
247+
status: 500,
248+
message: 'error body',
249+
headers: expect.objectContaining({
250+
'x-ld-fd-fallback': 'true',
251+
'x-ld-fd-fallback-ttl': '60',
252+
}),
253+
}),
254+
);
255+
});
256+
257+
test('dispatches error with status and headers from onprogress mid-stream', () => {
258+
const onerror = jest.fn();
259+
260+
mockXhr.getAllResponseHeaders = jest.fn(
261+
() => 'X-Ld-Fd-Fallback: true\r\nX-Ld-Fd-Fallback-Ttl: 60\r\nContent-Type: text/event-stream',
262+
);
263+
mockXhr.responseText = 'error body';
264+
265+
eventSource.onerror = onerror;
266+
267+
jest.runAllTimers();
268+
269+
// readyState 3 (LOADING), not DONE, so this exercises onprogress's
270+
// error branch rather than onreadystatechange's.
271+
mockXhr.readyState = 3;
272+
mockXhr.status = 500;
273+
mockXhr.onprogress();
274+
275+
expect(onerror).toHaveBeenCalledWith(
276+
expect.objectContaining({
277+
status: 500,
278+
headers: expect.objectContaining({
279+
'x-ld-fd-fallback': 'true',
280+
'x-ld-fd-fallback-ttl': '60',
281+
}),
282+
}),
283+
);
284+
});
285+
286+
test('dispatches error with status and headers from onreadystatechange before retrying', () => {
287+
const onerror = jest.fn();
288+
289+
mockXhr.getAllResponseHeaders = jest.fn(
290+
() => 'X-Ld-Fd-Fallback: true\r\nX-Ld-Fd-Fallback-Ttl: 60\r\nContent-Type: text/event-stream',
291+
);
292+
mockXhr.responseText = 'error body';
293+
294+
eventSource.onerror = onerror;
295+
296+
jest.runAllTimers();
297+
298+
mockXhr.readyState = 4;
299+
mockXhr.status = 500;
300+
mockXhr.onreadystatechange();
301+
302+
expect(onerror).toHaveBeenCalledWith(
303+
expect.objectContaining({
304+
status: 500,
305+
headers: expect.objectContaining({
306+
'x-ld-fd-fallback': 'true',
307+
'x-ld-fd-fallback-ttl': '60',
308+
}),
309+
}),
310+
);
311+
});
312+
313+
test('invokes retryAndHandleError from onprogress when the connection never reaches DONE', () => {
314+
const retryAndHandleError = jest.fn(() => false);
315+
316+
mockXhr.getAllResponseHeaders = jest.fn(
317+
() => 'X-Ld-Fd-Fallback: true\r\nX-Ld-Fd-Fallback-Ttl: 60\r\nContent-Type: text/event-stream',
318+
);
319+
mockXhr.responseText = 'error body';
320+
321+
const es = new EventSource<EventName>(uri, { logger, retryAndHandleError });
322+
es.onerror = jest.fn();
323+
324+
jest.runAllTimers();
325+
326+
// readyState never advances to DONE for this attempt, so
327+
// onreadystatechange never fires; onprogress must drive the retry
328+
// on its own.
329+
mockXhr.readyState = 3;
330+
mockXhr.status = 500;
331+
mockXhr.onprogress();
332+
333+
expect(retryAndHandleError).toHaveBeenCalledTimes(1);
334+
expect(retryAndHandleError).toHaveBeenCalledWith(
335+
expect.objectContaining({
336+
status: 500,
337+
message: 'error body',
338+
headers: expect.objectContaining({
339+
'x-ld-fd-fallback': 'true',
340+
'x-ld-fd-fallback-ttl': '60',
341+
}),
342+
}),
343+
);
344+
});
345+
346+
test('does not invoke retryAndHandleError twice when onprogress and onreadystatechange observe the same failed attempt', () => {
347+
const retryAndHandleError = jest.fn(() => false);
348+
349+
mockXhr.getAllResponseHeaders = jest.fn(
350+
() => 'X-Ld-Fd-Fallback: true\r\nX-Ld-Fd-Fallback-Ttl: 60\r\nContent-Type: text/event-stream',
351+
);
352+
mockXhr.responseText = 'error body';
353+
354+
const es = new EventSource<EventName>(uri, { logger, retryAndHandleError });
355+
es.onerror = jest.fn();
356+
357+
jest.runAllTimers();
358+
359+
// onprogress observes the error first (LOADING), then the same response
360+
// reaches DONE via onreadystatechange. retryAndHandleError must fire
361+
// exactly once for this attempt despite both handlers seeing the error.
362+
mockXhr.readyState = 3;
363+
mockXhr.status = 500;
364+
mockXhr.onprogress();
365+
366+
mockXhr.readyState = 4;
367+
mockXhr.onreadystatechange();
368+
369+
expect(retryAndHandleError).toHaveBeenCalledTimes(1);
370+
});
226371
});

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

Lines changed: 59 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@
1818
* the stream was actually running. onprogress fires as soon as the response
1919
* starts loading, so headers are available as soon as the connection opens
2020
* instead of only at teardown.
21+
* 3. onprogress now also drives retryAndHandleError/reconnect on an error status,
22+
* guarded exactly-once against onreadystatechange (see isFirstErrorForThisAttempt
23+
* below). Previously only onreadystatechange reaching DONE could trigger a retry,
24+
* so an error response that never reached DONE - the same never-fires-
25+
* readystatechange-while-streaming platforms note 1 above describes - would never
26+
* recover or have its response headers (e.g. an FDv1 fallback directive) read.
2127
*/
2228
import type { EventSourceEvent, EventSourceListener, EventSourceOptions, EventType } from './types';
2329

@@ -177,14 +183,40 @@ export default class EventSource<E extends string = never> {
177183

178184
this._handleEvent(this._xhr.responseText || '');
179185
} else {
186+
const isFirstErrorForThisAttempt = this._status !== this.ERROR;
180187
this._status = this.ERROR;
188+
const headers = this._parseResponseHeaders(this._xhr.getAllResponseHeaders());
181189

182190
this.dispatch('error', {
183191
type: 'error',
184192
message: this._xhr.responseText,
185193
xhrStatus: this._xhr.status,
186194
xhrState: this._xhr.readyState,
195+
status: this._xhr.status,
196+
headers,
187197
});
198+
199+
// A bad status observed here may never be followed by a DONE
200+
// readystatechange: a streaming error response, or a platform
201+
// that does not fire readystatechange reliably, leaves
202+
// onreadystatechange silent. Retry from onprogress too, guarded
203+
// so a later DONE for the same attempt does not trigger a second
204+
// retry.
205+
if (isFirstErrorForThisAttempt) {
206+
if (!this._retryAndHandleError) {
207+
this._tryConnect();
208+
} else {
209+
const shouldRetry = this._retryAndHandleError({
210+
status: this._xhr.status,
211+
message: this._xhr.responseText,
212+
headers,
213+
});
214+
215+
if (shouldRetry) {
216+
this._tryConnect();
217+
}
218+
}
219+
}
188220
}
189221
};
190222

@@ -226,30 +258,46 @@ export default class EventSource<E extends string = never> {
226258
this._tryConnect();
227259
}
228260
} else {
261+
// Mirrors onprogress's error-retry guard: if onprogress already
262+
// retried for this attempt's error, this DONE for the same attempt
263+
// must not trigger a second retry. This also relies on
264+
// readystatechange(DONE) firing before the native onerror handler
265+
// for the same failed request - onerror sets _status = ERROR
266+
// without retrying, so if it ran first this guard would
267+
// incorrectly suppress the retry below. XHR fires
268+
// readystatechange(DONE) before error, and RN follows that
269+
// ordering, so this holds today.
270+
const isFirstErrorForThisAttempt = this._status !== this.ERROR;
229271
this._status = this.ERROR;
272+
const headers = this._parseResponseHeaders(this._xhr.getAllResponseHeaders());
230273

231274
this.dispatch('error', {
232275
type: 'error',
233276
message: this._xhr.responseText,
234277
xhrStatus: this._xhr.status,
235278
xhrState: this._xhr.readyState,
279+
status: this._xhr.status,
280+
headers,
236281
});
237282

238283
if (this._xhr.readyState === XMLHttpRequest.DONE) {
239284
this._logger?.debug('[EventSource][onreadystatechange][ERROR] Response status error.');
240285

241-
if (!this._retryAndHandleError) {
242-
// by default just try and reconnect if there's an error.
243-
this._tryConnect();
244-
} else {
245-
// custom retry logic taking into account status codes.
246-
const shouldRetry = this._retryAndHandleError({
247-
status: this._xhr.status,
248-
message: this._xhr.responseText,
249-
});
250-
251-
if (shouldRetry) {
286+
if (isFirstErrorForThisAttempt) {
287+
if (!this._retryAndHandleError) {
288+
// by default just try and reconnect if there's an error.
252289
this._tryConnect();
290+
} else {
291+
// custom retry logic taking into account status codes.
292+
const shouldRetry = this._retryAndHandleError({
293+
status: this._xhr.status,
294+
message: this._xhr.responseText,
295+
headers,
296+
});
297+
298+
if (shouldRetry) {
299+
this._tryConnect();
300+
}
253301
}
254302
}
255303
}

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,15 @@ export interface ErrorEvent {
3131
message: string;
3232
xhrState: number;
3333
xhrStatus: number;
34+
/**
35+
* Present when the error corresponds to an HTTP response with a numeric
36+
* status, as opposed to a network-level failure with no response at all.
37+
* Consumers' `errorFilter`/`onerror` handlers rely on this to tell an
38+
* HTTP-status error apart from a network error.
39+
*/
40+
status?: number;
41+
/** Parsed response headers, present under the same condition as `status`. */
42+
headers?: Record<string, string>;
3443
}
3544

3645
export interface CustomEvent<E extends string> {

0 commit comments

Comments
 (0)