vestrel00 / contacts-android

Android Contacts API Library written in Kotlin with Java interoperability. No more ContentProviders and cursors. Say goodbye to ContactsContract. Build your own contacts app!
https://vestrel00.github.io/contacts-android/
Apache License 2.0
601 stars 36 forks source link

Permissions module extension `xxxWithPermission()` may crash app #143

Closed alorma closed 2 years ago

alorma commented 2 years ago

Code example:

When running an insert after, done after queryWithPermission() it crashes the app with the below stacktrace.

  suspend fun create(name: String, phone: String?, email: String?): Long? {
    val rawContact = NewRawContact().apply {
      setName { displayName = name }
      if (phone != null) {
        addPhone {
          number = phone
          type = PhoneEntity.Type.OTHER
        }
      }
      if (email != null) {
        addEmail {
          address = email
          type = EmailEntity.Type.OTHER
        }
      }
    }
    return contacts
      .insertWithPermission()
      .rawContacts(rawContact)
      .commitWithContext()
      .rawContactId(rawContact)
  }

Stacktrace:

    java.lang.RuntimeException: Failure delivering result ResultInfo{who=@android:requestPermissions:, request=42, result=-1, data=Intent { act=android.content.pm.action.REQUEST_PERMISSIONS (has extras) }} to activity {com.alorma.tempcontacts/com.karumi.dexter.DexterActivity}: android.view.WindowManager$BadTokenException: Unable to add window -- token null is not valid; is your activity running?
        at android.app.ActivityThread.deliverResults(ActivityThread.java:5360)
        at android.app.ActivityThread.handleSendResult(ActivityThread.java:5401)
        at android.app.servertransaction.ActivityResultItem.execute(ActivityResultItem.java:51)
        at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2267)
        at android.os.Handler.dispatchMessage(Handler.java:107)
        at android.os.Looper.loop(Looper.java:237)
        at android.app.ActivityThread.main(ActivityThread.java:8167)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:496)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1100)
     Caused by: android.view.WindowManager$BadTokenException: Unable to add window -- token null is not valid; is your activity running?
        at android.view.ViewRootImpl.setView(ViewRootImpl.java:1147)
        at android.view.WindowManagerGlobal.addView(WindowManagerGlobal.java:471)
        at android.view.WindowManagerImpl.addView(WindowManagerImpl.java:95)
alorma commented 2 years ago

It also happens when denying permission.

alorma commented 2 years ago

I run this library on app made with compose

alorma commented 2 years ago

Repo example: https://github.com/alorma/TemporalContacts

vestrel00 commented 2 years ago

I'll look into this possibly tomorrow. One thing I can say now is that this looks like it is caused by the permissions module code; queryWithPermission() and/or insertWithPermission(). I wrote the permissions extensions using Dexter; https://github.com/Karumi/Dexter, which might not fully work with coroutines and/or Jetpack compose.

So, what you can do if you don't want to wait for me to fix this issue is to handle permissions yourself without using the permissions extensions in this lib.

vestrel00 commented 2 years ago

Unrelated to the crash, I noticed that you can reduce the lines of code in your app.

So instead of,

val rawContact = NewRawContact().apply {
      setName { displayName = name }
      if (phone != null) {
        addPhone {
          number = phone
          type = PhoneEntity.Type.OTHER
        }
      }
      if (email != null) {
        addEmail {
          address = email
          type = EmailEntity.Type.OTHER
        }
      }
}

You can just do,

val rawContact = NewRawContact().apply {
      setName { displayName = name }
      addPhone {
        number = phone
        type = PhoneEntity.Type.OTHER
      }
      addEmail {
        address = email
        type = EmailEntity.Type.OTHER
      }
}

You don't need the null checks. All data entity properties (i.e. number, type, address) are nullable. If the important data (i.e. the phone number, email address) are null or blank, the insert APIs will ignore it and it will NOT be inserted into the database. You can read more about this in How do I learn more about "blank" data?.

vestrel00 commented 2 years ago

One more question. Do you have the developer option "Don't keep activities" enabled? I'm not saying that the crash should happen because of it but It will help me debug the issue 😄

alorma commented 2 years ago

Uhmmm, no, i had it disabled.

As well, my project is a single activity

alorma commented 2 years ago

I successfully created contact using allowBlanks(true) and requesting permissions READ, WRITE and GET_ACCOUNTS

vestrel00 commented 2 years ago

Cool! Two questions;

  1. Did you use insertWithPermission?
  2. Does it still work if you do NOT call allowBlanks(true)?
alorma commented 2 years ago

NO, I not used insertWithPermission(), as I managed. permissions on other way

alorma commented 2 years ago

It works without allowBlanks(true) yes

vestrel00 commented 2 years ago

Thanks! I'll try to reproduce this on your temporal contacts app tomorrow. Do you have a branch or commit that still uses insertWithPermission() that the happens in?

alorma commented 2 years ago

main branch commit: 5a7f9f0177572c4300c54e97a8a15b9eb76a1d34

vestrel00 commented 2 years ago

I haven't had the chance to investigate this yet as I have been focusing on other things and this is not happening in the sample app. It is also not part of the core module so the urgency is a little lower IMO. Most folks probably use different permissions libraries anyways. The extensions provided in the permissions module are optional. That does NOT mean that I don't care about this!! This is scheduled to be included in the v0.2.1 Release so I am planning to work on it soon-ish 😁

Anyways, I'll add a help wanted sign in case someone else wants to take a look at this sooner.

vestrel00 commented 2 years ago

Note to self. I was able to reproduce this same exact stack trace on the sample app by declining permissions on a Samsung Galaxy A71 running Android 11.

vestrel00 commented 2 years ago

Migrating away from dexter may or may not solve this issue. Regardless, the migration away from dexter should occur before attempting to fix this bug. The bug might just go away after the migration.

So, this should not be worked on until #101 is done.

vestrel00 commented 2 years ago

I verified that switching from Dexter to TedPermission does the trick! I can no longer reproduce the issue!

This will be resolved in scope of #101 😁

vestrel00 commented 2 years ago

No breaking changes for library users!

vestrel00 commented 2 years ago

This is included in 0.2.4!

CODES77 commented 1 year ago

CODES77