WalletConnect / WalletConnectKotlinV2

WalletConnect Kotlin SDK v2
Apache License 2.0
224 stars 79 forks source link

Connection state change is always reported as false on some devices #556

Closed kamilargent closed 1 year ago

kamilargent commented 1 year ago

Describe the bug On some devices, the connection state is constantly reported as false - even though the user has an internet connection.

SDK Version

To Reproduce Steps to reproduce the behavior:

  1. Setup a device with at least 2 active networks e.g. Bluetooth and LTE
  2. Try to initialize the SDK

Expected behavior A user is able to interact with SDK when in reality he can't do anything

Device (please complete the following information):

Additional context As a result in the logs we can constantly see onConnectionStateChange(ConnectionState(isAvailable=false))

The logic in ConnectivityState to check the network connectivity is not covering all cases. It is common for the user to have multiple active networks, e.g. when plugging in Bluetooth headphones on Samsung devices. Sometimes it can even happen that there are multiple networks with internet capabilities. Because of that iterating over all networks is necessary to check internet capabilities.

The isConnected could be modified to sth like this:

    private fun isConnected(): Boolean = connectivityManager.run {
        allNetworks.any { network ->
            getNetworkCapabilities(network)?.run {
                hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) && hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED)
            } ?: false
        }
    }

This is just an example: the allNetworks is deprecated since API 31 since it's not efficient - it can be achieved in multiple ways using data reported by network callback.

PS. The NetworkRequest can be improved by listening to only networks that have internet capabilities - it should have better performance.

jakubuid commented 1 year ago

Hey, @kamilargent thanks for reporting.

I did some digging and I was indeed at some point able to reproduce your issue on the Samsung device, however, my conclusions are slightly different though.

On Samsung devices whenever you enable the Bluetooth connection, the Samsung's Bluetooth features dialog pops up, which causes the current application to be backgrounded, which is the reason of receiving the onConnectionStateChange(ConnectionState(isAvailable=false)) because SDK closes the wss connection in AUTOMATIC when that happens.

However, every time when I closed the Samsung's Bluetooth dialog the wss connection where opened again.

For more context, you can have a look on the video included below.

It'd be great if you could attached a video from your side where I can see the app and logs from logcat at the same time.

https://user-images.githubusercontent.com/23366251/211790427-dc6619e5-755a-4c13-957d-4ea8615127c2.mp4

kamilargent commented 1 year ago

That is not the use case I am describing. I am aware of the WebSocket closing when the app is in the background (which is what happens when the system activity is shown).

When I plug in my headphones first and then enable LTE/Wi-Fi, the headphones are treated as an activeNetwork on my Samsung - not the LTE/Wi-Fi. The app logs just constantly spew out onConnectionStateChange(ConnectionState(isAvailable=false)) so it won't be of much help.

jakubuid commented 1 year ago

Thanks for the better reproduction steps. However, using the same device and replicating your steps I was not able to reproduce it. What we can do though, I can do code changes based on your suggestions and send you a SNAPSHOT build so that you can test on your side if it resolves your issue.

kamilargent commented 1 year ago

Sounds good. You can also mark me for the PR review - I was fixing similar issue in the past so I should be able to provide some feedback before manually testing. Thanks!