Kw fallback emitter suggestion

src/js/utils/sentryeventemitter.ts

@@ -85,7 +85,7 @@ export function createSentryEventEmitter(
     addListener,
     removeListener,
     once(eventType: NewFrameEventName, listener: (event: NewFrameEvent) => void) {
-      fallbackEventEmitter?.startListenerAsync();
+      fallbackEventEmitter?.onceNewFrame();
 
       const tmpListener = (event: NewFrameEvent): void => {
         listener(event);

src/js/utils/sentryeventemitterfallback.ts

@@ -1,8 +1,10 @@
 import { logger, timestampInSeconds } from '@sentry/utils';
-import { DeviceEventEmitter } from 'react-native';
 
 import { NATIVE } from '../wrapper';
-import { NewFrameEventName } from './sentryeventemitter';
+import type { NewFrameEvent, SentryEventEmitter } from './sentryeventemitter';
+import { createSentryEventEmitter, NewFrameEventName } from './sentryeventemitter';
+
+export const FALLBACK_TIMEOUT_MS = 10_000;
 
 export type FallBackNewFrameEvent = { newFrameTimestampInSeconds: number; isFallback?: boolean };
 export interface SentryEventEmitterFallback {
@@ -11,83 +13,84 @@ export interface SentryEventEmitterFallback {
    * This method is synchronous in JS but the event emitter starts asynchronously.
    */
   initAsync: () => void;
-  startListenerAsync: () => void;
+  onceNewFrame: (listener: (event: FallBackNewFrameEvent) => void) => void;
 }
 
 /**
  * Creates emitter that allows to listen to UI Frame events when ready.
  */
-export function createSentryFallbackEventEmitter(): SentryEventEmitterFallback {
-  let NativeEmitterCalled: boolean = false;
-  let isListening = false;
+export function createSentryFallbackEventEmitter(
+  emitter: SentryEventEmitter = createSentryEventEmitter(),
+  fallbackTimeoutMs = FALLBACK_TIMEOUT_MS,
+): SentryEventEmitterFallback {
+  let fallbackTimeout: ReturnType<typeof setTimeout> | undefined;
+  let animationFrameTimestampSeconds: number | undefined;
+  let nativeNewFrameTimestampSeconds: number | undefined;
 
-  function defaultFallbackEventEmitter(): void {
+  function getAnimationFrameTimestampSeconds(): void {
     // https://reactnative.dev/docs/timers#timers
     // NOTE: The current implementation of requestAnimationFrame is the same
     // as setTimeout(0). This isn't exactly how requestAnimationFrame
     // is supposed to work on web, so it doesn't get called when UI Frames are rendered.: https://github.com/facebook/react-native/blob/5106933c750fee2ce49fe1945c3e3763eebc92bc/packages/react-native/ReactCommon/react/runtime/TimerManager.cpp#L442-L443
     requestAnimationFrame(() => {
-      if (NativeEmitterCalled) {
-        NativeEmitterCalled = false;
-        isListening = false;
+      if (fallbackTimeout === undefined) {
         return;
       }
-      const seconds = timestampInSeconds();
-      waitForNativeResponseOrFallback(seconds, 'JavaScript');
+      animationFrameTimestampSeconds = timestampInSeconds();
     });
   }
 
-  function waitForNativeResponseOrFallback(fallbackSeconds: number, origin: string): void {
-    let firstAttemptCompleted = false;
-
-    const checkNativeResponse = (): void => {
-      if (NativeEmitterCalled) {
-        NativeEmitterCalled = false;
-        isListening = false;
-        return; // Native Replied the bridge with a timestamp.
-      }
-      if (!firstAttemptCompleted) {
-        firstAttemptCompleted = true;
-        setTimeout(checkNativeResponse, 3_000);
-      } else {
-        logger.log(`[Sentry] Native event emitter did not reply in time. Using ${origin} fallback emitter.`);
-        isListening = false;
-        DeviceEventEmitter.emit(NewFrameEventName, {
-          newFrameTimestampInSeconds: fallbackSeconds,
-          isFallback: true,
-        });
-      }
-    };
-
-    // Start the retry process
-    checkNativeResponse();
+  function getNativeNewFrameTimestampSeconds(): void {
+    NATIVE.getNewScreenTimeToDisplay()
+      .then(resolve => {
+        if (fallbackTimeout === undefined) {
+          return;
+        }
+        nativeNewFrameTimestampSeconds = resolve ?? undefined;
+      })
+      .catch(reason => {
+        logger.error('Failed to receive Native fallback timestamp.', reason);
+      });
   }
 
   return {
     initAsync() {
-      DeviceEventEmitter.addListener(NewFrameEventName, () => {
-        // Avoid noise from pages that we do not want to track.
-        if (isListening) {
-          NativeEmitterCalled = true;
-        }
-      });
+      emitter.initAsync(NewFrameEventName);
     },
 
-    startListenerAsync() {
-      isListening = true;
+    onceNewFrame(listener: (event: FallBackNewFrameEvent) => void) {
+      animationFrameTimestampSeconds = undefined;
+      nativeNewFrameTimestampSeconds = undefined;
+
+      const internalListener = (event: NewFrameEvent): void => {
+        clearTimeout(fallbackTimeout);
+        fallbackTimeout = undefined;
+        animationFrameTimestampSeconds = undefined;
+        nativeNewFrameTimestampSeconds = undefined;
+        listener(event);
+      };
+      fallbackTimeout = setTimeout(() => {
+        if (nativeNewFrameTimestampSeconds) {
+          logger.log('Native event emitter did not reply in time');
+          return listener({
+            newFrameTimestampInSeconds: nativeNewFrameTimestampSeconds,
+            isFallback: true,
+          });
+        } else if (animationFrameTimestampSeconds) {
+          logger.log('[Sentry] Native event emitter did not reply in time. Using JavaScript fallback emitter.');
+          return listener({
+            newFrameTimestampInSeconds: animationFrameTimestampSeconds,
+            isFallback: true,
+          });
+        } else {
+          emitter.removeListener(NewFrameEventName, internalListener);
+          logger.error('Failed to receive any fallback timestamp.');
+        }
+      }, fallbackTimeoutMs);
 
-      NATIVE.getNewScreenTimeToDisplay()
-        .then(resolve => {
-          if (resolve) {
-            waitForNativeResponseOrFallback(resolve, 'Native');
-          } else {
-            defaultFallbackEventEmitter();
-          }
-        })
-        .catch((reason: Error) => {
-          logger.error('Failed to recceive Native fallback timestamp.', reason);
-          defaultFallbackEventEmitter();
-        });
+      getNativeNewFrameTimestampSeconds();
+      getAnimationFrameTimestampSeconds();
+      emitter.once(NewFrameEventName, internalListener);
     },
   };
 }

test/utils/sentryeventemitterfallback.test.ts

@@ -1,22 +1,7 @@
-import { DeviceEventEmitter } from 'react-native';
-
 import { NewFrameEventName } from '../../src/js/utils/sentryeventemitter';
 import { createSentryFallbackEventEmitter } from '../../src/js/utils/sentryeventemitterfallback';
 
 // Mock dependencies
-
-jest.mock('react-native', () => {
-  return {
-    DeviceEventEmitter: {
-      addListener: jest.fn(),
-      emit: jest.fn(),
-    },
-    Platform: {
-      OS: 'ios',
-    },
-  };
-});
-
 jest.mock('../../src/js/utils/environment', () => ({
   isTurboModuleEnabled: () => false,
 }));
@@ -48,12 +33,6 @@ describe('SentryEventEmitterFallback', () => {
     NATIVE.getNewScreenTimeToDisplay = jest.fn();
   });
 
-  it('should initialize and add a listener', () => {
-    emitter.initAsync();
-
-    expect(DeviceEventEmitter.addListener).toHaveBeenCalledWith(NewFrameEventName, expect.any(Function));
-  });
-
   it('should start listener and use fallback when native call returned undefined/null', async () => {
     jest.useFakeTimers();
     const spy = jest.spyOn(require('@sentry/utils'), 'timestampInSeconds');
@@ -62,19 +41,20 @@ describe('SentryEventEmitterFallback', () => {
 
     (NATIVE.getNewScreenTimeToDisplay as jest.Mock).mockReturnValue(Promise.resolve());
 
-    emitter.startListenerAsync();
+    const listener = jest.fn();
+    emitter.onceNewFrame(listener);
 
     // Wait for the next event loop to allow startListenerAsync to call NATIVE.getNewScreenTimeToDisplay
     await Promise.resolve();
 
     await expect(NATIVE.getNewScreenTimeToDisplay).toHaveBeenCalled();
-    expect(logger.error).not.toHaveBeenCalledWith('Failed to recceive Native fallback timestamp.', expect.any(Error));
+    expect(logger.error).not.toHaveBeenCalledWith('Failed to receive Native fallback timestamp.', expect.any(Error));
 
     // Simulate retries and timer
     jest.runAllTimers();
 
     // Ensure fallback event is emitted
-    expect(DeviceEventEmitter.emit).toHaveBeenCalledWith(NewFrameEventName, {
+    expect(listener).toHaveBeenCalledWith({
       newFrameTimestampInSeconds: fallbackTime,
       isFallback: true,
     });
@@ -90,23 +70,24 @@ describe('SentryEventEmitterFallback', () => {
 
     (NATIVE.getNewScreenTimeToDisplay as jest.Mock).mockRejectedValue(new Error('Failed'));
 
-    emitter.startListenerAsync();
-
     const spy = jest.spyOn(require('@sentry/utils'), 'timestampInSeconds');
     const fallbackTime = Date.now() / 1000;
     spy.mockReturnValue(fallbackTime);
 
+    const listener = jest.fn();
+    emitter.onceNewFrame(listener);
+
     // Wait for the next event loop to allow startListenerAsync to call NATIVE.getNewScreenTimeToDisplay
     await Promise.resolve();
 
     await expect(NATIVE.getNewScreenTimeToDisplay).toHaveBeenCalled();
-    expect(logger.error).toHaveBeenCalledWith('Failed to recceive Native fallback timestamp.', expect.any(Error));
+    expect(logger.error).toHaveBeenCalledWith('Failed to receive Native fallback timestamp.', expect.any(Error));
 
     // Simulate retries and timer
     jest.runAllTimers();
 
     // Ensure fallback event is emitted
-    expect(DeviceEventEmitter.emit).toHaveBeenCalledWith(NewFrameEventName, {
+    expect(listener).toHaveBeenCalledWith({
       newFrameTimestampInSeconds: fallbackTime,
       isFallback: true,
     });
@@ -125,19 +106,20 @@ describe('SentryEventEmitterFallback', () => {
 
     (NATIVE.getNewScreenTimeToDisplay as jest.Mock).mockRejectedValue(new Error('Failed'));
 
-    emitter.startListenerAsync();
+    const listener = jest.fn();
+    emitter.onceNewFrame(listener);
 
     // Wait for the next event loop to allow startListenerAsync to call NATIVE.getNewScreenTimeToDisplay
     await Promise.resolve();
 
     await expect(NATIVE.getNewScreenTimeToDisplay).toHaveBeenCalled();
-    expect(logger.error).toHaveBeenCalledWith('Failed to recceive Native fallback timestamp.', expect.any(Error));
+    expect(logger.error).toHaveBeenCalledWith('Failed to receive Native fallback timestamp.', expect.any(Error));
 
     // Simulate retries and timer
     jest.runAllTimers();
 
     // Ensure fallback event is emitted
-    expect(DeviceEventEmitter.emit).toHaveBeenCalledWith(NewFrameEventName, {
+    expect(listener).toHaveBeenCalledWith({
       newFrameTimestampInSeconds: fallbackTime,
       isFallback: true,
     });
@@ -156,7 +138,8 @@ describe('SentryEventEmitterFallback', () => {
 
     NATIVE.getNewScreenTimeToDisplay = () => Promise.resolve(null);
 
-    emitter.startListenerAsync();
+    const listener = jest.fn();
+    emitter.onceNewFrame(listener);
 
     // Wait for the next event loop to allow startListenerAsync to call NATIVE.getNewScreenTimeToDisplay
     await Promise.resolve();
@@ -165,7 +148,7 @@ describe('SentryEventEmitterFallback', () => {
     jest.runAllTimers();
 
     // Ensure fallback event is emitted
-    expect(DeviceEventEmitter.emit).toHaveBeenCalledWith(NewFrameEventName, {
+    expect(listener).toHaveBeenCalledWith({
       newFrameTimestampInSeconds: fallbackTime,
       isFallback: true,
     });
@@ -181,21 +164,19 @@ describe('SentryEventEmitterFallback', () => {
 
     (NATIVE.getNewScreenTimeToDisplay as jest.Mock).mockResolvedValueOnce(nativeTimestamp);
 
-    emitter.startListenerAsync();
+    const listener = jest.fn();
+    emitter.onceNewFrame(listener);
 
     expect(NATIVE.getNewScreenTimeToDisplay).toHaveBeenCalled();
   });
 
   it('should not emit if original event emitter was called', async () => {
     jest.useFakeTimers();
 
-    const mockAddListener = jest.fn();
-    DeviceEventEmitter.addListener = mockAddListener;
-
     // Capture the callback passed to addListener
     // eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/ban-types
     let callback: Function = () => {};
-    mockAddListener.mockImplementationOnce((eventName, cb) => {
+    const mockOnce = jest.fn().mockImplementationOnce((eventName, cb) => {
       if (eventName === NewFrameEventName) {
         callback = cb;
       }
@@ -204,24 +185,40 @@ describe('SentryEventEmitterFallback', () => {
       };
     });
 
+    emitter = createSentryFallbackEventEmitter({
+      addListener: jest.fn(),
+      initAsync: jest.fn(),
+      closeAllAsync: jest.fn(),
+      removeListener: jest.fn(),
+      once: mockOnce,
+    });
+
     emitter.initAsync();
-    emitter.startListenerAsync();
-    callback();
+    const listener = jest.fn();
+    emitter.onceNewFrame(listener);
+    callback({
+      newFrameTimestampInSeconds: 67890,
+    });
 
     // Wait for the next event loop to allow startListenerAsync to call NATIVE.getNewScreenTimeToDisplay
     await Promise.resolve();
 
     // Simulate retries and timer
     jest.runAllTimers();
 
-    expect(DeviceEventEmitter.emit).not.toBeCalled();
+    // Ensure fallback event is emitted
+    expect(listener).toHaveBeenCalledWith({
+      newFrameTimestampInSeconds: 67890,
+      isFallback: undefined,
+    });
     expect(logger.log).not.toBeCalled();
   });
 
   it('should retry up to maxRetries and emit fallback if no response', async () => {
     jest.useFakeTimers();
 
-    emitter.startListenerAsync();
+    const listener = jest.fn();
+    emitter.onceNewFrame(listener);
 
     // Wait for the next event loop to allow startListenerAsync to call NATIVE.getNewScreenTimeToDisplay
     await Promise.resolve();
@@ -231,10 +228,7 @@ describe('SentryEventEmitterFallback', () => {
     // Simulate retries and timer
     jest.runAllTimers();
 
-    expect(DeviceEventEmitter.emit).toHaveBeenCalledWith(
-      NewFrameEventName,
-      expect.objectContaining({ isFallback: true }),
-    );
+    expect(listener).toHaveBeenCalledWith(expect.objectContaining({ isFallback: true }));
     expect(logger.log).toHaveBeenCalledWith(expect.stringContaining('Native event emitter did not reply in time'));
 
     jest.useRealTimers();