Skip to content
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

Merged
merged 26 commits into from
Oct 1, 2020

Conversation

bengourley
Copy link
Contributor

@bengourley bengourley commented Sep 22, 2020

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.

@bugsnagbot
Copy link
Collaborator

bugsnagbot commented Sep 22, 2020

@bugsnag/browser bundle size diff

Minified Minfied + Gzipped
Before 39.62 kB 12.23 kB
After 39.62 kB 12.23 kB
± No change No change

Generated by 🚫 dangerJS against fc53d42

Copy link
Contributor

@twometresteve twometresteve left a 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.

@bengourley
Copy link
Contributor Author

I've tested this in debug/prod builds on both platforms, using the rn0.63 example in this repo. All working as expected.
However, I can't approve the PR since I originally raised it 🙃

Copy link
Contributor

@twometresteve twometresteve left a 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.

@bengourley bengourley merged commit f4e6876 into next Oct 1, 2020
@bengourley bengourley deleted the integration/rn-native-stack branch October 1, 2020 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants