feat: Use contexts to handle ExtraErrorData

[CPatchane]

Hi here 👋

We used to have a custom event processor to add extra data to our events and we saw that there is now an integration to handle that: ExtraErrorData which is cool to remove our custom processor :)

The thing is that (I don't know if it was wanted), this integration extracts all information into an object using the event name like:

...
extra: {
  MyErrorName: {
    extra1: "info1",
    extra2: "info2",
  }
}
...

UI Preview:

This object is less well formatted on the Sentry interface compared to having all information at the extra object root like:

...
extra: {
  extra1: "info1",
  extra2: "info2",
}
...

UI Preview:

So this PR adds a new option toRoot (I am clearly opened if you have a better option name, I was not inspired), disabled by default, to allow putting all extra information to the event object root and overwriting if there were already existing extra information the same attributes.

[kamilogorek]

Hey! Looks really nice! I think it may be a useful addition :)
The only thing I'd ask for is, can you change /** JSDoc */ clause to correct one now as we have more than one possible config option? depth was rather self-explanatory, but toRoot may not be so much, especially overriding behavior.

[kamilogorek]

TODO: Update docs https://github.com/getsentry/sentry-docs/blob/master/src/collections/_documentation/platforms/javascript/index.md

[kamilogorek]

Possible names: merge, mergeToParent, mergeAttributes, toParent, raw, flat, mergeFlat, hard to choose 😅

[HazAT]

We slowly want to discourage the usage of extra.
WDYT @kamilogorek if we create a new context containing all the data of the error object?
like:

contexts: {
  error: {
  ....
  }
}

[kamilogorek]

@HazAT that'd work as well!

[CPatchane]

@kamilogorek with this new context usage then is this PR still relevant? Do you plan to use this new context soon in a next version?

[kamilogorek]

@CPatchane it's already available in the Sentry and in the SDK. We can change this PR to reflect it.

What has to be done is:
_extractErrorData should always return extraErrorInfo directly (just as you'd apply toRoot to it)

• toRoot option becomes then obsolete, as it'll always create a new context, so can be safely removed
• enhanceEventWithErrorData should be modified to return a new context instead. So instead of:

return {
  ...event,
  extra,
};

it should be

return {
  ...event,
  contexts,
};

where contexts is:

contexts = {
  ...event.contexts,
  ...normalizedErrorData,
};

[kamilogorek]

@CPatchane context still has to have a name (my bad by explaining it incorrectly, sorry)

{
  contexts: {
    error: {
      // extra data goes here
    }
  }
}

The name will be used to well "name" the UI block as shown in my screenshot above.

[CPatchane]

Ok I was wondering myself about this part also, I change that 👍

[CPatchane]

Is this test still relevant?

[CPatchane]

🤔 The tests all pass green in my local environment, do you see what's wrong?

[kamilogorek]

They work just fine locally for me as well.

Can you try to add this snippet to .travis.yml and push it to the PR?

addons:
  chrome: stable

Current version Travis use by default is 15 versions old.

[kamilogorek]

@CPatchane actually nevermind, just update a whole ubuntu distro :)

Change dist: trusty to dist: bionic in .travis.yml and push the commit.

[kamilogorek]

Fixed :) I wonder what happened that it broke all of a sudden.

Code looks good to me. WDYT @HazAT?

[CPatchane]

Any news @kamilogorek / @HazAT ?

[HazAT]

Thx 🙏

[kamilogorek]

Thanks for the ping :) Will make a release at the end of the week. Cheers!