Some browser integrations does not respect the Integration#setupOnce signature

[ghostd]

Package + Version

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

Version:

5.0.0

Description

Hi,

It seems the integration (provided by the browser package) does not implement the correct signature of setupOnce, eg:
https://github.com/getsentry/sentry-javascript/blob/master/packages/browser/src/integrations/globalhandlers.ts#L50
https://github.com/getsentry/sentry-javascript/blob/master/packages/browser/src/integrations/useragent.ts#L22
https://github.com/getsentry/sentry-javascript/blob/master/packages/browser/src/integrations/trycatch.ts#L172
https://github.com/getsentry/sentry-javascript/blob/master/packages/browser/src/integrations/linkederrors.ts#L43

(maybe others)

Regards

[HazAT]

@ghostd Thanks for pointing this out, is this actually blocking you from using it, does the linter break or anything?
Since these integrations directly live inside @sentry/browser we do not need to rely on the injected functions, instead, we use the one provided in @sentry/browser.

So technically you are right but we don't need to do it.

[ghostd]

@HazAT
I'm trying to debug an issue with the last version, and for now i'm not sure if it's a bug of sentry or a bug in my own code :) I found this inconsistency during my debug.

My init code is (as described in the documentation):

  const client = new BrowserClient({
    dsn: "https://token@sentry.io/XXXX",
    release: `${guessVersion()}`,
    environment: guessEnvironment(),
    beforeSend: beforeSend,
  });
  hub = new Hub(client);

I had to manually add the integrations to have it:

import {BrowserClient, defaultIntegrations, Hub} from "@sentry/browser";

  const client = new BrowserClient({
    dsn: "https://token@sentry.io/XXXX",
    defaultIntegrations: defaultIntegrations,
    release: `${guessVersion()}`,
    environment: guessEnvironment(),
    beforeSend: beforeSend,
  });
  hub = new Hub(client);

And now, the calls to getCurrentHub().getIntegration(UserAgent) seems always return null (i had some debug traces into the sentry package).

I guess i missed something... i'm still digging

[HazAT]

defaultIntegrations is a boolean
you have to do integrations: defaultIntegrations

[ghostd]

Thanks, i tried that too. In both cases, the user agent data are not added. getCurrentHub().getIntegration(UserAgent) still returns null

[HazAT]

OK, that's strange, is it possible for you to go into node_modules and change the transpiled code directly do add the missing parameters there:
So search for line:

UserAgent.prototype.setupOnce = function () {

and replace it with:

UserAgent.prototype.setupOnce = function (addGlobalEventProcessor, getCurrentHub) {

[ghostd]

Already tried ;) This does not resolve my issue.

    UserAgent.prototype.setupOnce = function (addGlobalEventProcessor, getCurrentHub) {
        addGlobalEventProcessor(function (event) {
          console.log("UserAgent integration current hub", getCurrentHub())
          console.log("UserAgent integration ?", getCurrentHub().getIntegration(UserAgent))
            if (getCurrentHub().getIntegration(UserAgent)) {
                if (!global$4.navigator || !global$4.location) {
                    return event;
                }
                // HTTP Interface: https://docs.sentry.io/clientdev/interfaces/http/?platform=javascript
                var request = event.request || {};
                request.url = request.url || global$4.location.href;
                request.headers = request.headers || {};
                request.headers['User-Agent'] = global$4.navigator.userAgent;
                return tslib_1.__assign({}, event, { request: request });
            }
            return event;
        });
    };

The second log returns: "UserAgent integration ? null"

In Hub.prototype.getIntegration, this.getClient() is always falsy.

[HazAT]

Ohhh, I know that the problem is, Integrations only work if there is a currentHub, we have to add this to the documentation.

Please make sure you do the following:

import { makeMain } from "@sentry/hub";

// YOUR CODE
// after you have the hub instance

makeMain(hub);

Hope this fixes it.
Edit: You probably have to install @sentry/hub

[ghostd]

Good catch!

Thank you so much for this 'live' debug :)

BTW, if i've several app/widgets into my HTML page, and can each call makeMain? No risks to have one app overriding the sibling hub?

Edit:
not sure all is fixed. I'll investigate further tomorrow.

[ghostd]

Sorry, that worked because i had a weird call; but with a clean node_modules, the makeMain call is not enough.

Weirdly, if i add a log into UserAgent.prototype.setupOnce it works, if i remove the log it does not:

    UserAgent.prototype.setupOnce = function () {
      // console.log("setup UserAgent", getCurrentHub().getIntegration(UserAgent))
        addGlobalEventProcessor(function (event) {
            if (getCurrentHub().getIntegration(UserAgent)) {
                if (!global$4.navigator || !global$4.location) {
                    return event;
                }
                // HTTP Interface: https://docs.sentry.io/clientdev/interfaces/http/?platform=javascript
                var request = event.request || {};
                request.url = request.url || global$4.location.href;
                request.headers = request.headers || {};
                request.headers['User-Agent'] = global$4.navigator.userAgent;
                return tslib_1.__assign({}, event, { request: request });
            }
            return event;
        });
    };

The event callback is not called, it looks like a timing issue.

Edit:
The relevant part of the commented line seems to be getCurrentHub(). If i add getCurrentHub() before addGlobalEventProcessor, it works.

Edit2:
it seems the global.__SENTRY__.globalEventProcessors is overwritten (or init twice ?)

[HazAT]

@ghostd Sorry for about this, apparently this was always broken, it's not specific to 5.0.0.
I will discuss this tomorrow with the team, it seems to be kind of a fundamental problem.

If I may ask, what is the specific reason you want to have a client/hub with integrations running instead of using init?

[ghostd]

@HazAT i have pages which can contain several app/widgets (different framework, different release cycle, different source repository, and so on), and some of those will use Sentry to handle their errors.

So, maybe i misunderstood the documentation, it seems each of these apps will need a dedicated Sentry instance to avoid conflicts. I followed the documentation. Am i wrong with this?

[HazAT]

No, you are right.

So I have a working running example here, I hope this helps and sorry for the inconvenience:

https://codesandbox.io/s/l7qrkjn7nm

[ghostd]

Thanks, i'm not at the office right now. I'll give a shot tomorrow morning. I'll let you know.

[ghostd]

That does not work. Here is a summary of my code :

In a error-management.js file:

import * as Sentry from "@sentry/browser";

let hub;
export const installErrorHandler = () => {
  console.log("Should be UserAgent", Sentry.defaultIntegrations[6])
  const client = new Sentry.BrowserClient({
    dsn: "https://token@sentry.io/123",
    integrations: [Sentry.defaultIntegrations[6]],
    release: guessRelease(),
    environment: guessEnvironment(),
    beforeSend: beforeSend,
  });
  hub = new Sentry.Hub(client);
  // Sentry.getCurrentHub();
  // makeMain(hub);
};

const beforeSend = (event, hint) => {
  // event should contain a 'request' property with the user agent data
  console.error('beforeSend', event, hint);
  return null;
};

export const captureReactError = (error, errorInfo) => {
  hub.run(currentHub => {
    currentHub.withScope(scope => {
      scope.setExtras(errorInfo);
      currentHub.captureException(error);
    });
  });
};

installErrorHandler is called in my index.js before calling React.render. My main Application component is wrapped by a ErrorBoundary component with the following code:

  componentDidCatch(error, info) {
    this.setState({hasError: true});
    captureReactError(error, info);
  }

If i uncomment Sentry.getCurrentHub(); in the init phase, all is fine :-/