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

Fix: don't crash app when applicationContext is not available #217

Merged
merged 3 commits into from
May 7, 2024

Conversation

buenaflor
Copy link
Contributor

@buenaflor buenaflor commented May 3, 2024

📜 Description

lateinit crashes the application if not set for example when using the Phoenix library

When calling ProcessPhoenix.triggerRebirth(context) the app is actually restarted twice and only one the second time the content providers are loaded.

💡 Motivation and Context

Fixes #212

💚 How did you test it?

Manual, SentryContextProviderTest

📝 Checklist

You have to check all boxes before merging:

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.

🔮 Next steps

@buenaflor buenaflor requested a review from romtsn as a code owner May 3, 2024 12:51
@buenaflor buenaflor changed the title Fix: don't crash app then applicationContext is not available Fix: don't crash app when applicationContext is not available May 3, 2024

val context = applicationContext ?: run {
// TODO: add logging later
return
Copy link
Member

Choose a reason for hiding this comment

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

tbh, I am a bit hesitant with this approach, because if someone removes our SentryContextProvider for whatever reason (as mentioned here for example), the SDK init will silently fail and the users wont be aware of that. Perhaps there's a better way to handle ProcessPhoenix specifically?

Copy link
Member

Choose a reason for hiding this comment

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

is the problem that ContentProviders are not loaded at all, or that context or context.applicationContext is null?

Copy link
Contributor Author

@buenaflor buenaflor May 6, 2024

Choose a reason for hiding this comment

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

I've set up some debug logs and the application init is triggered twice and during the the content provider is not loaded before so applicationContext crashes due to lateinit but on the second load it works fine because the content provider has already set the context:

Rebirth triggered now (click button that runs rebirth)
Try init sentry android: app context null (app tries to restart process, content provider is not triggered before)
Content provider loaded (now content provider is triggered)
Try init sentry android: app context sample.kmp.app.android.SentryApplication@d0c4ab0 (now we have access to app context)

Copy link
Member

Choose a reason for hiding this comment

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

ok, I see, I guess there's no many ways around it anyway. After we implement logging that should help also 👍

@buenaflor buenaflor merged commit 9c79652 into main May 7, 2024
12 checks passed
@buenaflor buenaflor deleted the fix/context-crash branch May 7, 2024 11:37
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.

Problem when used together with Phoenix library
2 participants