Skip to content

Cannot change/override breadcrumbs integration options #1981

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.

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

Already on GitHub? Sign in to your account

Closed
4 tasks
d7ark opened this issue Apr 2, 2019 · 12 comments
Closed
4 tasks

Cannot change/override breadcrumbs integration options #1981

d7ark opened this issue Apr 2, 2019 · 12 comments

Comments

@d7ark
Copy link

d7ark commented Apr 2, 2019

Package + Version

  • [*] @sentry/browser
  • @sentry/node
  • raven-js
  • raven-node (raven for node)
  • other:

Version:

5.0.2

Description

Simple demo: https://github.com/d7ark/sentry-breadcrumbs-issue (need to provide sentry dsn in index.js).

I cannot find way to override Breadcrumbs integration options. Way provided in documentation is not working.

integrations: [new Sentry.Integrations.Breadcrumbs({ console: false })].

Solution from example app is also not working.

I have a hunch this is because the defaultIntegrations are creating new Integrations.Breadcrumbs which adds event listeners with default options (everything true) and there's no way to remove those listeners. But Im not sure.

I've found a workaround using beforeBreadcrumb but I'd prefer to use intented way (Breadcrumbs integration with { dom: false }).

  // workaround using beforeBreadcrumb
  Sentry.init({
    beforeBreadcrumb: removeUiBreadcrumbs,
    dsn: SENTRY_DSN,
  });

  function removeUiBreadcrumbs(breadcrumb) {
    return breadcrumb.category.startsWith('ui.') ? null : breadcrumb;
  }
@HazAT
Copy link
Member

HazAT commented Apr 2, 2019

Thanks for taking the time writing this excellent bug report, I am looking into this right now.

@HazAT
Copy link
Member

HazAT commented Apr 2, 2019

found the issue, 5.0.4 will fix this
#1983

@HazAT
Copy link
Member

HazAT commented Apr 2, 2019

OK, we found the issue, the fix is not as simple as we thought.
We hope to fix it tomorrow with 5.0.5

@d7ark
Copy link
Author

d7ark commented Apr 2, 2019

It's perfectly fine. Thank you for your work. I'll keep an eye out.

@HazAT
Copy link
Member

HazAT commented Apr 3, 2019

@d7ark So the problem is that we are emitting a breadcrumb in the try/catch integration we always emit a breadcrumb in case of an error. So the breadcrumb integration itself is working correctly.
We still haven't found a good solution for this since wrapping addEventListener twice is tricky.

So to confirm, if you also remove the TryCatch Integration it should not emit a breadcrumb anymore.

HazAT added a commit that referenced this issue Apr 17, 2019
HazAT added a commit that referenced this issue Apr 18, 2019
@HazAT HazAT closed this as completed in bd3f72e Apr 18, 2019
@HazAT
Copy link
Member

HazAT commented Apr 18, 2019

The fix for this will be shipped in the new version today:

The way it works then is like this:

Sentry.init({ 
...
    integrations(integrations) {
      integrations = integrations.filter(
        integration => integration.name !== "Breadcrumbs"
      );
      return [
        ...integrations,
        new Sentry.Integrations.Breadcrumbs({ dom: false })
      ];
    },
...
  });

@zeusstl
Copy link

zeusstl commented Oct 4, 2021

Hello,

SDK:
Name | sentry.cocoa
Version | 7.3.0

I receive this error implementing the recommendation above from @HazAT .

Sentry.Integrations.Breadcrumbs is not a constructor
    at integrations (App.tsx:83)
    at getIntegrationsToSetup (integration.js:29)
    at Object.setupIntegrations (integration.js:59)
    at ReactNativeClient.BaseClient.setupIntegrations (baseclient.js:155)
    at Hub.bindClient (hub.js:58)
    at initAndBind (sdk.js:19)
    at Object.init (sdk.js:74)
    at eval (App.tsx:65)
    at invokePassiveEffectCreate (ReactNativeRenderer-dev.js:19280)
    at Object.invokeGuardedCallbackProd (ReactNativeRenderer-dev.js:93)
...

I also tried the following and it gave the same error:
(Reference: https://docs.sentry.io/platforms/javascript/configuration/integrations/default/#modifying-system-integrations)

Sentry.init({
    dsn: SENTRY_DSN,
    debug: false,
    environment: APP_ENV,
    release: version,
    integrations: [new Sentry.Integrations.Breadcrumbs({ console: false })],
});

I deactivate console logs in production and Sentry is reactivating them which causes the production react native app to get overwhelmed on real devices.

Can anyone advise on where to find more accurate/complete documentation for how to deactivate the console breadcrumbs? (Or if you just have an answer that you can post here, that would be greatly appreciated.)

@kamilogorek
Copy link
Contributor

Can you show a larger piece of code that includes your imports?

@zeusstl
Copy link

zeusstl commented Oct 5, 2021

import React, { useState, useEffect } from 'react';
import { View } from 'react-native';
import codePush from 'react-native-code-push';
import { MenuProvider } from 'react-native-popup-menu';
import { NativeRouter } from 'react-router-native';
import { Provider } from 'react-redux';
import { PersistGate } from 'redux-persist/integration/react';
import * as Sentry from '@sentry/react-native';
import {ThemeProvider} from 'styled-components/native';
import {useTheme} from 'src/config/theme';
import Router from 'src/ui/AppRouter';

Is there something I should be explicitly importing?

@kamilogorek
Copy link
Contributor

Change Sentry.Integrations.Breadcrumbs to Sentry.BrowserIntegrations.Breadcrumbs and it should work - https://github.com/getsentry/sentry-react-native/blob/master/src/js/index.ts#L46

@zeusstl
Copy link

zeusstl commented Oct 6, 2021

Thanks. That resolved it. In the meantime, I found this too which stopped the console logs from being shown in the breadcrumbs:

beforeBreadcrumb(breadcrumb, hint) {
    return (breadcrumb.category === "console" && env=="production") ? null : breadcrumb;
},

Wondering if there is an advantage to one over the other - beforeBreadcrumb vs the integrations solution. It seems like not even implementing the integration in the first place would be advantageous, right?

Any thoughts there?

@kamilogorek
Copy link
Contributor

It seems like not even implementing the integration in the first place would be advantageous, right?

Correct. Disabling integration will prevent the wrapping of the API completely, which will make sure its not performing any unnecessary work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants