Skip to content

Commit de4c63d

Browse files
committed
fixes
1 parent f43a4f5 commit de4c63d

5 files changed

Lines changed: 172 additions & 39 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
- Add `disableAutoUpload` option to Expo plugin to disable source map and debug symbol uploads ([#6195](https://github.com/getsentry/sentry-react-native/pull/6195))
1414
- Expose `pauseAppHangTracking` and `resumeAppHangTracking` APIs on iOS ([#6192](https://github.com/getsentry/sentry-react-native/pull/6192))
1515
- Better route and dynamic param extraction for Expo Router ([#6197](https://github.com/getsentry/sentry-react-native/pull/6197))
16+
- Attach the active TurboModule method to native crash reports as `contexts.turbo_module` + `turbo_module.name` / `turbo_module.method` tags ([#6227](https://github.com/getsentry/sentry-react-native/pull/6227))
1617

1718
### Fixes
1819

packages/core/src/js/turbomodule/turboModuleTracker.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,26 @@ export function pushTurboModuleCall(args: {
9393
return call.callId;
9494
}
9595

96+
/**
97+
* Updates the `kind` of a previously-pushed call (in place) and re-syncs the
98+
* scope if the call is currently the active one. Used by
99+
* {@link wrapTurboModule} once it discovers that a method's return value is
100+
* thenable.
101+
*
102+
* Returns `true` if the call was found and relabelled.
103+
*/
104+
export function relabelTurboModuleCallKind(callId: number, kind: 'sync' | 'async', scope?: Scope): boolean {
105+
const call = stack.find(c => c.callId === callId);
106+
if (!call || call.kind === kind) {
107+
return !!call;
108+
}
109+
call.kind = kind;
110+
if (stack[stack.length - 1] === call) {
111+
syncToScope(call, scope);
112+
}
113+
return true;
114+
}
115+
96116
/**
97117
* Records the end of a TurboModule method invocation previously started with
98118
* {@link pushTurboModuleCall}. Pops the matching frame off the stack and

packages/core/src/js/turbomodule/wrapTurboModule.ts

Lines changed: 54 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
11
import { logger } from '@sentry/react';
22

3-
import { popTurboModuleCall, pushTurboModuleCall } from './turboModuleTracker';
4-
5-
const WRAPPED_FLAG = '__sentryTurboModuleWrapped__';
3+
import { popTurboModuleCall, pushTurboModuleCall, relabelTurboModuleCallKind } from './turboModuleTracker';
64

75
/**
8-
* Marker added to wrapped modules so we never double-wrap (which would push the
9-
* same call twice onto the tracker stack).
6+
* Modules we've already wrapped. Tracked off-module so that even sealed proxies
7+
* (which can't accept a marker property) are protected from double-wrapping.
108
*/
11-
interface MaybeWrapped {
12-
[WRAPPED_FLAG]?: boolean;
9+
let wrappedModules = new WeakSet<object>();
10+
11+
/** Tests only. */
12+
export function _resetWrappedModules(): void {
13+
wrappedModules = new WeakSet<object>();
1314
}
1415

1516
/**
@@ -18,8 +19,8 @@ interface MaybeWrapped {
1819
* `module` reference for chaining convenience.
1920
*
2021
* - Sync methods are tracked as `kind: 'sync'` and popped right after the call.
21-
* - Async methods (those returning a thenable) are tracked as `kind: 'async'`
22-
* and popped when the returned promise settles.
22+
* - Async methods (those returning a thenable) are relabelled to `kind: 'async'`
23+
* right after the call dispatches and popped when the returned promise settles.
2324
*
2425
* `skip` can be used to opt specific method names out of tracking (e.g. very
2526
* hot, no-op methods like RN's `addListener`/`removeListeners` event-emitter
@@ -34,15 +35,24 @@ export function wrapTurboModule<T extends object>(
3435
return module;
3536
}
3637

37-
const maybeWrapped = module as T & MaybeWrapped;
38-
if (maybeWrapped[WRAPPED_FLAG]) {
38+
if (wrappedModules.has(module)) {
3939
return module;
4040
}
41+
wrappedModules.add(module);
4142

4243
const skip = new Set(options.skip ?? []);
44+
const methodNames = collectMethodNames(module);
45+
46+
if (methodNames.length === 0) {
47+
logger.warn(
48+
`[TurboModuleTracker] No methods discovered on '${name}' — TurboModule context will not be attached for this module. ` +
49+
`This usually means the module is a JSI HostObject that only materialises methods on first access.`,
50+
);
51+
return module;
52+
}
4353

4454
const target = module as unknown as Record<string, unknown>;
45-
for (const key of Object.keys(target)) {
55+
for (const key of methodNames) {
4656
if (skip.has(key)) {
4757
continue;
4858
}
@@ -52,9 +62,9 @@ export function wrapTurboModule<T extends object>(
5262
}
5363
const originalFn = original as (...a: unknown[]) => unknown;
5464

55-
target[key] = function sentryTurboModuleWrapper(this: unknown, ...args: unknown[]): unknown {
65+
const wrapper = function sentryTurboModuleWrapper(this: unknown, ...args: unknown[]): unknown {
5666
// We don't know yet whether `original` is sync or async — start optimistic
57-
// as sync, upgrade the scope context if the result is thenable.
67+
// as sync, relabel to 'async' if the result turns out to be thenable.
5868
const callId = pushTurboModuleCall({ name, method: key, kind: 'sync' });
5969
let result: unknown;
6070
try {
@@ -65,13 +75,7 @@ export function wrapTurboModule<T extends object>(
6575
}
6676

6777
if (isThenable(result)) {
68-
// Re-record as async — clearer in the report. We just overwrite the
69-
// existing tracker frame in place by popping + re-pushing with a fresh
70-
// id would lose ordering, so instead we leave the stack frame alone
71-
// and only relabel for the scope on completion (it's the *active*
72-
// call's `kind` that ends up in `contexts.turbo_module`, and the
73-
// outer perf-logger driven users can push with `kind: 'async'`
74-
// directly when they know up front).
78+
relabelTurboModuleCallKind(callId, 'async');
7579
return (result as Promise<unknown>).then(
7680
value => {
7781
popTurboModuleCall(callId);
@@ -87,29 +91,41 @@ export function wrapTurboModule<T extends object>(
8791
popTurboModuleCall(callId);
8892
return result;
8993
};
90-
}
9194

92-
try {
93-
Object.defineProperty(module, WRAPPED_FLAG, {
94-
value: true,
95-
enumerable: false,
96-
configurable: false,
97-
writable: false,
98-
});
99-
} catch (e) {
100-
// Some TurboModule proxies are sealed — that's fine, we still patched the
101-
// methods, but a second wrap call would be a no-op anyway because the
102-
// properties now point at our wrappers (re-wrapping would still push
103-
// through to `original` which is itself a wrapper, but the per-call
104-
// pushes would double up). Log so this is visible during development.
105-
logger.warn(
106-
`[TurboModuleTracker] Could not mark ${name} as wrapped — repeated wrapping would double-track invocations.`,
107-
);
95+
try {
96+
target[key] = wrapper;
97+
} catch {
98+
// Sealed / non-writable property — can't intercept this method, but we
99+
// can still wrap the rest. Skip silently; the module-level method-count
100+
// check above is the cliff that catches the "wrapped nothing" case.
101+
}
108102
}
109103

110104
return module;
111105
}
112106

107+
/**
108+
* Returns the union of own + prototype-chain method names on `module`,
109+
* deduplicated and skipping `Object.prototype`. Walking the prototype chain is
110+
* necessary for JSI HostObject-backed TurboModule proxies under RN's New
111+
* Architecture, which can expose methods via the proto chain rather than as
112+
* own enumerable properties.
113+
*/
114+
function collectMethodNames(module: object): string[] {
115+
const seen = new Set<string>();
116+
let current: object | null = module;
117+
while (current && current !== Object.prototype) {
118+
for (const key of Object.getOwnPropertyNames(current)) {
119+
if (key === 'constructor') {
120+
continue;
121+
}
122+
seen.add(key);
123+
}
124+
current = Object.getPrototypeOf(current) as object | null;
125+
}
126+
return Array.from(seen);
127+
}
128+
113129
function isThenable(value: unknown): value is PromiseLike<unknown> {
114130
if (!value || (typeof value !== 'object' && typeof value !== 'function')) {
115131
return false;

packages/core/test/turbomodule/turboModuleTracker.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
getTurboModuleCallStack,
77
popTurboModuleCall,
88
pushTurboModuleCall,
9+
relabelTurboModuleCallKind,
910
} from '../../src/js/turbomodule/turboModuleTracker';
1011

1112
describe('turboModuleTracker', () => {
@@ -90,6 +91,32 @@ describe('turboModuleTracker', () => {
9091
expect(getActiveTurboModuleCall()?.callId).toBe(id);
9192
});
9293

94+
it('relabels the active call kind and re-syncs the scope', () => {
95+
const id = pushTurboModuleCall({ name: 'RNSentry', method: 'captureEnvelope', kind: 'sync', scope });
96+
97+
const relabelled = relabelTurboModuleCallKind(id, 'async', scope);
98+
99+
expect(relabelled).toBe(true);
100+
expect(getActiveTurboModuleCall()?.kind).toBe('async');
101+
expect(scope.getScopeData().contexts.turbo_module).toMatchObject({ kind: 'async' });
102+
});
103+
104+
it('relabels a non-top frame without touching the scope context', () => {
105+
const outer = pushTurboModuleCall({ name: 'M', method: 'outer', kind: 'sync', scope });
106+
pushTurboModuleCall({ name: 'M', method: 'inner', kind: 'sync', scope });
107+
108+
relabelTurboModuleCallKind(outer, 'async', scope);
109+
110+
// outer was relabelled
111+
expect(getTurboModuleCallStack().find(c => c.callId === outer)?.kind).toBe('async');
112+
// but the active scope context still describes the top of stack ('inner', sync)
113+
expect(scope.getScopeData().contexts.turbo_module).toMatchObject({ method: 'inner', kind: 'sync' });
114+
});
115+
116+
it('relabel returns false for an unknown id', () => {
117+
expect(relabelTurboModuleCallKind(9999, 'async')).toBe(false);
118+
});
119+
93120
it('assigns monotonically increasing call ids', () => {
94121
const a = pushTurboModuleCall({ name: 'M', method: 'a', kind: 'sync', scope });
95122
const b = pushTurboModuleCall({ name: 'M', method: 'b', kind: 'sync', scope });

packages/core/test/turbomodule/wrapTurboModule.test.ts

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@ import * as SentryCore from '@sentry/core';
22
import { Scope } from '@sentry/core';
33

44
import { _resetTurboModuleTracker, getTurboModuleCallStack } from '../../src/js/turbomodule/turboModuleTracker';
5-
import { wrapTurboModule } from '../../src/js/turbomodule/wrapTurboModule';
5+
import { _resetWrappedModules, wrapTurboModule } from '../../src/js/turbomodule/wrapTurboModule';
66

77
describe('wrapTurboModule', () => {
88
let scope: Scope;
99

1010
beforeEach(() => {
1111
_resetTurboModuleTracker();
12+
_resetWrappedModules();
1213
scope = new Scope();
1314
jest.spyOn(SentryCore, 'getCurrentScope').mockReturnValue(scope);
1415
});
@@ -74,6 +75,26 @@ describe('wrapTurboModule', () => {
7475
expect(getTurboModuleCallStack()).toEqual([]);
7576
});
7677

78+
it('relabels async calls to kind="async" once the call returns a thenable', () => {
79+
let resolveCall: (value: string) => void = () => undefined;
80+
const module = {
81+
asyncOp: () =>
82+
new Promise<string>(resolve => {
83+
resolveCall = resolve;
84+
}),
85+
};
86+
87+
wrapTurboModule('Mod', module);
88+
89+
const promise = module.asyncOp();
90+
91+
expect(getTurboModuleCallStack()[0]).toMatchObject({ method: 'asyncOp', kind: 'async' });
92+
expect(scope.getScopeData().contexts.turbo_module).toMatchObject({ method: 'asyncOp', kind: 'async' });
93+
94+
resolveCall('done');
95+
return promise;
96+
});
97+
7798
it('pops when an async method rejects', async () => {
7899
const module = {
79100
asyncFail: () => Promise.reject(new Error('nope')),
@@ -115,6 +136,54 @@ describe('wrapTurboModule', () => {
115136
expect(module.doStuff).toBe(wrappedOnce);
116137
});
117138

139+
it('does not double-wrap a sealed module on repeated calls', () => {
140+
const module: { doStuff: () => void } = {
141+
doStuff: () => undefined,
142+
};
143+
wrapTurboModule('Mod', module);
144+
Object.seal(module);
145+
146+
// Repeated wrap attempts must be no-ops, otherwise every real call would
147+
// push the same frame multiple times onto the tracker stack.
148+
wrapTurboModule('Mod', module);
149+
wrapTurboModule('Mod', module);
150+
151+
module.doStuff();
152+
expect(getTurboModuleCallStack()).toEqual([]);
153+
});
154+
155+
it('discovers methods exposed via the prototype chain (JSI HostObject shape)', () => {
156+
let stackDuringCall: ReturnType<typeof getTurboModuleCallStack> = [];
157+
class HostObjectLike {
158+
public doStuff(): string {
159+
stackDuringCall = getTurboModuleCallStack();
160+
return 'ok';
161+
}
162+
}
163+
const module = new HostObjectLike();
164+
165+
// sanity: methods are exposed via the prototype, not as own properties
166+
expect(Object.keys(module)).toEqual([]);
167+
168+
wrapTurboModule('HostObj', module);
169+
170+
const result = module.doStuff();
171+
172+
expect(result).toBe('ok');
173+
expect(stackDuringCall).toHaveLength(1);
174+
expect(stackDuringCall[0]).toMatchObject({ name: 'HostObj', method: 'doStuff' });
175+
});
176+
177+
it('warns and bails out cleanly when no methods are discoverable', () => {
178+
const warnSpy = jest.spyOn(require('@sentry/react').logger, 'warn').mockImplementation(() => undefined);
179+
180+
const opaque = Object.create(null) as object;
181+
182+
wrapTurboModule('Opaque', opaque);
183+
184+
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("No methods discovered on 'Opaque'"));
185+
});
186+
118187
it('ignores non-function properties', () => {
119188
const module: { version: string; doStuff: () => number } = {
120189
version: '1.0.0',

0 commit comments

Comments
 (0)