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

Enhance network connection status in ReachabilityMonitor of Datastore #2738

Open
1 task done
aladine opened this issue Mar 14, 2024 · 1 comment
Open
1 task done
Labels
datastore DataStore category/plugins improvement Any improvement that's not a bug and not requesting new functionality

Comments

@aladine
Copy link

aladine commented Mar 14, 2024

Before opening, please confirm:

Language and Async Model

Java, Kotlin, RxJava

Amplify Categories

DataStore

Gradle script dependencies

// Put output below this line
    api("com.amplifyframework:aws-api:2.14.11")
    api("com.amplifyframework:core:2.14.11")
    api("com.amplifyframework:aws-datastore:2.14.11")
    api("com.amplifyframework:rxbindings:2.14.11")

Environment information

# Put output below this line

8.3.0

Please include any relevant guides or documentation you're referencing

ReachabilityMonitor detect network connection but it doesn't check whether it is connect

Describe the feature request

The original commit dfb5b55#diff-8dd0015f3eddee483b38c9b008f9db975c9840c2709abbfca6a783f1b9baa8f4 adds ReachabilityMonitorImpl with a DefaultConnectivityProvider which indicates there is network available:


    override val hasActiveNetwork: Boolean
        get() = connectivityManager?.let { it.activeNetwork != null }
            ?: run {
                throw DataStoreException(
                    "ReachabilityMonitor has not been configured.",
                    "Call ReachabilityMonitor.configure() before calling ReachabilityMonitor.getObservable()"
                )
            }

However, even when network is active, it may not connect to internet. Since sync operation in Datastore depends on ReachabilityMonitor to provide network status, it could lead to too many sync attempts when internet is not available but there is still active network.

Please kindly let me know if ReachabilityMonitor is lacking or serves a distinct purpose in Datastore. Possible enhancement I could think of:

  • Instead of checking activeNetwork, connectivity provider could retrieve getNetworkCapabilities and check the network capability is valid.
  • Network callback should have an extra [onCapabilitiesChanged()
  • A more fine grained detail to support different Android SDK version for network checking.

Resources:

Initialization steps (if applicable)

No response

Code Snippet

// Put your code below this line.

amplifyconfiguration.json

No response

GraphQL Schema

// Put your schema below this line

Additional information and screenshots

No response

@github-actions github-actions bot added the pending-triage Issue is pending triage label Mar 14, 2024
@mattcreaser mattcreaser added improvement Any improvement that's not a bug and not requesting new functionality datastore DataStore category/plugins and removed pending-triage Issue is pending triage labels Mar 14, 2024
@mattcreaser
Copy link
Contributor

Solid suggestion @aladine. We'll look at getting this prioritized, but I also encourage you to give it a shot and open a PR yourself!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datastore DataStore category/plugins improvement Any improvement that's not a bug and not requesting new functionality
Projects
None yet
Development

No branches or pull requests

2 participants