fix: error responses are not propagated in RN eventsource

Author: joker23

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

@@ -223,4 +223,149 @@ describe('EventSource', () => {
 
     expect(onopen).toHaveBeenCalledTimes(1);
   });
+
+  test('calls retryAndHandleError with parsed response headers on an error response', () => {
+    const retryAndHandleError = jest.fn(() => false);
+
+    mockXhr.getAllResponseHeaders = jest.fn(
+      () => 'X-Ld-Fd-Fallback: true\r\nX-Ld-Fd-Fallback-Ttl: 60\r\nContent-Type: text/event-stream',
+    );
+    mockXhr.responseText = 'error body';
+
+    const es = new EventSource<EventName>(uri, { logger, retryAndHandleError });
+    es.onerror = jest.fn();
+
+    jest.runAllTimers();
+
+    mockXhr.readyState = 4;
+    mockXhr.status = 500;
+    mockXhr.onreadystatechange();
+
+    expect(retryAndHandleError).toHaveBeenCalledTimes(1);
+    expect(retryAndHandleError).toHaveBeenCalledWith(
+      expect.objectContaining({
+        status: 500,
+        message: 'error body',
+        headers: expect.objectContaining({
+          'x-ld-fd-fallback': 'true',
+          'x-ld-fd-fallback-ttl': '60',
+        }),
+      }),
+    );
+  });
+
+  test('dispatches error with status and headers from onprogress mid-stream', () => {
+    const onerror = jest.fn();
+
+    mockXhr.getAllResponseHeaders = jest.fn(
+      () => 'X-Ld-Fd-Fallback: true\r\nX-Ld-Fd-Fallback-Ttl: 60\r\nContent-Type: text/event-stream',
+    );
+    mockXhr.responseText = 'error body';
+
+    eventSource.onerror = onerror;
+
+    jest.runAllTimers();
+
+    // readyState 3 (LOADING), not DONE, so this exercises onprogress's
+    // error branch rather than onreadystatechange's.
+    mockXhr.readyState = 3;
+    mockXhr.status = 500;
+    mockXhr.onprogress();
+
+    expect(onerror).toHaveBeenCalledWith(
+      expect.objectContaining({
+        status: 500,
+        headers: expect.objectContaining({
+          'x-ld-fd-fallback': 'true',
+          'x-ld-fd-fallback-ttl': '60',
+        }),
+      }),
+    );
+  });
+
+  test('dispatches error with status and headers from onreadystatechange before retrying', () => {
+    const onerror = jest.fn();
+
+    mockXhr.getAllResponseHeaders = jest.fn(
+      () => 'X-Ld-Fd-Fallback: true\r\nX-Ld-Fd-Fallback-Ttl: 60\r\nContent-Type: text/event-stream',
+    );
+    mockXhr.responseText = 'error body';
+
+    eventSource.onerror = onerror;
+
+    jest.runAllTimers();
+
+    mockXhr.readyState = 4;
+    mockXhr.status = 500;
+    mockXhr.onreadystatechange();
+
+    expect(onerror).toHaveBeenCalledWith(
+      expect.objectContaining({
+        status: 500,
+        headers: expect.objectContaining({
+          'x-ld-fd-fallback': 'true',
+          'x-ld-fd-fallback-ttl': '60',
+        }),
+      }),
+    );
+  });
+
+  test('invokes retryAndHandleError from onprogress when the connection never reaches DONE', () => {
+    const retryAndHandleError = jest.fn(() => false);
+
+    mockXhr.getAllResponseHeaders = jest.fn(
+      () => 'X-Ld-Fd-Fallback: true\r\nX-Ld-Fd-Fallback-Ttl: 60\r\nContent-Type: text/event-stream',
+    );
+    mockXhr.responseText = 'error body';
+
+    const es = new EventSource<EventName>(uri, { logger, retryAndHandleError });
+    es.onerror = jest.fn();
+
+    jest.runAllTimers();
+
+    // readyState never advances to DONE for this attempt, so
+    // onreadystatechange never fires; onprogress must drive the retry
+    // on its own.
+    mockXhr.readyState = 3;
+    mockXhr.status = 500;
+    mockXhr.onprogress();
+
+    expect(retryAndHandleError).toHaveBeenCalledTimes(1);
+    expect(retryAndHandleError).toHaveBeenCalledWith(
+      expect.objectContaining({
+        status: 500,
+        message: 'error body',
+        headers: expect.objectContaining({
+          'x-ld-fd-fallback': 'true',
+          'x-ld-fd-fallback-ttl': '60',
+        }),
+      }),
+    );
+  });
+
+  test('does not invoke retryAndHandleError twice when onprogress and onreadystatechange observe the same failed attempt', () => {
+    const retryAndHandleError = jest.fn(() => false);
+
+    mockXhr.getAllResponseHeaders = jest.fn(
+      () => 'X-Ld-Fd-Fallback: true\r\nX-Ld-Fd-Fallback-Ttl: 60\r\nContent-Type: text/event-stream',
+    );
+    mockXhr.responseText = 'error body';
+
+    const es = new EventSource<EventName>(uri, { logger, retryAndHandleError });
+    es.onerror = jest.fn();
+
+    jest.runAllTimers();
+
+    // onprogress observes the error first (LOADING), then the same response
+    // reaches DONE via onreadystatechange. retryAndHandleError must fire
+    // exactly once for this attempt despite both handlers seeing the error.
+    mockXhr.readyState = 3;
+    mockXhr.status = 500;
+    mockXhr.onprogress();
+
+    mockXhr.readyState = 4;
+    mockXhr.onreadystatechange();
+
+    expect(retryAndHandleError).toHaveBeenCalledTimes(1);
+  });
 });

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

