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

Remove auto flush #47

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Remove auto flush #47

wants to merge 3 commits into from

Conversation

dhindrik
Copy link
Owner

No description provided.

@IeuanWalker
Copy link
Contributor

My only concern with this is missing telemetry when the app crashes.

From my understanding Flush is what sends the telemetry to application insights.

Does OnSleep always gets called even if the app crashes? If not it would be best to add a call to Flush in the HandleCrash method.

Is there any other situations where the app can be killed and OnSleep not triggered? (not used/ looked into the event much before)

Personally i havnt seen any performance issues using this, so not sure if this is preemptive optimising with little evidence, that could potentially cause issues

@dhindrik
Copy link
Owner Author

That is the reason I added flush everywhere from the beginning.

Sleep will not be called when the app crash, at lest you cannot trust it to be called.

I have tried to add Flush in the HandleCrash method, and it seems to work. But I will do more testing before (if I do) I merge this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants