search

captbaritone / raven-for-redux

A Raven middleware for Redux
295 stars 25 forks source link

Add an action's 'payload' property to breadcrumb data if it exists #3

Closed joshfarrant closed 7 years ago

joshfarrant commented 7 years ago

Just a thought that could potentially be a useful addition - It's quite likely that a lot of people using Redux will also be using the flux-standard-actions standard for structuring their actions.

It could be useful to include the action's payload property (if it exists) in the data property of Raven's breadcrumb to help add a bit more context to actions in the breadcrumb in Sentry.

This could also be extended to including all of an action's properties (apart from type) in the breadcrumb's data, which would then mean that all actions (in whatever format) could be passed through to Sentry and examined.

Interested to hear your thoughts on this!

joshfarrant commented 7 years ago

Only potential issue with this, which I hadn't previously considered, is that you could end up pushing sensitive data, eg.

{
  type: 'USER_LOGIN',
  payload: {
    username: 'joebloggs',
    password: '123'
  }
}

Definitely don't want that in Sentry! In hindsight, this could be a good enough reason in itself to not implement this, assuming there's no way to filter this out.

captbaritone commented 7 years ago

This is a great idea, unfortunately there are some "gottchas". The biggest limitation is that Sentry won't accept reports with too much data. If you attach every action (some of which might contain entire API responses) you can quickly end up in a place where exceptions are not actually stored, since Sentry returns 400 when Raven tries to report it.

Also, the data attribute must be a flat object. You can get around this with something like JSON.stringify() but it's kinda a hack.

If this is still something you want to pursue, I've added a new option breadcrumbDataFromAction (see the README for docs) which should let you achieve your goal. Basically it lets you define a function which generates the breadcrumb data object.

There was a similar conversation over here: https://github.com/ngokevin/redux-raven-middleware/issues/24

joshfarrant commented 7 years ago

Thanks for looking into this!