@@ -18,6 +18,12 @@
  *    the stream was actually running. onprogress fires as soon as the response
  *    starts loading, so headers are available as soon as the connection opens
  *    instead of only at teardown.
+ * 3. onprogress now also drives retryAndHandleError/reconnect on an error status,
+ *    guarded exactly-once against onreadystatechange (see isFirstErrorForThisAttempt
+ *    below). Previously only onreadystatechange reaching DONE could trigger a retry,
+ *    so an error response that never reached DONE - the same never-fires-
+ *    readystatechange-while-streaming platforms note 1 above describes - would never
+ *    recover or have its response headers (e.g. an FDv1 fallback directive) read.
  */
 import type { EventSourceEvent, EventSourceListener, EventSourceOptions, EventType } from './types';
 
@@ -177,14 +183,40 @@ export default class EventSource<E extends string = never> {
 
           this._handleEvent(this._xhr.responseText || '');
         } else {
+          const isFirstErrorForThisAttempt = this._status !== this.ERROR;
           this._status = this.ERROR;
+          const headers = this._parseResponseHeaders(this._xhr.getAllResponseHeaders());
 
           this.dispatch('error', {
             type: 'error',
             message: this._xhr.responseText,
             xhrStatus: this._xhr.status,
             xhrState: this._xhr.readyState,
+            status: this._xhr.status,
+            headers,
           });
+
+          // A bad status observed here may never be followed by a DONE
+          // readystatechange: a streaming error response, or a platform
+          // that does not fire readystatechange reliably, leaves
+          // onreadystatechange silent. Retry from onprogress too, guarded
+          // so a later DONE for the same attempt does not trigger a second
+          // retry.
+          if (isFirstErrorForThisAttempt) {
+            if (!this._retryAndHandleError) {
+              this._tryConnect();
+            } else {
+              const shouldRetry = this._retryAndHandleError({
+                status: this._xhr.status,
+                message: this._xhr.responseText,
+                headers,
+              });
+
+              if (shouldRetry) {
+                this._tryConnect();
+              }
+            }
+          }
         }
       };
 
