Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

fix: before raising ANR event, we check ProcessErrorStateInfo if available #412

Merged
merged 6 commits into from
May 16, 2020
Merged

fix: before raising ANR event, we check ProcessErrorStateInfo if available #412

merged 6 commits into from
May 16, 2020

Conversation

marandaneto
Copy link
Contributor

@marandaneto marandaneto commented May 15, 2020

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

fix: before raising ANR event, we check ProcessErrorStateInfo if available

💡 Motivation and Context

ANR events are too noisy
#406

💚 How did you test it?

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing

🔮 Next steps

@marandaneto marandaneto marked this pull request as ready for review May 15, 2020 13:15
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

Minor notes but this is huge improvement!

}
boolean isAnr = false;
for (ActivityManager.ProcessErrorStateInfo item : processesInErrorState) {
if (item.condition == NOT_RESPONDING) {
Copy link
Member

Choose a reason for hiding this comment

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

So this is a best effort since it could give false positive? Meaning there's some process in ANR state but this isn't the current one? Either way would improve a lot already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's only the processes of this app, so that's why it's a list. an App can have multiple processes but its not common at all.

@codecov-io
Copy link

codecov-io commented May 16, 2020

Codecov Report

Merging #412 into master will increase coverage by 0.45%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #412      +/-   ##
============================================
+ Coverage     59.72%   60.18%   +0.45%     
- Complexity      790      809      +19     
============================================
  Files            89       92       +3     
  Lines          3620     3747     +127     
  Branches        344      360      +16     
============================================
+ Hits           2162     2255      +93     
- Misses         1311     1338      +27     
- Partials        147      154       +7     
Impacted Files Coverage Δ Complexity Δ
.../core/SendCachedEventFireAndForgetIntegration.java 72.41% <0.00%> (-9.41%) 4.00% <0.00%> (ø%)
.../src/main/java/io/sentry/core/SendCachedEvent.java 54.90% <0.00%> (-4.19%) 8.00% <0.00%> (ø%)
...c/main/java/io/sentry/core/cache/SessionCache.java 53.63% <0.00%> (-1.93%) 23.00% <0.00%> (ø%)
...e/src/main/java/io/sentry/core/EnvelopeSender.java 67.01% <0.00%> (-0.46%) 16.00% <0.00%> (+1.00%) ⬇️
...main/java/io/sentry/core/HttpTransportFactory.java 100.00% <0.00%> (ø) 1.00% <0.00%> (ø%)
...a/io/sentry/core/SendFireAndForgetEventSender.java 76.19% <0.00%> (ø) 3.00% <0.00%> (?%)
...ain/java/io/sentry/core/SentryExecutorService.java 100.00% <0.00%> (ø) 6.00% <0.00%> (?%)
...o/sentry/core/SendFireAndForgetEnvelopeSender.java 78.26% <0.00%> (ø) 3.00% <0.00%> (?%)
sentry-core/src/main/java/io/sentry/core/Hub.java 65.37% <0.00%> (+0.10%) 60.00% <0.00%> (ø%)
...java/io/sentry/core/transport/AsyncConnection.java 70.91% <0.00%> (+0.13%) 19.00% <0.00%> (+2.00%)
... and 3 more

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 07d9eaa...da0067b. Read the comment docs.

@marandaneto marandaneto merged commit ba434de into getsentry:master May 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants