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

Stop/resume client on fatal crash #10

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

cristipufu
Copy link
Member

Ref: #9

@TTalpey
Copy link
Contributor

TTalpey commented Sep 12, 2024

I like some things, but not sure about others.

I don't think it's a good idea to add the start/stop/running methods to the public API. I don't see apps actually wanting to control those, and it makes the interaction with the crashreporter more complicated. It's true that AppCenter.Analytics had some support for this, but it was a persistent setting, more oriented toward opt-out than actual API control. I'd much rather that part stayed internal for the Aptabase client, or at least, more fleshed out before committing it to the API.

That said, it's maybe good to expose the new TrackEventAsync explicitly like you've done. It allows a bit more visibility for apps that have important events to ensure the best-effort queuing was safe. Still, AppCenter.Analytics didn't seem to need it, that was handled internally to Analytics.TrackEvent.

Bottom line, I'd prefer to keep things slim and simple and not put too much machinery around this, especially in the fatal-exception path. The SDK can only do so much before the OS tears the app down, after all.

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.

2 participants