diff --git a/CHANGELOG.md b/CHANGELOG.md index b042a91ec9..be0abb7d65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ ## Unreleased +### Fixes + +- Enable fetch instrumentation when Expo SDK 56's native `expo/fetch` is active ([#6226](https://github.com/getsentry/sentry-react-native/pull/6226)) + ### Internal - Convert `sentry.gradle` to Kotlin DSL (`sentry.gradle.kts`) ([#6119](https://github.com/getsentry/sentry-react-native/pull/6119)) diff --git a/packages/core/etc/sentry-react-native.api.md b/packages/core/etc/sentry-react-native.api.md index 5c1ca742ea..735d0440e8 100644 --- a/packages/core/etc/sentry-react-native.api.md +++ b/packages/core/etc/sentry-react-native.api.md @@ -855,7 +855,7 @@ export function wrapExpoRouter(router: T): T; // // src/js/feedback/integration.ts:21:5 - (ae-forgotten-export) The symbol "ScreenshotButtonProps" needs to be exported by the entry point index.d.ts // src/js/feedback/integration.ts:23:5 - (ae-forgotten-export) The symbol "FeedbackFormTheme" needs to be exported by the entry point index.d.ts -// src/js/tracing/reactnativetracing.ts:94:3 - (ae-forgotten-export) The symbol "ReactNativeTracingState" needs to be exported by the entry point index.d.ts +// src/js/tracing/reactnativetracing.ts:90:3 - (ae-forgotten-export) The symbol "ReactNativeTracingState" needs to be exported by the entry point index.d.ts // src/js/tracing/reactnavigation.ts:219:3 - (ae-forgotten-export) The symbol "RouteOverrideProvider" needs to be exported by the entry point index.d.ts // (No @packageDocumentation comment for this package) diff --git a/packages/core/src/js/integrations/breadcrumbs.ts b/packages/core/src/js/integrations/breadcrumbs.ts index 80064bc722..371e28fdcb 100644 --- a/packages/core/src/js/integrations/breadcrumbs.ts +++ b/packages/core/src/js/integrations/breadcrumbs.ts @@ -2,7 +2,7 @@ import type { Integration } from '@sentry/core'; import { breadcrumbsIntegration as browserBreadcrumbsIntegration } from '@sentry/browser'; -import { isWeb } from '../utils/environment'; +import { isExpoFetchEnabled, isWeb } from '../utils/environment'; interface BreadcrumbsOptions { /** @@ -26,9 +26,10 @@ interface BreadcrumbsOptions { * Log HTTP requests done with the global Fetch API. * * Disabled by default in React Native because fetch is built on XMLHttpRequest. - * Enabled by default on web. + * Enabled by default on web and when Expo's native fetch (`expo/fetch`) is active. * - * Setting `fetch: true` and `xhr: true` will cause duplicates in React Native. + * Setting `fetch: true` and `xhr: true` will cause duplicates in React Native + * when using the default XHR-based fetch polyfill. */ fetch: boolean; @@ -47,10 +48,11 @@ interface BreadcrumbsOptions { /** * Log HTTP requests done with the XHR API. * - * Because React Native global fetch is built on XMLHttpRequest, - * this will also log `fetch` network requests. - * - * Setting `fetch: true` and `xhr: true` will cause duplicates in React Native. + * In standard React Native, fetch is built on XMLHttpRequest, + * so XHR breadcrumbs also capture fetch requests. + * When Expo's native fetch (`expo/fetch`) is active, XHR does not + * capture fetch requests — both `fetch` and `xhr` can be enabled + * without duplicates. */ xhr: boolean; } @@ -63,7 +65,7 @@ export const breadcrumbsIntegration = (options: Partial = {} console: true, sentry: true, ...options, - fetch: options.fetch ?? (isWeb() ? true : false), + fetch: options.fetch ?? (isWeb() || isExpoFetchEnabled()), dom: isWeb() ? (options.dom ?? true) : false, history: isWeb() ? (options.history ?? true) : false, }; diff --git a/packages/core/src/js/tracing/reactnativetracing.ts b/packages/core/src/js/tracing/reactnativetracing.ts index 0b2284f3a3..01080efa9e 100644 --- a/packages/core/src/js/tracing/reactnativetracing.ts +++ b/packages/core/src/js/tracing/reactnativetracing.ts @@ -3,7 +3,7 @@ import type { Client, Event, EventHint, Integration, StartSpanOptions } from '@s import { instrumentOutgoingRequests } from '@sentry/browser'; import { debug, getClient } from '@sentry/core'; -import { isWeb } from '../utils/environment'; +import { isExpoFetchEnabled, isWeb } from '../utils/environment'; import { getDevServer } from './../integrations/debugsymbolicatorutils'; import { getTransactionEventDiscardReason } from './onSpanEndUtils'; import { addDefaultOpForSpanFrom, addThreadInfoToSpan, defaultIdleOptions } from './span'; @@ -74,11 +74,7 @@ function getDefaultTracePropagationTargets(): RegExp[] | undefined { } export const defaultReactNativeTracingOptions: ReactNativeTracingOptions = { - // Fetch in React Native is a `whatwg-fetch` polyfill which uses XHR under the hood. - // This causes duplicates when both `traceFetch` and `traceXHR` are enabled at the same time. - // https://github.com/facebook/react-native/blob/28945c68da056ab2ac01de7e542a845b2bca6096/packages/react-native/Libraries/Network/fetch.js - // (RN Web uses browsers native fetch implementation) - traceFetch: isWeb() ? true : false, + traceFetch: false, traceXHR: true, enableHTTPTimings: true, }; @@ -98,8 +94,15 @@ export const reactNativeTracingIntegration = ( currentRoute: undefined, }; + // RN's default fetch is a `whatwg-fetch` polyfill over XHR, so tracing both causes duplicates. + // Expo SDK 56+ replaces fetch with a native implementation (`expo/fetch`) that bypasses XHR, + // so fetch tracing must be enabled for network requests to be captured. + // Computed here (not at module level) to ensure expo/fetch polyfill is installed before detection. + const traceFetchDefault = isWeb() || isExpoFetchEnabled(); + const finalOptions = { ...defaultReactNativeTracingOptions, + traceFetch: traceFetchDefault, ...options, beforeStartSpan: options.beforeStartSpan ?? ((options: StartSpanOptions) => options), finalTimeoutMs: options.finalTimeoutMs ?? defaultIdleOptions.finalTimeout, diff --git a/packages/core/src/js/utils/environment.ts b/packages/core/src/js/utils/environment.ts index abc3c85ed3..b67f9a9fec 100644 --- a/packages/core/src/js/utils/environment.ts +++ b/packages/core/src/js/utils/environment.ts @@ -59,6 +59,19 @@ export function isWeb(): boolean { return Platform.OS === 'web'; } +/** + * Checks if Expo's native fetch implementation (`expo/fetch`) is active. + * Expo SDK 56+ replaces `globalThis.fetch` with a native implementation + * that bypasses XHR entirely. The SDK needs to know this to enable + * fetch instrumentation instead of relying on XHR instrumentation. + */ +export function isExpoFetchEnabled(): boolean { + return ( + typeof globalThis.fetch === 'function' && + (globalThis.fetch as unknown as Record)[Symbol.for('expo.builtin')] === true + ); +} + /** Checks if the current platform is not web */ export function notWeb(): boolean { return Platform.OS !== 'web'; diff --git a/packages/core/test/integrations/breadcrumbs.test.ts b/packages/core/test/integrations/breadcrumbs.test.ts index 3053726603..77f848b7a4 100644 --- a/packages/core/test/integrations/breadcrumbs.test.ts +++ b/packages/core/test/integrations/breadcrumbs.test.ts @@ -43,6 +43,35 @@ describe('breadcrumbsIntegration', () => { }); }); + it('enables fetch breadcrumbs when expo/fetch is active', () => { + jest.spyOn(environment, 'isWeb').mockReturnValue(false); + jest.spyOn(environment, 'isExpoFetchEnabled').mockReturnValue(true); + + breadcrumbsIntegration(); + + expect(browserBreadcrumbsIntegration).toHaveBeenCalledWith({ + xhr: true, + console: true, + sentry: true, + dom: false, + fetch: true, + history: false, + }); + }); + + it('respects explicit fetch: false even when expo/fetch is active', () => { + jest.spyOn(environment, 'isWeb').mockReturnValue(false); + jest.spyOn(environment, 'isExpoFetchEnabled').mockReturnValue(true); + + breadcrumbsIntegration({ fetch: false }); + + expect(browserBreadcrumbsIntegration).toHaveBeenCalledWith( + expect.objectContaining({ + fetch: false, + }), + ); + }); + it('respects custom options React Native options', () => { jest.spyOn(environment, 'isWeb').mockReturnValue(false); diff --git a/packages/core/test/tracing/reactnativetracing.test.ts b/packages/core/test/tracing/reactnativetracing.test.ts index 987fb27d15..8c2dfa40ed 100644 --- a/packages/core/test/tracing/reactnativetracing.test.ts +++ b/packages/core/test/tracing/reactnativetracing.test.ts @@ -17,7 +17,7 @@ jest.mock('../../src/js/wrapper', () => { import type { TestClient } from '../mocks/client'; import { reactNativeTracingIntegration } from '../../src/js/tracing/reactnativetracing'; -import { isWeb } from '../../src/js/utils/environment'; +import { isExpoFetchEnabled, isWeb } from '../../src/js/utils/environment'; import { setupTestClient } from '../mocks/client'; jest.mock('../../src/js/tracing/utils', () => { @@ -104,6 +104,78 @@ describe('ReactNativeTracing', () => { }); }); + describe('traceFetch default', () => { + it('disables fetch tracing on mobile without expo/fetch', () => { + (isWeb as jest.MockedFunction).mockReturnValue(false); + (isExpoFetchEnabled as jest.MockedFunction).mockReturnValue(false); + const instrumentOutgoingRequests = jest.spyOn(SentryBrowser, 'instrumentOutgoingRequests'); + setupTestClient({ + enableStallTracking: false, + integrations: [reactNativeTracingIntegration()], + }); + + expect(instrumentOutgoingRequests).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + traceFetch: false, + traceXHR: true, + }), + ); + }); + + it('enables fetch tracing when expo/fetch is active', () => { + (isWeb as jest.MockedFunction).mockReturnValue(false); + (isExpoFetchEnabled as jest.MockedFunction).mockReturnValue(true); + const instrumentOutgoingRequests = jest.spyOn(SentryBrowser, 'instrumentOutgoingRequests'); + setupTestClient({ + enableStallTracking: false, + integrations: [reactNativeTracingIntegration()], + }); + + expect(instrumentOutgoingRequests).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + traceFetch: true, + traceXHR: true, + }), + ); + }); + + it('enables fetch tracing on web', () => { + (isWeb as jest.MockedFunction).mockReturnValue(true); + (isExpoFetchEnabled as jest.MockedFunction).mockReturnValue(false); + const instrumentOutgoingRequests = jest.spyOn(SentryBrowser, 'instrumentOutgoingRequests'); + setupTestClient({ + enableStallTracking: false, + integrations: [reactNativeTracingIntegration()], + }); + + expect(instrumentOutgoingRequests).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + traceFetch: true, + }), + ); + }); + + it('respects explicit user option over defaults', () => { + (isWeb as jest.MockedFunction).mockReturnValue(false); + (isExpoFetchEnabled as jest.MockedFunction).mockReturnValue(true); + const instrumentOutgoingRequests = jest.spyOn(SentryBrowser, 'instrumentOutgoingRequests'); + setupTestClient({ + enableStallTracking: false, + integrations: [reactNativeTracingIntegration({ traceFetch: false })], + }); + + expect(instrumentOutgoingRequests).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + traceFetch: false, + }), + ); + }); + }); + describe('View Names event processor', () => { it('Do not overwrite event app context', () => { const integration = reactNativeTracingIntegration(); diff --git a/packages/core/test/tracing/timetodisplay.test.tsx b/packages/core/test/tracing/timetodisplay.test.tsx index d02a99d0c6..991aa9043e 100644 --- a/packages/core/test/tracing/timetodisplay.test.tsx +++ b/packages/core/test/tracing/timetodisplay.test.tsx @@ -43,6 +43,7 @@ import { nowInSeconds, secondAgoTimestampMs, secondInFutureTimestampMs } from '. jest.mock('../../src/js/utils/environment', () => ({ isWeb: jest.fn().mockReturnValue(false), + isExpoFetchEnabled: jest.fn().mockReturnValue(false), isTurboModuleEnabled: jest.fn().mockReturnValue(false), })); diff --git a/packages/core/test/utils/environment.test.ts b/packages/core/test/utils/environment.test.ts new file mode 100644 index 0000000000..d2270eb290 --- /dev/null +++ b/packages/core/test/utils/environment.test.ts @@ -0,0 +1,45 @@ +import { isExpoFetchEnabled } from '../../src/js/utils/environment'; + +describe('isExpoFetchEnabled', () => { + const originalFetch = globalThis.fetch; + + afterEach(() => { + if (originalFetch) { + globalThis.fetch = originalFetch; + } else { + delete (globalThis as Partial).fetch; + } + }); + + it('returns false when globalThis.fetch is undefined', () => { + delete (globalThis as Partial).fetch; + expect(isExpoFetchEnabled()).toBe(false); + }); + + it('returns false when globalThis.fetch is a plain function without expo.builtin symbol', () => { + globalThis.fetch = jest.fn() as unknown as typeof fetch; + expect(isExpoFetchEnabled()).toBe(false); + }); + + it('returns true when globalThis.fetch has the expo.builtin symbol', () => { + const expoFetch = jest.fn() as unknown as typeof fetch; + Object.defineProperty(expoFetch, Symbol.for('expo.builtin'), { + value: true, + enumerable: false, + configurable: false, + }); + globalThis.fetch = expoFetch; + expect(isExpoFetchEnabled()).toBe(true); + }); + + it('returns false when expo.builtin symbol is present but not true', () => { + const expoFetch = jest.fn() as unknown as typeof fetch; + Object.defineProperty(expoFetch, Symbol.for('expo.builtin'), { + value: false, + enumerable: false, + configurable: false, + }); + globalThis.fetch = expoFetch; + expect(isExpoFetchEnabled()).toBe(false); + }); +});