Some browser integrations does not respect the Integration#setupOnce signature #1969

ghostd · 2019-04-01T14:37:43Z

Package + Version

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

The text was updated successfully, but these errors were encountered:

HazAT · 2019-04-01T14:50:25Z

@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 · 2019-04-01T14:58:38Z

@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://[email protected]/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://[email protected]/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 · 2019-04-01T15:05:59Z

defaultIntegrations is a boolean
you have to do integrations: defaultIntegrations

ghostd · 2019-04-01T15:12:28Z

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

HazAT · 2019-04-01T15:18:19Z

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 · 2019-04-01T15:23:04Z

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 · 2019-04-01T15:24:43Z

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 · 2019-04-01T15:43:31Z

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 · 2019-04-01T16:13:50Z

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 · 2019-04-01T19:17:53Z

@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 · 2019-04-01T19:32:04Z

@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 · 2019-04-01T19:53:30Z

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 · 2019-04-01T19:56:08Z

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

ghostd · 2019-04-02T06:06:54Z

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://[email protected]/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 :-/

ghostd · 2019-04-02T07:33:29Z

Here are my last find.

When the processors are added with addGlobalEventProcessor, the global __SENTRY__ object contains a logger and an array (of processors). Later, Scope.prototype.applyToEvent is called, the global __SENTRY__ object only contains a hub property and an empty array (of processors)

HazAT · 2019-04-02T07:33:29Z

I was able to find the issue, fixed it, 5.0.3 is on it's way. Your code should work just fine then.

ghostd · 2019-04-02T07:34:08Z

Amazing, your commit exactly match my find :) Thanks

HazAT · 2019-04-02T07:35:01Z

👍 Thanks for helping me debugging this.

HazAT · 2019-04-02T08:01:11Z

5.0.3 is released, please try it

ghostd · 2019-04-02T08:37:12Z

It works with the hub.run wrapper (worth to add it into the advanced usage documentation):

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

Thanks!

HazAT self-assigned this Apr 1, 2019

HazAT closed this as completed in 797f99c Apr 2, 2019

ghostd mentioned this issue Apr 30, 2019

GlobalHandler does not report a global error #2044

Closed

8 tasks

dorianrod mentioned this issue Dec 13, 2019

Breadcrumb : Sentry events create breadcrumb with parameter sentry = false #2357

Closed

onurtemizkan mentioned this issue May 5, 2021

Vue integration does not work when using Sentry Client directly #3381

Closed

6 tasks

Uh oh!

Some browser integrations does not respect the Integration#setupOnce signature #1969

Some browser integrations does not respect the Integration#setupOnce signature #1969

Comments

ghostd commented Apr 1, 2019

Package + Version

Version:

Description

HazAT commented Apr 1, 2019

Uh oh!

ghostd commented Apr 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HazAT commented Apr 1, 2019

Uh oh!

ghostd commented Apr 1, 2019

Uh oh!

HazAT commented Apr 1, 2019

Uh oh!

ghostd commented Apr 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HazAT commented Apr 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghostd commented Apr 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghostd commented Apr 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HazAT commented Apr 1, 2019

Uh oh!

ghostd commented Apr 1, 2019

Uh oh!

HazAT commented Apr 1, 2019

Uh oh!

ghostd commented Apr 1, 2019

Uh oh!

ghostd commented Apr 2, 2019

Uh oh!

ghostd commented Apr 2, 2019

Uh oh!

HazAT commented Apr 2, 2019

Uh oh!

ghostd commented Apr 2, 2019

Uh oh!

HazAT commented Apr 2, 2019

Uh oh!

HazAT commented Apr 2, 2019

Uh oh!

ghostd commented Apr 2, 2019

Uh oh!

ghostd commented Apr 1, 2019 •

edited

Loading

ghostd commented Apr 1, 2019 •

edited

Loading

HazAT commented Apr 1, 2019 •

edited

Loading

ghostd commented Apr 1, 2019 •

edited

Loading

ghostd commented Apr 1, 2019 •

edited

Loading