Feature: Check server health in the background #2666
Conversation
There was a problem hiding this comment.
Overall, a good start but some things that need to be fleshed out more.
I am internally (slowly) working on a persistent socket connection service, but within Kotlin, maybe you can wait for that to be finished so you get some inspiration? It'll be hardened as a persistent service, so start on boot, restart itself if it gets killed, and create a persistent notification that its active, all which we would want this service to do as well. Not sure when I'll be able to finish that, but hopefully in the next few weeks.
| @@ -151,6 +151,7 @@ dependencies { | |||
| implementation 'androidx.browser:browser:1.7.0' | |||
| implementation 'androidx.activity:activity-ktx:1.8.2' | |||
| implementation "androidx.work:work-runtime:2.9.0" | |||
| implementation "androidx.work:work-runtime-ktx:2.9.0" | |||
There was a problem hiding this comment.
I think we can remove the runtime impl above this, because -ktx just adds kotlin extensions, don't need the duplicate one
| companion object { | ||
| const val tag = "connection-error-notification" | ||
|
|
||
| private const val NOTIFICATION_ID = -2 |
| if (call.argument("operation") as? String == "create") { | ||
| createErrorNotification(context) | ||
| } else if (call.argument("operation") as? String == "clear") { | ||
| clearErrorNotification(context) | ||
| } | ||
|
|
||
| result.success(null) |
There was a problem hiding this comment.
Let's keep all the notification cancelling logic in DeleteNotificationHandler, and just pass ID -2 from Dart itself
| fun getBBServerUrl(context: Context): BBServerInfo { | ||
| val prefs = context.getSharedPreferences("FlutterSharedPreferences", 0) | ||
|
|
||
| return BBServerInfo( | ||
| prefs.getString("flutter.serverAddress", null), | ||
| prefs.getString("flutter.guidAuthKey", null) | ||
| ) | ||
| } |
There was a problem hiding this comment.
I would prefer that we force the worker to go through firebase to get the URL, because there is no guarantee the one in the preferences is most up to date. I do need to update the getServerUrl logic to handle setups that don't have firebase though, because it will just error currently. Overall just need to support firebase-less setups better
| if (Platform.isAndroid) | ||
| Obx(() => SettingsSwitch( | ||
| initialVal: ss.settings.backgroundServerPinging.value, | ||
| title: "Background Server Pinging", |
| import 'package:bluebubbles/services/services.dart'; | ||
| import 'package:get/get.dart'; | ||
|
|
||
| HealthService healthService = Get.isRegistered<HealthService>() ? Get.find<HealthService>() : Get.put(HealthService()); | ||
|
|
||
| class HealthService extends GetxService { | ||
| Future<void> setBackgroundPingingState(bool enabled) async { | ||
| if (enabled) { | ||
| await enableBackgroundPinging(); | ||
| } else { | ||
| await disableBackgroundPinging(); | ||
| } | ||
| } | ||
|
|
||
| Future<void> enableBackgroundPinging() async { | ||
| await mcs.invokeMethod("health-check-setup", {"enabled": true}); | ||
| } | ||
|
|
||
| Future<void> disableBackgroundPinging() async { | ||
| await mcs.invokeMethod("health-check-setup", {"enabled": false}); | ||
| } | ||
| } |
There was a problem hiding this comment.
Are we initializing this on app startup? We need to somehow make sure the ping service is started or running when the app starts (main.dart)
There also needs to be a mechanism to restart the service on boot or if it gets killed by the OS.
| @@ -11,6 +11,8 @@ import com.bluebubbles.messaging.services.firebase.ServerUrlRequestHandler | |||
| import io.flutter.plugin.common.MethodCall | |||
| import io.flutter.plugin.common.MethodChannel | |||
|
|
|||
| data class BBServerInfo(val url: String?, val guid: String?) | |||
There was a problem hiding this comment.
You'll also need to handle custom headers, if the user has set any, so this will get a bit more complex
|
|
||
| override suspend fun doWork(): Result { | ||
| return withContext(Dispatchers.IO) { | ||
| networkObserver.start() |
There was a problem hiding this comment.
Do we need a network observer at all if you are already checking in-line what the network state is when the worker runs?
| val work = PeriodicWorkRequestBuilder<HealthWorker>(REPEAT_MINUTES, TimeUnit.MINUTES) | ||
| .setConstraints(Constraints | ||
| .Builder() | ||
| .setRequiredNetworkType(NetworkType.CONNECTED) |
There was a problem hiding this comment.
Similar to above, should this not mean that the worker only runs if the phone is connected to network? So no need to even observe network state.
| "${URLEncoder.encode(k, "UTF-8")}=${URLEncoder.encode(parameters[k], "UTF-8")}" | ||
| } | ||
|
|
||
| val url = URL("${info.url}/api/v1/ping?$paramsStr") |
There was a problem hiding this comment.
It may be useful to only just ping the raw URL (landing page URL) so we don't have to deal with user password, just food for thought.
Optional feature (disabled by default in server management panel). Periodically check server health in the background with a periodic worker (run every 30 minutes when there's network). Shows the server connection error notification when health check fails, clears it when it comes back.