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

enha: end session when user kills the App. #423

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

enha: end session when user kills the App. #423

wants to merge 3 commits into from

Conversation

marandaneto
Copy link
Contributor

@marandaneto marandaneto commented May 19, 2020

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

enha: end session when the user kills the App.

💡 Motivation and Context

often we end the session only on restart due to the user killing the App.
I suggest having a Service that monitors onTaskRemoved and ends a session that flushes to the disk (with correct timestamp).
onTaskRemoved is a best-effort, ending on restart is still needed if something else happened, eg onTaskRemoved is not called

💚 How did you test it?

running it on API 16, 26 and 29

📝 Checklist

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

🔮 Next steps

@marandaneto marandaneto marked this pull request as draft May 19, 2020 10:09
@marandaneto
Copy link
Contributor Author

@philipphofmann are you aware of any side effects of doing it or even different behaviors that won't work?
I know that a few brands/models don't call onTaskRemoved but it's not the case for the majority and it's a best-effort solution.

@codecov-commenter
Copy link

codecov-commenter commented May 19, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@fb3680e). Click here to learn what that means.
The diff coverage is 37.50%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #423   +/-   ##
=========================================
  Coverage          ?   60.15%           
  Complexity        ?      810           
=========================================
  Files             ?       92           
  Lines             ?     3752           
  Branches          ?      361           
=========================================
  Hits              ?     2257           
  Misses            ?     1340           
  Partials          ?      155           
Impacted Files Coverage Δ Complexity Δ
...-core/src/main/java/io/sentry/core/HubAdapter.java 4.25% <0.00%> (ø) 2.00 <0.00> (?)
...try-core/src/main/java/io/sentry/core/NoOpHub.java 57.14% <0.00%> (ø) 16.00 <0.00> (?)
...java/io/sentry/core/transport/AsyncConnection.java 69.84% <0.00%> (ø) 19.00 <0.00> (?)
sentry-core/src/main/java/io/sentry/core/Hub.java 65.37% <100.00%> (ø) 60.00 <0.00> (?)
sentry-core/src/main/java/io/sentry/core/IHub.java 85.71% <100.00%> (ø) 7.00 <1.00> (?)
...main/java/io/sentry/core/transport/Connection.java 100.00% <0.00%> (ø) 2.00% <0.00%> (?%)
...c/main/java/io/sentry/core/protocol/DebugMeta.java 0.00% <0.00%> (ø) 0.00% <0.00%> (?%)
...main/java/io/sentry/core/hints/SessionEndHint.java 100.00% <0.00%> (ø) 1.00% <0.00%> (?%)
...e/src/main/java/io/sentry/core/SentryEnvelope.java 100.00% <0.00%> (ø) 6.00% <0.00%> (?%)
... and 87 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 fb3680e...8fa9236. Read the comment docs.

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.

2 participants