Skip to content

Commit 4203401

Browse files
lucas-zimermankrystofwoldrich
lucas-zimerman
and
krystofwoldrich
authored
Kw fallback emitter suggestion (#4171)
* Emitter suggestion * fix tests --------- Co-authored-by: Krystof Woldrich <[email protected]>
1 parent a99c3c1 commit 4203401

File tree

3 files changed

+100
-103
lines changed

3 files changed

+100
-103
lines changed

‎src/js/utils/sentryeventemitter.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ export function createSentryEventEmitter(
8585
addListener,
8686
removeListener,
8787
once(eventType: NewFrameEventName, listener: (event: NewFrameEvent) => void) {
88-
fallbackEventEmitter?.startListenerAsync();
88+
fallbackEventEmitter?.onceNewFrame();
8989

9090
const tmpListener = (event: NewFrameEvent): void => {
9191
listener(event);

‎src/js/utils/sentryeventemitterfallback.ts‎

Lines changed: 59 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { logger, timestampInSeconds } from '@sentry/utils';
2-
import { DeviceEventEmitter } from 'react-native';
32

43
import { NATIVE } from '../wrapper';
5-
import { NewFrameEventName } from './sentryeventemitter';
4+
import type { NewFrameEvent, SentryEventEmitter } from './sentryeventemitter';
5+
import { createSentryEventEmitter, NewFrameEventName } from './sentryeventemitter';
6+
7+
export const FALLBACK_TIMEOUT_MS = 10_000;
68

79
export type FallBackNewFrameEvent = { newFrameTimestampInSeconds: number; isFallback?: boolean };
810
export interface SentryEventEmitterFallback {
@@ -11,83 +13,84 @@ export interface SentryEventEmitterFallback {
1113
* This method is synchronous in JS but the event emitter starts asynchronously.
1214
*/
1315
initAsync: () => void;
14-
startListenerAsync: () => void;
16+
onceNewFrame: (listener: (event: FallBackNewFrameEvent) => void) => void;
1517
}
1618

1719
/**
1820
* Creates emitter that allows to listen to UI Frame events when ready.
1921
*/
20-
export function createSentryFallbackEventEmitter(): SentryEventEmitterFallback {
21-
let NativeEmitterCalled: boolean = false;
22-
let isListening = false;
22+
export function createSentryFallbackEventEmitter(
23+
emitter: SentryEventEmitter = createSentryEventEmitter(),
24+
fallbackTimeoutMs = FALLBACK_TIMEOUT_MS,
25+
): SentryEventEmitterFallback {
26+
let fallbackTimeout: ReturnType<typeof setTimeout> | undefined;
27+
let animationFrameTimestampSeconds: number | undefined;
28+
let nativeNewFrameTimestampSeconds: number | undefined;
2329

24-
function defaultFallbackEventEmitter(): void {
30+
function getAnimationFrameTimestampSeconds(): void {
2531
// https://reactnative.dev/docs/timers#timers
2632
// NOTE: The current implementation of requestAnimationFrame is the same
2733
// as setTimeout(0). This isn't exactly how requestAnimationFrame
2834
// 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
2935
requestAnimationFrame(() => {
30-
if (NativeEmitterCalled) {
31-
NativeEmitterCalled = false;
32-
isListening = false;
36+
if (fallbackTimeout === undefined) {
3337
return;
3438
}
35-
const seconds = timestampInSeconds();
36-
waitForNativeResponseOrFallback(seconds, 'JavaScript');
39+
animationFrameTimestampSeconds = timestampInSeconds();
3740
});
3841
}
3942

40-
function waitForNativeResponseOrFallback(fallbackSeconds: number, origin: string): void {
41-
let firstAttemptCompleted = false;
42-
43-
const checkNativeResponse = (): void => {
44-
if (NativeEmitterCalled) {
45-
NativeEmitterCalled = false;
46-
isListening = false;
47-
return; // Native Replied the bridge with a timestamp.
48-
}
49-
if (!firstAttemptCompleted) {
50-
firstAttemptCompleted = true;
51-
setTimeout(checkNativeResponse, 3_000);
52-
} else {
53-
logger.log(`[Sentry] Native event emitter did not reply in time. Using ${origin} fallback emitter.`);
54-
isListening = false;
55-
DeviceEventEmitter.emit(NewFrameEventName, {
56-
newFrameTimestampInSeconds: fallbackSeconds,
57-
isFallback: true,
58-
});
59-
}
60-
};
61-
62-
// Start the retry process
63-
checkNativeResponse();
43+
function getNativeNewFrameTimestampSeconds(): void {
44+
NATIVE.getNewScreenTimeToDisplay()
45+
.then(resolve => {
46+
if (fallbackTimeout === undefined) {
47+
return;
48+
}
49+
nativeNewFrameTimestampSeconds = resolve ?? undefined;
50+
})
51+
.catch(reason => {
52+
logger.error('Failed to receive Native fallback timestamp.', reason);
53+
});
6454
}
6555

6656
return {
6757
initAsync() {
68-
DeviceEventEmitter.addListener(NewFrameEventName, () => {
69-
// Avoid noise from pages that we do not want to track.
70-
if (isListening) {
71-
NativeEmitterCalled = true;
72-
}
73-
});
58+
emitter.initAsync(NewFrameEventName);
7459
},
7560

76-
startListenerAsync() {
77-
isListening = true;
61+
onceNewFrame(listener: (event: FallBackNewFrameEvent) => void) {
62+
animationFrameTimestampSeconds = undefined;
63+
nativeNewFrameTimestampSeconds = undefined;
64+
65+
const internalListener = (event: NewFrameEvent): void => {
66+
clearTimeout(fallbackTimeout);
67+
fallbackTimeout = undefined;
68+
animationFrameTimestampSeconds = undefined;
69+
nativeNewFrameTimestampSeconds = undefined;
70+
listener(event);
71+
};
72+
fallbackTimeout = setTimeout(() => {
73+
if (nativeNewFrameTimestampSeconds) {
74+
logger.log('Native event emitter did not reply in time');
75+
return listener({
76+
newFrameTimestampInSeconds: nativeNewFrameTimestampSeconds,
77+
isFallback: true,
78+
});
79+
} else if (animationFrameTimestampSeconds) {
80+
logger.log('[Sentry] Native event emitter did not reply in time. Using JavaScript fallback emitter.');
81+
return listener({
82+
newFrameTimestampInSeconds: animationFrameTimestampSeconds,
83+
isFallback: true,
84+
});
85+
} else {
86+
emitter.removeListener(NewFrameEventName, internalListener);
87+
logger.error('Failed to receive any fallback timestamp.');
88+
}
89+
}, fallbackTimeoutMs);
7890

79-
NATIVE.getNewScreenTimeToDisplay()
80-
.then(resolve => {
81-
if (resolve) {
82-
waitForNativeResponseOrFallback(resolve, 'Native');
83-
} else {
84-
defaultFallbackEventEmitter();
85-
}
86-
})
87-
.catch((reason: Error) => {
88-
logger.error('Failed to recceive Native fallback timestamp.', reason);
89-
defaultFallbackEventEmitter();
90-
});
91+
getNativeNewFrameTimestampSeconds();
92+
getAnimationFrameTimestampSeconds();
93+
emitter.once(NewFrameEventName, internalListener);
9194
},
9295
};
9396
}

‎test/utils/sentryeventemitterfallback.test.ts‎

Lines changed: 40 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,7 @@
1-
import { DeviceEventEmitter } from 'react-native';
2-
31
import { NewFrameEventName } from '../../src/js/utils/sentryeventemitter';
42
import { createSentryFallbackEventEmitter } from '../../src/js/utils/sentryeventemitterfallback';
53

64
// Mock dependencies
7-
8-
jest.mock('react-native', () => {
9-
return {
10-
DeviceEventEmitter: {
11-
addListener: jest.fn(),
12-
emit: jest.fn(),
13-
},
14-
Platform: {
15-
OS: 'ios',
16-
},
17-
};
18-
});
19-
205
jest.mock('../../src/js/utils/environment', () => ({
216
isTurboModuleEnabled: () => false,
227
}));
@@ -48,12 +33,6 @@ describe('SentryEventEmitterFallback', () => {
4833
NATIVE.getNewScreenTimeToDisplay = jest.fn();
4934
});
5035

51-
it('should initialize and add a listener', () => {
52-
emitter.initAsync();
53-
54-
expect(DeviceEventEmitter.addListener).toHaveBeenCalledWith(NewFrameEventName, expect.any(Function));
55-
});
56-
5736
it('should start listener and use fallback when native call returned undefined/null', async () => {
5837
jest.useFakeTimers();
5938
const spy = jest.spyOn(require('@sentry/utils'), 'timestampInSeconds');
@@ -62,19 +41,20 @@ describe('SentryEventEmitterFallback', () => {
6241

6342
(NATIVE.getNewScreenTimeToDisplay as jest.Mock).mockReturnValue(Promise.resolve());
6443

65-
emitter.startListenerAsync();
44+
const listener = jest.fn();
45+
emitter.onceNewFrame(listener);
6646

6747
// Wait for the next event loop to allow startListenerAsync to call NATIVE.getNewScreenTimeToDisplay
6848
await Promise.resolve();
6949

7050
await expect(NATIVE.getNewScreenTimeToDisplay).toHaveBeenCalled();
71-
expect(logger.error).not.toHaveBeenCalledWith('Failed to recceive Native fallback timestamp.', expect.any(Error));
51+
expect(logger.error).not.toHaveBeenCalledWith('Failed to receive Native fallback timestamp.', expect.any(Error));
7252

7353
// Simulate retries and timer
7454
jest.runAllTimers();
7555

7656
// Ensure fallback event is emitted
77-
expect(DeviceEventEmitter.emit).toHaveBeenCalledWith(NewFrameEventName, {
57+
expect(listener).toHaveBeenCalledWith({
7858
newFrameTimestampInSeconds: fallbackTime,
7959
isFallback: true,
8060
});
@@ -90,23 +70,24 @@ describe('SentryEventEmitterFallback', () => {
9070

9171
(NATIVE.getNewScreenTimeToDisplay as jest.Mock).mockRejectedValue(new Error('Failed'));
9272

93-
emitter.startListenerAsync();
94-
9573
const spy = jest.spyOn(require('@sentry/utils'), 'timestampInSeconds');
9674
const fallbackTime = Date.now() / 1000;
9775
spy.mockReturnValue(fallbackTime);
9876

77+
const listener = jest.fn();
78+
emitter.onceNewFrame(listener);
79+
9980
// Wait for the next event loop to allow startListenerAsync to call NATIVE.getNewScreenTimeToDisplay
10081
await Promise.resolve();
10182

10283
await expect(NATIVE.getNewScreenTimeToDisplay).toHaveBeenCalled();
103-
expect(logger.error).toHaveBeenCalledWith('Failed to recceive Native fallback timestamp.', expect.any(Error));
84+
expect(logger.error).toHaveBeenCalledWith('Failed to receive Native fallback timestamp.', expect.any(Error));
10485

10586
// Simulate retries and timer
10687
jest.runAllTimers();
10788

10889
// Ensure fallback event is emitted
109-
expect(DeviceEventEmitter.emit).toHaveBeenCalledWith(NewFrameEventName, {
90+
expect(listener).toHaveBeenCalledWith({
11091
newFrameTimestampInSeconds: fallbackTime,
11192
isFallback: true,
11293
});
@@ -125,19 +106,20 @@ describe('SentryEventEmitterFallback', () => {
125106

126107
(NATIVE.getNewScreenTimeToDisplay as jest.Mock).mockRejectedValue(new Error('Failed'));
127108

128-
emitter.startListenerAsync();
109+
const listener = jest.fn();
110+
emitter.onceNewFrame(listener);
129111

130112
// Wait for the next event loop to allow startListenerAsync to call NATIVE.getNewScreenTimeToDisplay
131113
await Promise.resolve();
132114

133115
await expect(NATIVE.getNewScreenTimeToDisplay).toHaveBeenCalled();
134-
expect(logger.error).toHaveBeenCalledWith('Failed to recceive Native fallback timestamp.', expect.any(Error));
116+
expect(logger.error).toHaveBeenCalledWith('Failed to receive Native fallback timestamp.', expect.any(Error));
135117

136118
// Simulate retries and timer
137119
jest.runAllTimers();
138120

139121
// Ensure fallback event is emitted
140-
expect(DeviceEventEmitter.emit).toHaveBeenCalledWith(NewFrameEventName, {
122+
expect(listener).toHaveBeenCalledWith({
141123
newFrameTimestampInSeconds: fallbackTime,
142124
isFallback: true,
143125
});
@@ -156,7 +138,8 @@ describe('SentryEventEmitterFallback', () => {
156138

157139
NATIVE.getNewScreenTimeToDisplay = () => Promise.resolve(null);
158140

159-
emitter.startListenerAsync();
141+
const listener = jest.fn();
142+
emitter.onceNewFrame(listener);
160143

161144
// Wait for the next event loop to allow startListenerAsync to call NATIVE.getNewScreenTimeToDisplay
162145
await Promise.resolve();
@@ -165,7 +148,7 @@ describe('SentryEventEmitterFallback', () => {
165148
jest.runAllTimers();
166149

167150
// Ensure fallback event is emitted
168-
expect(DeviceEventEmitter.emit).toHaveBeenCalledWith(NewFrameEventName, {
151+
expect(listener).toHaveBeenCalledWith({
169152
newFrameTimestampInSeconds: fallbackTime,
170153
isFallback: true,
171154
});
@@ -181,21 +164,19 @@ describe('SentryEventEmitterFallback', () => {
181164

182165
(NATIVE.getNewScreenTimeToDisplay as jest.Mock).mockResolvedValueOnce(nativeTimestamp);
183166

184-
emitter.startListenerAsync();
167+
const listener = jest.fn();
168+
emitter.onceNewFrame(listener);
185169

186170
expect(NATIVE.getNewScreenTimeToDisplay).toHaveBeenCalled();
187171
});
188172

189173
it('should not emit if original event emitter was called', async () => {
190174
jest.useFakeTimers();
191175

192-
const mockAddListener = jest.fn();
193-
DeviceEventEmitter.addListener = mockAddListener;
194-
195176
// Capture the callback passed to addListener
196177
// eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/ban-types
197178
let callback: Function = () => {};
198-
mockAddListener.mockImplementationOnce((eventName, cb) => {
179+
const mockOnce = jest.fn().mockImplementationOnce((eventName, cb) => {
199180
if (eventName === NewFrameEventName) {
200181
callback = cb;
201182
}
@@ -204,24 +185,40 @@ describe('SentryEventEmitterFallback', () => {
204185
};
205186
});
206187

188+
emitter = createSentryFallbackEventEmitter({
189+
addListener: jest.fn(),
190+
initAsync: jest.fn(),
191+
closeAllAsync: jest.fn(),
192+
removeListener: jest.fn(),
193+
once: mockOnce,
194+
});
195+
207196
emitter.initAsync();
208-
emitter.startListenerAsync();
209-
callback();
197+
const listener = jest.fn();
198+
emitter.onceNewFrame(listener);
199+
callback({
200+
newFrameTimestampInSeconds: 67890,
201+
});
210202

211203
// Wait for the next event loop to allow startListenerAsync to call NATIVE.getNewScreenTimeToDisplay
212204
await Promise.resolve();
213205

214206
// Simulate retries and timer
215207
jest.runAllTimers();
216208

217-
expect(DeviceEventEmitter.emit).not.toBeCalled();
209+
// Ensure fallback event is emitted
210+
expect(listener).toHaveBeenCalledWith({
211+
newFrameTimestampInSeconds: 67890,
212+
isFallback: undefined,
213+
});
218214
expect(logger.log).not.toBeCalled();
219215
});
220216

221217
it('should retry up to maxRetries and emit fallback if no response', async () => {
222218
jest.useFakeTimers();
223219

224-
emitter.startListenerAsync();
220+
const listener = jest.fn();
221+
emitter.onceNewFrame(listener);
225222

226223
// Wait for the next event loop to allow startListenerAsync to call NATIVE.getNewScreenTimeToDisplay
227224
await Promise.resolve();
@@ -231,10 +228,7 @@ describe('SentryEventEmitterFallback', () => {
231228
// Simulate retries and timer
232229
jest.runAllTimers();
233230

234-
expect(DeviceEventEmitter.emit).toHaveBeenCalledWith(
235-
NewFrameEventName,
236-
expect.objectContaining({ isFallback: true }),
237-
);
231+
expect(listener).toHaveBeenCalledWith(expect.objectContaining({ isFallback: true }));
238232
expect(logger.log).toHaveBeenCalledWith(expect.stringContaining('Native event emitter did not reply in time'));
239233

240234
jest.useRealTimers();

0 commit comments

Comments
 (0)