-
Notifications
You must be signed in to change notification settings - Fork 251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PLAT-5002] Include native stacktraces JS errors caused by native promise rejections #1061
Conversation
[PLAT-5001] Include native promise rejection stack traces in Android notifier
[PLAT-5000] Include native promise rejection stack traces in Cocoa notifier
dep: update RN android dependency
Add example app for React Native 0.60.0
Bump RN bugsnag-cocoa version to next
Add E2E scenario to verify RN native stack parsing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without looking through the whole changeset in too much detail, I'm content that we have paid adequate attention to testing on this piece of work.
I've tested this in debug/prod builds on both platforms, using the rn0.63 example in this repo. All working as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed primarily from the point of view that the right tests have been added and are being executed in CI. Approved on the basis that changeset has already been reviewed by PRs into the integration branch.
When a native module method is called from JS and the native promise is rejected, the JS layer receives the native stacktrace.
This PR brings together the native and JS changes required to include this stacktrace in the event as an additional entry in the errors array.
Note: landing these changes but with the JS part commented out. Awaiting pipeline update before this feature is enabled.