sentry-react-native icon indicating copy to clipboard operation
sentry-react-native copied to clipboard

Duplicate http / xhr breadcrumbs

Open AntoineThibi opened this issue 2 years ago • 11 comments

OS:

  • [ ] Windows
  • [x] MacOS
  • [ ] Linux

Platform:

  • [x] iOS
  • [ ] Android

SDK:

  • [x] @sentry/react-native (>= 1.0.0)
  • [ ] react-native-sentry (<= 0.43.2)

SDK version: 4.13.0

react-native version: 0.71.7

Are you using Expo?

  • [x] Yes
  • [ ] No

Are you using sentry.io or on-premise?

  • [ ] sentry.io (SaaS)
  • [ ] on-premise

If you are using sentry.io, please post a link to your issue so we can take a look:

[Link to issue]

Configuration:

(@sentry/react-native)

Sentry.init({
  dsn: 'https://[email protected]/...'
  enableInExpoDevelopment: true,
  tracesSampleRate: 1,
  normalizeDepth: 10,
  integrations: [
       new Sentry.Native.ReactNativeTracing({
          routingInstrumentation,
          tracePropagationTargets: [appEnv.apiUrl],
        }),
    ],
});

I have following issue:

When I have a natif crash, the breadcrumbs for the http request appears 3 times : Capture d’écran 2023-05-05 à 10 16 10

After looking into it, I have the impression that the breadcrumbs of category "http" are coming from the native : if I look into the beforeBreadcrumb in the Sentry.init (in JS), I can't find those breadcrumbs. I only find the "xhr" breadcrumb.

Steps to reproduce:

  • Init Sentry
  • Make an api call
  • Crash the app using Sentry.Native.nativeCrash

Actual result:

In the error, I have 3 times the same call API. If I filter my breadcrumbs with beforeBreadcrumbs, only the xhr will be affected.

Capture d’écran 2023-05-05 à 10 16 10

Expected result:

There should be only one log of the API call. It should be affected by the beforeBreadcrumbs in the Sentry.init.

AntoineThibi avatar May 05 '23 12:05 AntoineThibi

Hi, thank you for the message, this looks like a bug, we are looking into it.

This might be related to a sentry-cocoa issue.. Plus one duplicate is from the RN layer.

krystofwoldrich avatar May 08 '23 08:05 krystofwoldrich

I've tested this and the SDK is creating duplicate breadcrumbs.

krystofwoldrich avatar May 19 '23 11:05 krystofwoldrich

SentryNetworkTracker from sentry-cocoa gets executed on xhr and fetch requests from the RN JS layer.

Tested on the sample-new-arch with RN 0.71.

JS network requests are executed as NSURLRequest in RN on iOS.

krystofwoldrich avatar May 19 '23 21:05 krystofwoldrich

Good catch @krystofwoldrich so at the end, fetch forwards to iOS native, and then Sentry swizzling is able to detect it. The fix is more complex tho because we want breadcrumbs for JS and Native but also not duplicating it. An alternative is to not log fetch crumbs for iOS when native is enabled.

marandaneto avatar May 21 '23 07:05 marandaneto

I see multiple solutions each with different drawbacks ~(from most favorable to least)~:

~1. Only create breadcrumbs in JS (works with before-breadcrumb, lose native requests - not initiated from JS)~ ~2. Only create breadcrumbs in Obj-C (doesn't work with before-breadcrumbs, keeps all requests)~ 3. Don't create breadcrumbs on native for Requests from JS (works with before-breadcrumb, keeps all requests, requires marking the request from JS for example extra header that will be removed by sentry-cocoa interceptor). 4. Filter duplicates (it might not be possible to recognize duplicates and consecutive calls)

krystofwoldrich avatar May 23 '23 15:05 krystofwoldrich

Is this still in consideration for being worked on? The native breadcrumbs are unable to be filtered out using the beforeBreadcrumb hook.

My app makes a lot of minor network requests which aren't significant enough to be a breadcrumb. But without the filtering in beforeBreadcrumb, there's no way to prevent them from going into the breadcrumb stream and crowding out the real breadcrumbs that would be helpful for debugging.

Thanks for looking into it!

GriffinSSloves avatar Nov 02 '23 22:11 GriffinSSloves

@GriffinSSloves Hi, yes this is still on our backlog.

Currently, you can remove the native breadcrumbs in beforeSend. It's a workaround, the breadcrumbs will still be created but they won't show in Sentry.

Sentry.init({
  beforeSend: (event: Sentry.Event) => {
    event.breadcrumbs = event.breadcrumbs?.filter(
      // Add your condition
      breadcrumb => breadcrumb.category !== 'http',
    );
    return event;
  },
};

krystofwoldrich avatar Nov 03 '23 07:11 krystofwoldrich

probably better to use beforeBreadcrumb. So that it won't occupy the breadcrumb stack

beforeBreadcrumb: (breadcrumb: Sentry.Breadcrumb) => {
      console.log('sentry-event beforeBreadcrumb:', breadcrumb);

      // remove automatic http breadcrumbs (from native)
      const isHttp = breadcrumb.category === 'http';

      if (!isHttp) {
        return breadcrumb;
      }

      return null;
    },

LonelyCpp avatar Jun 05 '24 17:06 LonelyCpp

@LonelyCpp The beforeBreadcrumb callback is not executed for the native breadcrumbs.

They are added here -> https://github.com/getsentry/sentry-react-native/blob/5e2c81c3cb95db62ff47dc0dd2d841b5a744b7d5/src/js/integrations/devicecontext.ts#L94-L99

krystofwoldrich avatar Jun 07 '24 14:06 krystofwoldrich