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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ pod("Sentry") {
}
```

### Fixes

- Don't crash app when `applicationContext` is not available ([#217](https://github.com/getsentry/sentry-kotlin-multiplatform/pull/217))

### Enhancements

- Make `setSentryUnhandledExceptionHook` public ([#208](https://github.com/getsentry/sentry-kotlin-multiplatform/pull/208))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,18 @@ import io.sentry.kotlin.multiplatform.extensions.toAndroidSentryOptionsCallback
internal actual fun initSentry(configuration: OptionsConfiguration) {
val options = SentryOptions()
configuration.invoke(options)
SentryAndroid.init(applicationContext, options.toAndroidSentryOptionsCallback())

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 👍

}

SentryAndroid.init(context) { sentryOptions ->
options.toAndroidSentryOptionsCallback().invoke(sentryOptions)
}
}

internal lateinit var applicationContext: Context
internal var applicationContext: Context? = null
private set

public actual typealias Context = Context
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.mock
import kotlin.test.assertNotNull

@RunWith(AndroidJUnit4::class)
class SentryContextProviderTest : BaseSentryTest() {
Expand All @@ -22,8 +23,8 @@ class SentryContextProviderTest : BaseSentryTest() {
class SentryContextOnCreateTest : BaseSentryTest() {
@Test
fun `onCreate initializes applicationContext`() {
// Simple call to the lateinit applicationContext to make sure it's initialized
applicationContext
// Simple call to the applicationContext to make sure it's initialized
assertNotNull(applicationContext)
}
}

Expand Down
Loading