Skip to content

fix(core): Strip callback options before the native SDK starts #6847

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alwx merged 2 commits into from
Oct 6, 2026
Merged

fix(core): Strip callback options before the native SDK starts #6847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c5500c0
fix(core): Strip callback options before the native SDK starts
alwx Oct 6, 2026
469b756
chore: Link the CHANGELOG entry to the PR
alwx Oct 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
- Populate the Hermes runtime version on JS profiles instead of sending an empty value ([#6817](https://github.com/getsentry/sentry-react-native/pull/6817))
- Mark `@sentry/react-native` as side-effect free so bundlers can tree-shake unused exports ([#6829](https://github.com/getsentry/sentry-react-native/pull/6829))
- Resolve `@sentry/react-native` from the Android project directory in the Expo plugin `build.gradle` line, instead of the directory where Gradle started ([#6840](https://github.com/getsentry/sentry-react-native/pull/6840))
- If `beforeSendLog` or another callback option is set, the app no longer crashes on iOS when the native SDK emits a log ([#6847](https://github.com/getsentry/sentry-react-native/pull/6847))

Callback options are for the JavaScript layer only. The SDK now removes all of them before it starts the native SDK.

### Dependencies

Expand Down
33 changes: 20 additions & 13 deletions packages/core/src/js/wrapper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,22 @@ interface SentryNativeWrapper {

const EOL = encodeUTF8('\n');

/**
* A JS function crosses the bridge as a native callback that only accepts an array.
* The native SDK calls hooks such as `beforeSend` or `beforeSendLog` with a native
* object instead, which crashes the app.
*/
function withoutFunctionValues<T extends object>(options: T): T {
const source = options as Record<string, unknown>;
const result: Record<string, unknown> = {};
for (const key of Object.keys(source)) {
if (typeof source[key] !== 'function') {
result[key] = source[key];
}
}
return result as T;
}

/**
* Our internal interface for calling native functions
*/
Expand Down Expand Up @@ -314,21 +330,12 @@ export const NATIVE: SentryNativeWrapper = {

// filter out all the options that would crash native.
/* oxlint-disable typescript-eslint(no-unused-vars) */
const {
beforeSend,
beforeBreadcrumb,
beforeSendTransaction,
beforeSendMetric,
integrations,
ignoreErrors,
logsOrigin,
profilingOptions,
androidProfilingOptions,
onNativeLog,
...filteredOptions
} = options;
const { integrations, ignoreErrors, logsOrigin, profilingOptions, androidProfilingOptions, ...remainingOptions } =
options;
/* oxlint-enable typescript-eslint(no-unused-vars) */

const filteredOptions = withoutFunctionValues(remainingOptions);

// Move profilingOptions into _experiments
// Support deprecated androidProfilingOptions for backwards compatibility
const resolvedProfilingOptions = profilingOptions ?? androidProfilingOptions;
Comment on lines 331 to 341

@sentry sentry Bot Oct 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The withoutFunctionValues function only performs a shallow filter, failing to remove functions from nested objects like mobileReplayOptions, which can cause a native crash.
Severity: HIGH

Suggested Fix

Modify the withoutFunctionValues function to recursively traverse the options object and remove any nested functions. This will ensure that objects like mobileReplayOptions are properly sanitized before being passed to the native bridge.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/core/src/js/wrapper.ts#L330-L341

Potential issue: The `withoutFunctionValues` function is intended to strip function
properties from an options object before it's passed to the native layer. However, it
only performs a shallow filter. If an option like `mobileReplayOptions` is passed, which
is an object that can contain a function property like `beforeErrorSampling`, this
nested function is not removed. The `typeof` check for `mobileReplayOptions` returns
"object", so the entire object, including the nested function, is passed through. This
will cause a crash in the native SDK when it tries to process the function.

Did we get this right? 馃憤 / 馃憥 to inform future reviews.

@antonis antonis Oct 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds valid but I think it is preexisting

Expand Down
56 changes: 56 additions & 0 deletions packages/core/test/wrapper.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,62 @@ describe('Tests Native Wrapper', () => {
expect(NATIVE.enableNative).toBe(true);
});

test('filter beforeSendLog when initializing Native SDK', async () => {
await NATIVE.initNativeSdk({
dsn: VALID_DSN,
enableNative: true,
autoInitializeNativeSdk: true,
beforeSendLog: jest.fn(),
devServerUrl: undefined,
defaultSidecarUrl: undefined,
mobileReplayOptions: undefined,
});

expect(RNSentry.initNativeSdk).toHaveBeenCalled();
// @ts-expect-error mock value
const initParameter = RNSentry.initNativeSdk.mock.calls[0][0];
expect(initParameter).not.toHaveProperty('beforeSendLog');
expect(NATIVE.enableNative).toBe(true);
});

test('filter beforeSendSpan when initializing Native SDK', async () => {
await NATIVE.initNativeSdk({
dsn: VALID_DSN,
enableNative: true,
autoInitializeNativeSdk: true,
beforeSendSpan: jest.fn(),
devServerUrl: undefined,
defaultSidecarUrl: undefined,
mobileReplayOptions: undefined,
});

expect(RNSentry.initNativeSdk).toHaveBeenCalled();
// @ts-expect-error mock value
const initParameter = RNSentry.initNativeSdk.mock.calls[0][0];
expect(initParameter).not.toHaveProperty('beforeSendSpan');
expect(NATIVE.enableNative).toBe(true);
});

test('filter every function option when initializing Native SDK', async () => {
await NATIVE.initNativeSdk({
dsn: VALID_DSN,
enableNative: true,
autoInitializeNativeSdk: true,
onReady: jest.fn(),
onNativeLog: jest.fn(),
tracesSampler: jest.fn(),
devServerUrl: undefined,
defaultSidecarUrl: undefined,
mobileReplayOptions: undefined,
});

expect(RNSentry.initNativeSdk).toHaveBeenCalled();
// @ts-expect-error mock value
const initParameter = RNSentry.initNativeSdk.mock.calls[0][0];
expect(Object.values(initParameter).every(value => typeof value !== 'function')).toBe(true);
expect(NATIVE.enableNative).toBe(true);
});

test('passes sdkVersion to native SDK', async () => {
await NATIVE.initNativeSdk({
dsn: VALID_DSN,
Expand Down
Loading