captbaritone / raven-for-redux

A Raven middleware for Redux
295 stars 25 forks source link

Creating duplicate Raven errors. #12

Closed AndrewSouthpaw closed 7 years ago

AndrewSouthpaw commented 7 years ago

First off, thanks for putting this together! It's a huge improvement on redux-raven-middleware by being able to pass in an initialized Raven object.

I seem to be receiving duplicate errors in Sentry when throwing an error inside reducer code. One comes from Object.ReactErrorUtils.invokeGuardedCallback, and then another gets sent as well.

I'm guessing it's because we already have an initialized window.Raven object and it's also listening for an error, and raven-for-redux ends up rethrowing the error after capturing the exception with the Raven object?

Have you come across this issue before?

AndrewSouthpaw commented 7 years ago

... looks like when I copy your code from latest master, everything works fine. The npm package I was using did not include this commit which plausibly fixes the problem. Could you bump the version?

captbaritone commented 7 years ago

Thanks for reporting this. I'll take a look at cutting a new release and writing a regression test later tonight.

captbaritone commented 7 years ago

Interesting. I don't see a plausible reason for Raven to report an error twice, much less for my that commit to make any difference. Perhaps it has something to do with the way React tries to handle errors?

Either way, I've cut a new release (0.7.1) since it seems to help, but I'd still like to understand what's going os so that I can ensure we don't regress on this.

I seem to be receiving duplicate errors in Sentry when throwing an error inside reducer code. One comes from Object.ReactErrorUtils.invokeGuardedCallback, and then another gets sent as well.

Are you saying that they have slightly different stack traces? If so, would it be possible to share an example of the two stack traces?

Any chance of getting a reproducible example? If not, at the very least it would be helpful to know where exactly in React the error was triggered.

Also, if you have any other plugins/middlewares installed, that would be good info.

Thanks again for reporting this, and for any help you can offer chasing it down.

captbaritone commented 7 years ago

I'm going to close this issue. We can reopen if we get more info.

AndrewSouthpaw commented 7 years ago

I'll work on it this week and get back to you. :)

AndrewSouthpaw commented 7 years ago

Hey, sorry it took a while to get around to this one.

Here's the basics in a nutshell: I was throwing the error inside a reducer, a simple throw 'foo'. Then I would trigger that reducer action through a synchronous Redux action creator, and it would produce the two error reports. The action would be created by clicking a React button, which had something like onClick={() => dispatch(actions.blowUp())}.

As far as middlewares, nothing special: thunk, logger, and this middleware.

It's no longer showing two errors in Sentry, but it is sending two error XHR requests. I'm assuming Sentry is now smart enough to de-dupe them.

I could set up an endpoint for you to hit, trigger the error, and see the two requests going through, but I don't see much benefit to you and it would be a bit of effort to put together. :-/

We are calling Raven.showReportDialog to take advantage of their User Feedback feature, though I don't imagine that's related. Here's that code, in the Raven config:

    dataCallback: function(data) {
      /**
       * We have to wrap in a timeout because initially `data` doesn't have a Sentry event_id -- apparently that gets
       * after this callback. So we can be sneaky here and show the report dialog on the next tick.
       */
      setTimeout(function() {
        Raven.showReportDialog({
          eventId: data.event_id,
          dsn: 'https://2ba4d6f5015040cfb99bc2142c965b60@sentry.io/91101'
        });
      }, 0)
    }
captbaritone commented 7 years ago

Thanks for collecting those details. I'll try to create a similar example and see if I can reproduce.

captbaritone commented 7 years ago

Just for clarification, are you still seeing two XHR requests, even with raven-for-redux@0.7.1 ?

AndrewSouthpaw commented 7 years ago

Nope, I only see one XHR request for 0.7.1.

captbaritone commented 7 years ago

I've added a manual test here: https://github.com/captbaritone/raven-for-redux/blob/master/example/index.js#L46-L56 and tried that with 0.7.0 and I'm only seeing one XHR request. Any ideas of things to try?

AndrewSouthpaw commented 7 years ago

Only other idea is to try adding the Raven.showReportDialog in the dataCallback as I indicated above. I tried doing that on my own in your repo to help out, but I couldn't figure out how you got your tests running. :-/