captbaritone / raven-for-redux

A Raven middleware for Redux
295 stars 25 forks source link

React Native Sentry dosn't collect extradata #66

Closed Burgomehl closed 4 years ago

Burgomehl commented 6 years ago

I am building for android on windows. As mentioned in your readme i used createRavenMiddleware(Sentry) but it is not collecting extradata. If i only switch from Sentry to Raven it works, but i would like to use Sentry.

Package.json:

    "raven-for-redux": "^1.3.0",
    "raven-js": "^3.23.3",
    "react": "16.0.0-alpha.12",
    "react-native": "0.48.4",
    "react-native-sentry": "^0.35.3",

It seems like Raven.setDataCallback(function (data, original) { is never called.

captbaritone commented 6 years ago

Any thoughts @emmajam ?

emmajam commented 6 years ago

Oops oh dear!

Let me have a look... Can you send through a snippet of your createStore function, where you pass through createRavenMiddleware?

emmajam commented 6 years ago

So, double checked, and it's certainly working in my project, if you can give me some more info / code snippets. I'll try to reproduce your environment 😃@Burgomehl

captbaritone commented 6 years ago

Thanks for jumping on that @emmajam.

Can the React-native-sentry module run in a node environment? Might be good to add a test that uses it.

Burgomehl commented 6 years ago

That was fast ;-)

The interesting thing is, that it work (like expected) if I install Sentry in the index.js and Raven next to createRavenMiddelware and then pass Sentry.

createStore:

    // Load middleware
    let composedMiddleware = [
        thunk, 
        appEnhanceMiddleware,
        myAPImiddleware,
        webSocketMiddleware,
        middleware,
        sentryReducer,
    ];
emmajam commented 6 years ago

I'd like to apologise for being so eager to help 😂 😜

Okay progress!! However, in my React Native app I don't have Raven installed at all...

This is how I've got it setup in my app (hopefully it helps) @Burgomehl :

// index.js

import { configureStore } from './redux/store'
import MainApp from './containers/MainApp'

const { store, persistor } = configureStore()

const App = () => {
  return (
    <Provider store={store}>
      <PersistGate persistor={persistor}>
        <MainApp />
      </PersistGate>
    </Provider>
  )
}

Then in...

// redux/store.js

import { Sentry } from 'react-native-sentry'
import createRavenMiddleware from 'raven-for-redux'
...
...

export function setupSentry() { 
  Sentry.config(dsn).install()
} // I have this function in a `sentry-utils.js` file but I call it here before anything else runs in this file

....
export const configureStore = () => {
  const store = createStore(
    ...,
    applyMiddleware(
      ...,
      ...,
      createRavenMiddleware(Sentry, {
        stateTransformer,
        actionTransformer
      }),
      ...
    )
  )
  const persistor = persistStore(store)
  return { store, persistor }
}

hopefully this helps a little bit ... I'm pretty sure you have to call the Sentry.config(dsn).install() before you pass Sentry through to your createRavenMiddleware

Let me know how you go with this! 🙏

emmajam commented 6 years ago

@captbaritone yeah I'm pretty sure it will be able to run in a node env, I'll look into adding a test or two for it tonight after work 👍

scottwio commented 6 years ago

Just to confirm this seems to be an issue. I have the same problem as @Burgomehl installing how the documentation describes does not give the extra data (state, lastAction). The only way I could get this to work is to install Raven and pass that to createRavenMiddleware rather than Sentry. Passing Sentry gives me the breadcrumbs but not the extra data.

emmajam commented 6 years ago

Dang it, Sentry has the exact same APIs exposed, I'm not sure why it wouldn't be working..

@captbaritone I might create a sentry-for-redux repo based on this one (if thats okay) to handle the React Native side of things and test out if we can duplicate the functionality of raven-for-redux

@scottwio thanks for the feedback 👍

captbaritone commented 6 years ago

Emma,

Sounds good! Want to open a PR to remove the ~sentry~ React-Native documentation?

emmajam commented 6 years ago

Can do

captbaritone commented 6 years ago

Thanks!

emmajam commented 6 years ago

Sorry been a busy day - will sort this out tomorrow! 🤗

emmajam commented 6 years ago

7 days later...

https://github.com/captbaritone/raven-for-redux/pull/68

Created this also - https://github.com/emmajam/sentry-for-redux but only has a readme will push to it later this evening 👍

Burgomehl commented 6 years ago

the install() function is async maybe this is the problem?

emmajam commented 6 years ago

Hmm I don't think so, why do you think that would be causing issues? @Burgomehl

Burgomehl commented 6 years ago

Maybe some race conditions. Like in my example -> I did Sentry.install() and then Raven.install() and then it worked. But without Raven.install() it didn't worked. Why should the Raven.install() change the thing if I still do createRavenMiddleware(Sentry) ?

emmajam commented 6 years ago

So calling Sentry.install() triggers the raven install itself, return Sentry._ravenClient.install(), if you look through the react-native-sentry SDK.. https://github.com/getsentry/react-native-sentry/blob/master/lib/Sentry.js#L27

I completely agree with you. I'm confused as to why Sentry.install() alone isn't enough, your Raven.install() should technically have no impact as it should already be instantiated through the previous Sentry.install() ...

I'll have a play around after work tonight, need to put in some logs to decipher what's going on as off the top of my head it isn't overly clear 😱

emmajam commented 6 years ago

Lil' update have been doing some debugging - so we can't actually call the Sentry.setDataCallback() from our react native code - its triggered once after Sentry.install() but that's it, this is where I have been tricked! 😱- thinking it was my code calling it..

However, they have other methods that may be able to be use instead as a replacement. But can confirm the Sentry.captureBreadcrumb() is working as expected, will just need to implement a different implementation for RN for the callback method..

Will push hopefully tomorrow morning the new repo 🙆

mattryles commented 6 years ago

Hi @emmajam, Did you find a solution for this?

It's a shame about the setDataCallBack function, looking through the docs (https://docs.sentry.io/clients/react-native/config/) it doesn't seem the functionality is there. However there is setExtraContext, this seems to work as expected.

The problem is when do you call setExtraContext as I've not had much luck with the setShouldSendCallback function and not sure there is a better call site.

Matt

hirthbrian commented 5 years ago

I noticed that the setDataCallback is only overwritten when the Sentry.install() Promise is over. I had some trouble making this repo work for my React Native app so I made my own based on it. If anyone is interested I can rework some stuff and make a fork.

export const createSentryMiddleware = (Sentry) => {

  const sentryDsn = "";

  const breadcrumbDataFromAction = () => {};

  const breadcrumbMessageFromAction = action => action.type;

  const breadcrumbCategory = "redux-action";

  const actionTransformer = x => x;

  const stateTransformer = state => ({});

  const filterBreadcrumbActions = action => action.type;

  const getTags = state => ({});

  const getUserContext = state => ({});

  return (store) => {
    let lastAction;

    Sentry.config(sentryDsn, {
      // CONFIG HERE,
    }).install().then(() => {
      Sentry.setDataCallback((data, original) => {
        const state = store.getState();
        const newData = data;

        const reduxExtra = {
          lastAction: actionTransformer(lastAction),
          state: stateTransformer(state)
        };
        newData.extra = Object.assign(reduxExtra, newData.extra);
        newData.user = getUserContext(state);
        newData.tags = getTags(state);

        return original ? original(newData) : newData;
      })
    });

    return (next) => (action) => {
      if (filterBreadcrumbActions(action)) {
        Sentry.captureBreadcrumb({
          category: breadcrumbCategory,
          message: breadcrumbMessageFromAction(action),
          data: breadcrumbDataFromAction(action)
        });
      }

      lastAction = action;
      return next(action);
    };
  };
}
captbaritone commented 4 years ago

Check out https://github.com/vidit-sh/redux-sentry-middleware, I suspect it will work.