ui: display error dialog when saving subnet routes fails (#604)

Fixes tailscale/corp#26175

When setting subnet routing settings, for a variety of reasons the Tailscale backend may reject an entered value with a 400 error. Here we handle such errors in a user-facing fashion:

- We display an ErrorDialog with title 'Failed to save' and whatever error message the backend request returned. To do so, we introduce a new initializer for ErrorDialog that accepts a runtime-generated String instead of a fixed string resource.
- We ask the backend to provide an updated value of AdvertiseRoutes whenever the error dialog is dismissed by the user, and set it as the UI state, to ensure consistency between UI and backend upon a failed save.

Signed-off-by: Andrea Gottardo <[email protected]>

Author: agottardo

android/src/main/java/com/tailscale/ipn/ui/view/ErrorDialog.kt

@@ -53,8 +53,12 @@ enum class ErrorDialogType {
 
 @Composable
 fun ErrorDialog(type: ErrorDialogType, action: () -> Unit = {}) {
-  ErrorDialog(
-      title = type.title, message = type.message, buttonText = type.buttonText, onDismiss = action)
+    ErrorDialog(
+        title = type.title,
+        message = stringResource(id = type.message),
+        buttonText = type.buttonText,
+        onDismiss = action
+    )
 }
 
 @Composable
@@ -64,15 +68,30 @@ fun ErrorDialog(
     @StringRes buttonText: Int = R.string.ok,
     onDismiss: () -> Unit = {}
 ) {
-  AppTheme {
-    AlertDialog(
-      onDismissRequest = onDismiss,
-      title = { Text(text = stringResource(id = title)) },
-      text = { Text(text = stringResource(id = message)) },
-      confirmButton = {
-        PrimaryActionButton(onClick = onDismiss) { Text(text = stringResource(id = buttonText)) }
-      })
-  }
+    ErrorDialog(
+        title = title,
+        message = stringResource(id = message),
+        buttonText = buttonText,
+        onDismiss = onDismiss
+    )
+}
+
+@Composable
+fun ErrorDialog(
+    @StringRes title: Int = R.string.error,
+    message: String,
+    @StringRes buttonText: Int = R.string.ok,
+    onDismiss: () -> Unit = {}
+) {
+    AppTheme {
+        AlertDialog(
+            onDismissRequest = onDismiss,
+            title = { Text(text = stringResource(id = title)) },
+            text = { Text(text = message) },
+            confirmButton = {
+                PrimaryActionButton(onClick = onDismiss) { Text(text = stringResource(id = buttonText)) }
+            })
+    }
 }
 
 @Preview

android/src/main/java/com/tailscale/ipn/ui/view/SubnetRoutingView.kt

@@ -40,6 +40,7 @@ fun SubnetRoutingView(backToSettings: BackNavigation, model: SubnetRoutingViewMo
     val uriHandler = LocalUriHandler.current
     val isPresentingDialog by model.isPresentingDialog.collectAsState()
     val useSubnets by model.routeAll.collectAsState()
+    val currentError by model.currentError.collectAsState()
 
     Scaffold(topBar = {
         Header(R.string.subnet_routes, onBack = backToSettings, actions = {
@@ -56,6 +57,13 @@ fun SubnetRoutingView(backToSettings: BackNavigation, model: SubnetRoutingViewMo
     }) { innerPadding ->
         LoadingIndicator.Wrap {
             LazyColumn(modifier = Modifier.padding(innerPadding)) {
+                currentError?.let {
+                    item("error") {
+                        ErrorDialog(title = R.string.failed_to_save, message = it, onDismiss = {
+                            model.onErrorDismissed()
+                        })
+                    }
+                }
                 item("subnetsToggle") {
                     Setting.Switch(R.string.use_tailscale_subnets, isOn = useSubnets, onToggle = {
                         LoadingIndicator.start()

android/src/main/java/com/tailscale/ipn/ui/viewModel/SubnetRoutingViewModel.kt

@@ -63,6 +63,12 @@ class SubnetRoutingViewModel : ViewModel() {
      */
     val isTextFieldValueValid: StateFlow<Boolean> = MutableStateFlow(true)
 
+    /**
+     * If an error occurred while saving the ipn.Prefs to the backend this value is
+     * non-null. Subsequent successful attempts to save will clear it.
+     */
+    val currentError: MutableStateFlow<String?> = MutableStateFlow(null)
+
     init {
         viewModelScope.launch {
             // Any time the value entered by the user in the add/edit dialog changes, we determine
@@ -113,12 +119,14 @@ class SubnetRoutingViewModel : ViewModel() {
                 Client(viewModelScope).editPrefs(prefsOut, responseHandler = { result ->
                     if (result.isFailure) {
                         Log.e(TAG, "Error saving RouteAll: ${result.exceptionOrNull()}")
+                        currentError.set(result.exceptionOrNull()?.localizedMessage)
                         return@editPrefs
                     } else {
                         Log.d(
                             TAG,
                             "RouteAll set in backend. New value: ${result.getOrNull()?.RouteAll}"
                         )
+                        currentError.set(null)
                     }
                 })
             }
@@ -221,16 +229,30 @@ class SubnetRoutingViewModel : ViewModel() {
         Client(viewModelScope).editPrefs(prefsOut, responseHandler = { result ->
             if (result.isFailure) {
                 Log.e(TAG, "Error saving AdvertiseRoutes: ${result.exceptionOrNull()}")
+                currentError.set(result.exceptionOrNull()?.localizedMessage)
                 return@editPrefs
             } else {
                 Log.d(
                     TAG,
                     "AdvertiseRoutes set in backend. New value: ${result.getOrNull()?.AdvertiseRoutes}"
                 )
+                currentError.set(null)
             }
         })
     }
 
+    /**
+     * Clears the current error message and reloads the routes currently saved in the backend
+     * to the UI. We call this when dismissing an error upon saving the routes.
+     */
+    fun onErrorDismissed() {
+        currentError.set(null)
+        Client(viewModelScope).prefs { response ->
+            Log.d(TAG, "Reloading routes from backend due to failed save: $response")
+            this.advertisedRoutes.set(response.getOrNull()?.AdvertiseRoutes ?: emptyList())
+        }
+    }
+
     companion object RouteValidation {
         /**
          * Returns true if the given String is a valid IPv4 or IPv6 CIDR range, false otherwise.

android/src/main/res/values/strings.xml

@@ -321,5 +321,6 @@
     <string name="subnet_routing">Subnet routing</string>
     <string name="specifies_a_device_name_to_be_used_instead_of_the_automatic_default">Specifies a device name to be used instead of the automatic default.</string>
     <string name="hostname">Hostname</string>
+    <string name="failed_to_save">Failed to save</string>
 
 </resources>