Report original error from addReactError instead of fake rendering error

[eloytoro]

Motivation

The ErrorBoundary component will capture whatever error occurs inside of it and send it to datadog, when this happens it renders the fallback component supplied to it. This works similar to a try/catch, where errors that occur within a component tree will not cause the entire application to crash and instead show a fallback.

As of today there’s a big problem with how ErrorBoundary reports these errors to datadog and how that plays into the Error Tracking product.

The original thrown error is not the one being reported, instead a different error is with a different stack trace (the one obtained from react’s error info) and a different error name (or @error.type in datadog). This messes with Error Tracking’s grouping.

This is the anatomy of a react error sent to Datadog today:

It would make more sense to report the original error so it has the original stack and name, drop the cause since it wouldn’t be needed anymore and add the the component’s stack as an additional component_stack property reported to datadog.

Changes

In order to achieve this it's necessary to expose the strategy object through a new hook in plugins named onRumStart. Now additional information can be added to the payload outside of the event's context

Only after that the strategy object is accessed by the react plugin and used in addReactError to send the component's stack

Testing

Unit tests were added

[amortemousque]

💬 suggestion: ‏What about also passing the strategy in onInit hook instead of publicApi. This would limit the number of concepts in the React Plugin. To be more explicit we could also rename it startRumStrategy

[BenoitZugmeyer]

Passing the strategy in onInit is not that useful because it would be the preStartStrategy and there is a good chance that it won't be used right after the hook is called. I think it would make things more complex/confusing for little benefit.

[eloytoro]

in addition to what benoit said, the preStartStrategy buffer has already been flushed by the time onInit is called, which means that any usage of it would be completely ignored

[BenoitZugmeyer]

@eloytoro and I briefly discussed exposing the lifeCycle instead of the strategy, which might make things more flexible, but for now strategy seems simpler/enough. We can revisit later.

[eloytoro]

/merge

[dd-devflow]

Devflow running: /merge

View all feedbacks in Devflow UI.

---

2025-01-20 16:36:07 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

---

2025-01-20 16:55:54 UTC ⚠️ MergeQueue: This merge request was unqueued

This pull request was closed.

[eloytoro]

Closing this in favour of #3293 because the CI does not like PRs across forks