@@ -226,30 +258,46 @@ export default class EventSource<E extends string = never> {
             this._tryConnect();
           }
         } else {
+          // Mirrors onprogress's error-retry guard: if onprogress already
+          // retried for this attempt's error, this DONE for the same attempt
+          // must not trigger a second retry. This also relies on
+          // readystatechange(DONE) firing before the native onerror handler
+          // for the same failed request - onerror sets _status = ERROR
+          // without retrying, so if it ran first this guard would
+          // incorrectly suppress the retry below. XHR fires
+          // readystatechange(DONE) before error, and RN follows that
+          // ordering, so this holds today.
+          const isFirstErrorForThisAttempt = this._status !== this.ERROR;
           this._status = this.ERROR;
+          const headers = this._parseResponseHeaders(this._xhr.getAllResponseHeaders());
 
           this.dispatch('error', {
             type: 'error',
             message: this._xhr.responseText,
             xhrStatus: this._xhr.status,
             xhrState: this._xhr.readyState,
+            status: this._xhr.status,
+            headers,
           });
 
           if (this._xhr.readyState === XMLHttpRequest.DONE) {
             this._logger?.debug('[EventSource][onreadystatechange][ERROR] Response status error.');
 
-            if (!this._retryAndHandleError) {
-              // by default just try and reconnect if there's an error.
-              this._tryConnect();
-            } else {
-              // custom retry logic taking into account status codes.
-              const shouldRetry = this._retryAndHandleError({
-                status: this._xhr.status,
-                message: this._xhr.responseText,
-              });
-
-              if (shouldRetry) {
+            if (isFirstErrorForThisAttempt) {
+              if (!this._retryAndHandleError) {
+                // by default just try and reconnect if there's an error.
                 this._tryConnect();
+              } else {
+                // custom retry logic taking into account status codes.
+                const shouldRetry = this._retryAndHandleError({
+                  status: this._xhr.status,
+                  message: this._xhr.responseText,
+                  headers,
+                });
+
+                if (shouldRetry) {
+                  this._tryConnect();
+                }
               }
             }
           }

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

@@ -31,6 +31,15 @@ export interface ErrorEvent {
   message: string;
   xhrState: number;
   xhrStatus: number;
+  /**
+   * Present when the error corresponds to an HTTP response with a numeric
+   * status, as opposed to a network-level failure with no response at all.
+   * Consumers' `errorFilter`/`onerror` handlers rely on this to tell an
+   * HTTP-status error apart from a network error.
+   */
+  status?: number;
+  /** Parsed response headers, present under the same condition as `status`. */
+  headers?: Record<string, string>;
 }
 
 export interface CustomEvent<E extends string> {

packages/shared/sdk-client/__tests__/datasource/fdv2/StreamingFDv2Base.test.ts

@@ -635,6 +635,42 @@ it('clears a pending fallback directive when onopen fires without the fallback h
   base.close();
 });
 
+it('clears a committed fallback after a clean reconnect on the same source', async () => {
+  // Once a changeSet has been stamped with fdv1Fallback=true, that state is
+  // committed (not just pending). A later onopen on the SAME source with no
+  // fallback header must clear the committed flag too, not only the pending
+  // one, so a stray reconnect racing ahead of the orchestrator's shutdown
+  // doesn't keep stamping fresh results as fallback.
+  const mockEventSource = createMockEventSource();
+  const mockRequests = createMockRequests(mockEventSource);
+  const base = createBase(mockRequests, logger);
+  base.start();
+
+  // First open carries the fallback header; applied to the next changeSet.
+  mockEventSource.onopen({
+    type: 'open',
+    headers: { 'x-ld-fd-fallback': 'true', 'x-ld-fd-fallback-ttl': '60' },
+  });
+  sendFullTransfer(mockEventSource, [{ key: 'flag-a', version: 1, value: 'blue' }]);
+
+  const fallbackResult = await base.takeResult();
+  expect(fallbackResult.type).toBe('changeSet');
+  if (fallbackResult.type !== 'changeSet') return;
+  expect(fallbackResult.fdv1Fallback).toBe(true);
+
+  // Reconnect cleanly on the same source, with no fallback header this time.
+  mockEventSource.onopen({ type: 'open', headers: {} });
+  sendFullTransfer(mockEventSource, [{ key: 'flag-b', version: 1, value: 'green' }]);
+
+  const cleanResult = await base.takeResult();
+  expect(cleanResult.type).toBe('changeSet');
+  if (cleanResult.type !== 'changeSet') return;
+  expect(cleanResult.fdv1Fallback).toBe(false);
+  expect(cleanResult.fdv1FallbackTtlMs).toBeUndefined();
+
+  base.close();
+});
+
 it('close resets the pending fallback directive', async () => {
   // Calling close() must clear pendingFallback/pendingFallbackTtlMs so a
   // subsequently re-created source does not inherit stale state.

packages/shared/sdk-client/src/datasource/fdv2/StreamingFDv2Base.ts

@@ -397,8 +397,16 @@ export function createStreamingBase(config: {
         config.logger?.info('Opened LaunchDarkly stream connection');
         protocolHandler.reset();
 
-        // Reset pending directive on every connection open so a reconnect that
-        // does not carry the fallback header clears any state from a prior attempt.
+        // Reset both the committed and pending directive on every connection
+        // open so a reconnect that does not carry the fallback header clears
+        // any state from a prior attempt. Resetting the committed flag here
+        // does not undo an already-emitted fallback: the result queue is
+        // FIFO, so a `fdv1Fallback: true` result queued before this reconnect
+        // is always drained before anything this connection queues after it,
+        // and the orchestrator closes this instance as soon as it sees that
+        // result - any later result from this reconnect is abandoned unread.
+        fdv1Fallback = false;
+        fdv1FallbackTtlMs = undefined;
         pendingFallback = false;
         pendingFallbackTtlMs = undefined;