Skip to content

Feature: Add threads for Android platform exceptions #853

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

Merged
merged 12 commits into from
May 10, 2022

Conversation

ueman
Copy link
Collaborator

@ueman ueman commented May 2, 2022

📜 Description

Adds thread for android platform exception.

Example event: https://sentry.io/organizations/sentry-sdks/issues/3228921456/events/749fddaffc164a34a6efd82aba0f7f80/?project=5428562

💡 Motivation and Context

Fixes #841

💚 How did you test it?

  • New tests

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

@codecov-commenter
Copy link

codecov-commenter commented May 2, 2022

Codecov Report

Merging #853 (321e12d) into main (dcf755c) will increase coverage by 0.02%.
The diff coverage is 96.42%.

@@            Coverage Diff             @@
##             main     #853      +/-   ##
==========================================
+ Coverage   90.30%   90.33%   +0.02%     
==========================================
  Files         119      119              
  Lines        3764     3785      +21     
==========================================
+ Hits         3399     3419      +20     
- Misses        365      366       +1     
Impacted Files Coverage Δ
...or/android_platform_exception_event_processor.dart 96.51% <96.42%> (-0.42%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dcf755c...321e12d. Read the comment docs.

Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about the thread naming conventions, @marandaneto probably have better understanding of this.

The rest LGTM.

@ueman
Copy link
Collaborator Author

ueman commented May 3, 2022

After thinking a little bit more about it: I think it makes the most sense if the Android thread is marked as current = false, crashed = true and the Dart isolate is marked as current = true, crashed = false. What do you think?

@ueman ueman requested a review from marandaneto May 10, 2022 06:55
Co-authored-by: Manoel Aranda Neto <[email protected]>
@marandaneto marandaneto enabled auto-merge (squash) May 10, 2022 07:50
@marandaneto marandaneto merged commit f13b57b into getsentry:main May 10, 2022
@ueman ueman deleted the feature/android-thread branch May 10, 2022 07:52
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.

Threads are not correctly attached to events
4 participants