Make use of Internet connection checker more configurable

[cpswan]

Is your feature request related to a problem? Please describe.

We presently incorporate the Internet connection checker package, which by default makes TCP connections to popular DNS servers such as 1.1.1.1 and 8.8.8.8

This may cause alarm to some customers, especially if they're configuring different DNS servers (and it may seem to them that we've hard coded DNS into our binary).

Describe the solution you'd like

Use the RV server(s) as the default connection check IP(s)

Make the use of connection check selectable (at install time) via a flag

Describe alternatives you've considered

Allow users to define their own check points

[gkc]

Sadly it is used in multiple places throughout the at_client package. I will look into the effort required to configure it and/or remove the need for it

[gkc]

Did some analysis.

Direct usage of InternetConnectionChecker

In our core packages, we use the InternetConnectionChecker directly only in the at_client package, in the following places

• ConnectivityListener in connectivity_listener.dart : a simple wrapper around InternetConnectionChecker
  • ConnectivityListener is exported by the at_client package
  • ConnectivityListener provides a single subscribe method which listens to InternetConnectionChecker().onStatusChange and provides a stream of booleans
• NetworkUtil in network_util.dart : class with single method isNetworkAvailable() which uses InternetConnectionChecker
  • network_util.dart is not exported by the at_client package, and is only used internally within the at_client package
• RemoteSecondary in remote_secondary.dart : isAvailable() function calls InternetConnectionChecker().isHostReachable on the atServer's host and port
  • This is fine
  • Note: this appears to be dead code, not used anywhere in any packages except in a monitor unit test via a mock object, and should be marked deprecated

Proposals

1) Make at_client's usage of InternetConnectionChecker configurable

• Provide a way in at_client package to say what addresses to use in at_client's wrapper classes (ConnectivityListener and NetworkUtil)
  • and, accordingly, modify how the at_client wrappers use InternetConnectionChecker (which allows us to provide addresses to use rather than using InternetConnectionChecker's defaults

2) update sshnpd along the lines of @cpswan's proposal above

• use some suitable host:port address(es) for the connection check. Perhaps root.atsign.org:64?
• sshnpd uses ConnectivityListener directly, but only to print a message, no code logic

    ConnectivityListener().subscribe().listen((isConnected) {
      if (isConnected) {
        logger.warning('connection available');
      } else {
        logger.warning('connection lost');
      }
    });

• Note that we can't, from sshnpd, stop the indirect usages of the InternetConnectionChecker which are happening within the at_client package itself. But more on that in the next section
• EDIT, Aug 1 cease using ConnectivityListener. Instead, send a periodic heartbeat over the daemon's connection to its atServer.

3) Eliminate unnecessary usages of ConnectivityListener and NetworkUtil throughout the at_client package

TL;DR : all of the usages are unnecessary, and NetworkUtil can be removed entirely as it is not exported

• ConnectivityListener usages
  • Only used within NotificationServiceImpl to call _startMonitor() upon an isConnected event
    • Proposal Since the Monitor now has its own heartbeat mechanism, this usage can be removed
• NetworkUtil usages
  • Used by MonitorConnectivityChecker.checkConnectivity
    • only used by the Monitor's start method
    • Proposal Remove this usage, as it is definitely unnecessary because the very next thing the start method does is try to create a connection to the atServer
  • Used by SyncServiceImpl.processSyncRequests to decide whether or not to start a sync run (call syncInternal method)
    • this usage is unnecessary since sync runs (syncInternal) handle network failures properly
    • Proposal Remove this usage
  • Used by at_client_validation.validateAtKey as a guard on the check to see if a particular atSign exists

      // verifies if the sharedWith atSign exists.
      if (atKey.sharedWith != null && await NetworkUtil().isNetworkAvailable()) {
        await isAtSignExists(
            AtClientManager.getInstance().secondaryAddressFinder!,
            atKey.sharedWith!);
      }
    

    • Holy moly, this method is used a lot - happens in every single get request

        Future<AtValue> get(AtKey atKey,
            {bool isDedicated = false, GetRequestOptions? getRequestOptions}) async {
          Secondary? secondary;
          try {
            // validate the get request.
            await AtClientValidation().validateAtKey(atKey);
      

    • Good news is that it is only used by AtClientImpl.get and is pretty pointless in that context
    • Proposal Remove this usage

[gkc]

at_client pull request