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

SDK init after app start should not require SentryHybridSdkDidBecomeActive #4575

Open
vaind opened this issue Nov 26, 2024 · 5 comments
Open

Comments

@vaind
Copy link
Collaborator

vaind commented Nov 26, 2024

Description

Problem description

There's a notification exposed for hybrid SDKs which they're supposed to call when an app becomes active (SentryHybridSdkDidBecomeActive). This is because if the app was already active at the time the normal DidBecomeActive notification is registered, it would never get triggered.

The problem with this solution is that it shifts the issues to all individual consuming SDKs (which is bound to fail one way or another, not even taking into account that they need to figure out they're supposed to call this) instead of implementing it here.

Additionally, I wonder how this isn't a problem for anyone else initializing the SDK later while the app is already running? I.e. not talking about hybrid SDKs but any app using the Cocoa SDK.

Proposal:

How about we change the logic and automatically do what SentryHybridSdkDidBecomeActive does, i.e. start session if session tracking is enabled. Track OOM, etc.
We can first check that the app is in the foreground.

Context

Found this out when figuring out getsentry/sentry-dart#2412 (comment) with a fix in getsentry/sentry-dart#2452

@philipphofmann
Copy link
Member

I can't recall exactly why we came up with this solution. It seemed like a good choice when we implemented it in #913 to fix a related Dart issue getsentry/sentry-dart#274. While your proposal makes sense, why should we change it now? What problem do we solve? We already have it implemented like that in all SDKs; changing it is extra work.

@vaind
Copy link
Collaborator Author

vaind commented Nov 27, 2024

We already have it implemented like that in all SDKs; changing it is extra work.

  1. Flutter hot-restart didn't work until my recent fix to the implementation, basically doing what I suggest doing in the cocoa-SDK
  2. RN has the same implementation like Flutter did before, IDK like reloads/restarts are handled there, maybe it works.
  3. Neither .NET MAUI nor Unity use SentryHybridSdkDidBecomeActive at all. I haven't verified the behavior there but may have the same issue.
  4. Any application using sentry-cocoa directly that does manual init after it is already active (on screen) also has the same issue. I haven't verified that but pretty sure that would be the case, considering it's no different than Flutter initializing after the app is displayed.

Even the last one should be reason enough to fix this in the SDK & deprecate/ignore SentryHybridSdkDidBecomeActive

@vaind
Copy link
Collaborator Author

vaind commented Nov 27, 2024

@philipphofmann
Copy link
Member

Thanks for the detailed explanation. That makes sense. Do you have the bandwidth to open a PR for this?

@vaind
Copy link
Collaborator Author

vaind commented Nov 27, 2024

Not any time soon I'm afraid but I'll keep it in mind to check if it's still open for when I do.

@philipphofmann philipphofmann moved this from Needs Discussion to Backlog in Mobile & Cross Platform SDK Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

No branches or pull requests

2 participants