Callback to set options depending on captured exception

[TimPetricola]

When throwing an error, it sometimes make sense to add some extra data directly to the Error object like so:

const error = new Error('foo')
error.extra = {foo: 'bar'}
throw error

Right now, if we want this extra sent alongside the error, we need to override Raven.captureException like so:

const originalCaptureException = Raven.captureException.bind(Raven)
Raven.captureException = (ex, options) => {
  options = Object.assign({}, options)
  options.extra = Object.assign({}, ex.extra, options.extra)
  originalCaptureException(ex, options)
}

With this PR, it allows us to replace this overriding with a simple config:

Raven.config(SENTRY_DSN, {
  optionsFromException(ex, options) {
    options = Object.assign({}, options)
    options.extra = Object.assign({}, ex.extra, options.extra)
    return options
  }
})

[TimPetricola]

Could be replaced by this.sinon.match.any if Sinon is updated to >= 2.0 (support added in sinonjs/sinon#1076)

[dcramer]

Would it be good enough if dataCallback (or similar) just passed along the error object, and you were able to then augment the Sentry payload from there?

[LewisJEllis]

2 things I can think of that we should consider this against:

1. We already do something like this completely automatically in raven-node: any additional properties of the error object are added to the extra data. Source here; this PR, is, of course, more flexible, but we're generally averse to introducing new callback hooks without good reason because we could easily end up with 10 of them which would be hard to keep straight.

2. I can't find the issue right now, but I'm almost certain that we've gotten a request in the past to provide the error object as an argument in dataCallback/shouldSendCallback, and I think @dcramer made a comment against it on the notion that error object should be long since parsed/forgotten by the time we get that far. I'll keep looking for this issue.

[dcramer]

I want the error object in a callback per Slack convo. It's non trivial to do today, but it's the right solution.

[TimPetricola]

I totally see your point in not having too many callbacks. Having access to the error object in dataCallback would certainly be enough and would be perfect to centralize everything in this callback. I was also under the impression that the error object should be parsed at this point though.
Also, at this point, we could be dealing with an error or a message, so the signature should be flexible enough to handle either of them.

[TimPetricola]

Actually I'm wondering if it would not make more sense to introduce the same behavior as in raven-node for consistency?

[TimPetricola]

Any update on what we could do on this one? :)

[kamilogorek]

@benvinegar @dcramer was there a consensus whether we should expose error object itself in dataCallback or not?

[dcramer]

@kamilogorek IMO we should if we can. It makes it possible to do a lot more behavior by inspecting the object that you otherwise can't reasonable do.

[benvinegar]

We absolutely should, the only reason I waffled on this was that the dataCallback signature is getting bloated and was trying to think of a clever way to represent it. Also, whatever you do, you have to remember about all the other callbacks: shouldSendCallback (sigh) and breadcrumbCallback (where we should show the original object being "breadcrumbed", if it exists e.g. XHRs).

[kamilogorek]

Effectively duplicate of #980