Handle 'invalid server' error in server selection screen properly. (#214)

* Handle 'invalid server' error in server selection screen properly.

* Use `action_learn_more` for composing the server location footer action.

changelog.d/210.bugfix

@@ -0,0 +1 @@
+Handle 'invalid server' error in server selection screen properly.

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerError.kt

@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2023 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.features.login.impl.changeserver
+
+import androidx.annotation.StringRes
+import androidx.compose.runtime.Composable
+import androidx.compose.ui.res.stringResource
+import io.element.android.libraries.matrix.api.auth.AuthenticationException
+import io.element.android.libraries.ui.strings.R
+
+sealed class ChangeServerError : Throwable() {
+    data class InlineErrorMessage(@StringRes val messageId: Int) : ChangeServerError() {
+        @Composable
+        fun message(): String = stringResource(messageId)
+    }
+    object SlidingSyncAlert : ChangeServerError()
+
+    companion object {
+        fun from(error: Throwable): ChangeServerError = when (error) {
+            is AuthenticationException.SlidingSyncNotAvailable -> SlidingSyncAlert
+            else -> InlineErrorMessage(R.string.server_selection_invalid_homeserver_error)
+        }
+    }
+}

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt

@@ -48,7 +48,10 @@ class ChangeServerPresenter @Inject constructor(private val authenticationServic
 
         fun handleEvents(event: ChangeServerEvents) {
             when (event) {
-                is ChangeServerEvents.SetServer -> homeserver.value = event.server
+                is ChangeServerEvents.SetServer -> {
+                    homeserver.value = event.server
+                    handleEvents(ChangeServerEvents.ClearError)
+                }
                 ChangeServerEvents.Submit -> {
                     localCoroutineScope.submit(homeserver, changeServerAction)
                 }
@@ -68,6 +71,6 @@ class ChangeServerPresenter @Inject constructor(private val authenticationServic
             val domain = tryOrNull { URL(homeserverUrl.value) }?.host ?: homeserverUrl.value
             authenticationService.setHomeserver(domain)
             homeserverUrl.value = domain
-        }.execute(changeServerAction)
+        }.execute(changeServerAction, errorMapping = ChangeServerError::from)
     }
 }

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerState.kt

@@ -23,5 +23,5 @@ data class ChangeServerState(
     val changeServerAction: Async<Unit>,
     val eventSink: (ChangeServerEvents) -> Unit,
 ) {
-    val submitEnabled = homeserver.isNotEmpty() && changeServerAction !is Async.Loading
+    val submitEnabled = homeserver.isNotEmpty() && changeServerAction is Async.Uninitialized
 }

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerStateProvider.kt

@@ -17,6 +17,7 @@
 package io.element.android.features.login.impl.changeserver
 
 import androidx.compose.ui.tooling.preview.PreviewParameterProvider
+import io.element.android.libraries.ui.strings.R
 import io.element.android.libraries.architecture.Async
 
 open class ChangeServerStateProvider : PreviewParameterProvider<ChangeServerState> {
@@ -25,7 +26,11 @@ open class ChangeServerStateProvider : PreviewParameterProvider<ChangeServerStat
             aChangeServerState(),
             aChangeServerState().copy(homeserver = "matrix.org"),
             aChangeServerState().copy(homeserver = "matrix.org", changeServerAction = Async.Loading()),
-            aChangeServerState().copy(homeserver = "invalid.org", changeServerAction = Async.Failure(Throwable("An error"))),
+            aChangeServerState().copy(
+                homeserver = "invalid.org",
+                changeServerAction = Async.Failure(ChangeServerError.InlineErrorMessage(R.string.server_selection_invalid_homeserver_error))
+            ),
+            aChangeServerState().copy(homeserver = "invalid.org", changeServerAction = Async.Failure(ChangeServerError.SlidingSyncAlert)),
             aChangeServerState().copy(homeserver = "matrix.org", changeServerAction = Async.Success(Unit)),
         )
 }

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerView.kt

@@ -58,7 +58,6 @@ import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.tooling.preview.PreviewParameter
 import androidx.compose.ui.unit.dp
 import io.element.android.features.login.impl.R
-import io.element.android.features.login.impl.error.changeServerError
 import io.element.android.features.login.impl.util.LoginConstants
 import io.element.android.libraries.architecture.Async
 import io.element.android.libraries.designsystem.ElementTextStyles
@@ -80,7 +79,6 @@ import io.element.android.libraries.designsystem.theme.components.Text
 import io.element.android.libraries.designsystem.theme.components.TextField
 import io.element.android.libraries.designsystem.theme.components.TopAppBar
 import io.element.android.libraries.designsystem.theme.components.onTabOrEnterKeyFocusNext
-import io.element.android.libraries.matrix.api.auth.AuthenticationException
 import io.element.android.libraries.testtags.TestTags
 import io.element.android.libraries.testtags.testTag
 import io.element.android.libraries.ui.strings.R as StringR
@@ -101,6 +99,8 @@ fun ChangeServerView(
             state.changeServerAction !is Async.Loading
         }
     }
+    val invalidHomeserverError = (state.changeServerAction as? Async.Failure)?.error as? ChangeServerError.InlineErrorMessage
+    val slidingSyncNotSupportedError = (state.changeServerAction as? Async.Failure)?.error as? ChangeServerError.SlidingSyncAlert
     val focusManager = LocalFocusManager.current
 
     fun submit() {
@@ -200,53 +200,50 @@ fun ChangeServerView(
                     trailingIcon = if (homeserverFieldState.isNotEmpty()) {
                         {
                             IconButton(onClick = {
-                                homeserverFieldState = ""
+                                eventSink(ChangeServerEvents.SetServer(""))
                             }, enabled = interactionEnabled) {
                                 Icon(imageVector = Icons.Filled.Close, contentDescription = stringResource(StringR.string.a11y_clear))
                             }
                         }
                     } else null,
-                )
-                if (state.changeServerAction is Async.Failure) {
-                    if (state.changeServerAction.error is AuthenticationException.SlidingSyncNotAvailable) {
-                        SlidingSyncNotSupportedDialog(onLearnMoreClicked = {
-                            onLearnMoreClicked()
-                            eventSink(ChangeServerEvents.ClearError)
-                        }, onDismiss = {
-                            eventSink(ChangeServerEvents.ClearError)
-                        })
-                    } else {
-                        ChangeServerErrorDialog(
-                            error = state.changeServerAction.error,
-                            onDismiss = {
-                                eventSink(ChangeServerEvents.ClearError)
+                    isError = invalidHomeserverError != null,
+                    supportingText = {
+                        if (invalidHomeserverError != null) {
+                            Text(invalidHomeserverError.message(), color = MaterialTheme.colorScheme.error)
+                        } else {
+                            val footerMessage = stringResource(StringR.string.server_selection_server_footer)
+                            val footerAction = stringResource(StringR.string.action_learn_more)
+                            val footerText = buildAnnotatedString {
+                                val defaultColor = MaterialTheme.colorScheme.tertiary
+                                withStyle(ParagraphStyle(textAlign = TextAlign.Start)) {
+                                    withStyle(SpanStyle(color = defaultColor)) {
+                                        append(footerMessage)
+                                        append(" ")
+                                    }
+                                    val start = length
+                                    withStyle(SpanStyle(color = LinkColor)) {
+                                        append(footerAction)
+                                    }
+                                    addUrlAnnotation(UrlAnnotation(LoginConstants.SLIDING_SYNC_READ_MORE_URL), start, length)
+                                }
                             }
-                        )
-                    }
-                }
-                Spacer(Modifier.height(8.dp))
-                val footerMessage = stringResource(StringR.string.server_selection_server_footer)
-                val footerAction = stringResource(StringR.string.server_selection_server_footer_action)
-                val footerText = buildAnnotatedString {
-                    val defaultColor = MaterialTheme.colorScheme.tertiary
-                    withStyle(ParagraphStyle(textAlign = TextAlign.Start)) {
-                        withStyle(SpanStyle(color = defaultColor)) {
-                            append(footerMessage)
-                            append(" ")
+                            ClickableLinkText(
+                                text = footerText,
+                                interactionSource = MutableInteractionSource(),
+                                style = ElementTextStyles.Regular.caption1,
+                            )
                         }
-                        val start = length
-                        withStyle(SpanStyle(color = LinkColor)) {
-                            append(footerAction)
-                        }
-                        addUrlAnnotation(UrlAnnotation(LoginConstants.SLIDING_SYNC_READ_MORE_URL), start, length)
                     }
-                }
-                ClickableLinkText(
-                    text = footerText,
-                    interactionSource = MutableInteractionSource(),
-                    modifier = Modifier.padding(horizontal = 16.dp),
-                    style = ElementTextStyles.Regular.caption1,
+
                 )
+                if (slidingSyncNotSupportedError != null) {
+                    SlidingSyncNotSupportedDialog(onLearnMoreClicked = {
+                        onLearnMoreClicked()
+                        eventSink(ChangeServerEvents.ClearError)
+                    }, onDismiss = {
+                        eventSink(ChangeServerEvents.ClearError)
+                    })
+                }
                 Spacer(Modifier.height(32.dp))
                 Button(
                     onClick = ::submit,
@@ -271,9 +268,10 @@ fun ChangeServerView(
 }
 
 @Composable
-internal fun ChangeServerErrorDialog(error: Throwable, onDismiss: () -> Unit) {
+internal fun ChangeServerErrorDialog(title: String, message: String, onDismiss: () -> Unit) {
     ErrorDialog(
-        content = stringResource(changeServerError(error)),
+        title = title,
+        content = message,
         onDismiss = onDismiss
     )
 }

features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ErrorFormatter.kt

@@ -31,13 +31,3 @@ fun loginError(
         AuthErrorCode.UNKNOWN -> StringR.unknown_error
     }
 }
-
-fun changeServerError(
-    throwable: Throwable
-): Int {
-    val authException = throwable as? AuthenticationException ?: return StringR.unknown_error
-    return when (authException) {
-        is AuthenticationException.InvalidServerName -> StringR.login_error_homeserver_not_found
-        else -> StringR.unknown_error
-    }
-}

features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt

@@ -95,7 +95,7 @@ class ChangeServerPresenterTest {
             assertThat(loadingState.submitEnabled).isFalse()
             assertThat(loadingState.changeServerAction).isInstanceOf(Async.Loading::class.java)
             val successState = awaitItem()
-            assertThat(successState.submitEnabled).isTrue()
+            assertThat(successState.submitEnabled).isFalse()
             assertThat(successState.changeServerAction).isInstanceOf(Async.Success::class.java)
         }
     }
@@ -118,7 +118,7 @@ class ChangeServerPresenterTest {
             assertThat(loadingState.changeServerAction).isInstanceOf(Async.Loading::class.java)
             awaitItem() // Skip changing the url to the parsed domain
             val successState = awaitItem()
-            assertThat(successState.submitEnabled).isTrue()
+            assertThat(successState.submitEnabled).isFalse()
             assertThat(successState.changeServerAction).isInstanceOf(Async.Success::class.java)
             assertThat(successState.homeserver).isEqualTo("matrix.org")
         }
@@ -135,7 +135,7 @@ class ChangeServerPresenterTest {
             authServer.givenChangeServerError(Throwable())
             initialState.eventSink.invoke(ChangeServerEvents.Submit)
             val failureState = awaitItem()
-            assertThat(failureState.submitEnabled).isTrue()
+            assertThat(failureState.submitEnabled).isFalse()
             assertThat(failureState.changeServerAction).isInstanceOf(Async.Failure::class.java)
         }
     }
@@ -157,7 +157,7 @@ class ChangeServerPresenterTest {
 
             // Check an error was returned
             val submittedState = awaitItem()
-            assertThat(submittedState.changeServerAction).isEqualTo(Async.Failure<Unit>(A_THROWABLE))
+            assertThat(submittedState.changeServerAction).isInstanceOf(Async.Failure::class.java)
 
             // Assert the error is then cleared
             submittedState.eventSink(ChangeServerEvents.ClearError)

features/login/impl/src/test/kotlin/io/element/android/features/login/impl/error/ErrorFormatterTests.kt

@@ -55,44 +55,4 @@ class ErrorFormatterTests {
     }
 
     // endregion loginError
-
-    // region changeServerError
-
-    @Test
-    fun `changeServerError - invalid unknown error returns unknown error message`() {
-        val error = Throwable("Some unknown error")
-        assertThat(changeServerError(error)).isEqualTo(R.string.unknown_error)
-    }
-
-    @Test
-    fun `changeServerError - invalid auth error returns unknown error message`() {
-        val error = AuthenticationException.SlidingSyncNotAvailable("Some message. Also contains M_FORBIDDEN, but won't be parsed")
-        assertThat(changeServerError(error)).isEqualTo(R.string.unknown_error)
-    }
-
-    @Test
-    fun `changeServerError - unknown error returns unknown error message`() {
-        val error = AuthenticationException.Generic("M_UNKNOWN")
-        assertThat(changeServerError(error)).isEqualTo(R.string.unknown_error)
-    }
-
-    @Test
-    fun `changeServerError - forbidden error returns unknown error message`() {
-        val error = AuthenticationException.Generic("M_FORBIDDEN")
-        assertThat(changeServerError(error)).isEqualTo(R.string.unknown_error)
-    }
-
-    @Test
-    fun `changeServerError - user_deactivated error returns unknown error message`() {
-        val error = AuthenticationException.Generic("M_USER_DEACTIVATED")
-        assertThat(changeServerError(error)).isEqualTo(R.string.unknown_error)
-    }
-
-    @Test
-    fun `changeServerError - invalid server name error returns invalid server name error message`() {
-        val error = AuthenticationException.InvalidServerName("Server is not valid")
-        assertThat(changeServerError(error)).isEqualTo(R.string.login_error_homeserver_not_found)
-    }
-
-    // endregion changeServerError
 }

libraries/architecture/src/main/kotlin/io/element/android/libraries/architecture/Async.kt

@@ -36,13 +36,13 @@ sealed interface Async<out T> {
     }
 }
 
-suspend fun <T> (suspend () -> T).execute(state: MutableState<Async<T>>) {
+suspend fun <T> (suspend () -> T).execute(state: MutableState<Async<T>>, errorMapping: ((Throwable) -> Throwable)? = null) {
     try {
         state.value = Async.Loading()
         val result = this()
         state.value = Async.Success(result)
     } catch (error: Throwable) {
-        state.value = Async.Failure(error)
+        state.value = Async.Failure(errorMapping?.invoke(error) ?: error)
     }
 }
 

libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/dialogs/ErrorDialog.kt

@@ -18,7 +18,6 @@ package io.element.android.libraries.designsystem.components.dialogs
 
 import androidx.compose.material3.AlertDialog
 import androidx.compose.material3.AlertDialogDefaults
-import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.TextButton
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
@@ -36,8 +35,8 @@ import io.element.android.libraries.ui.strings.R as StringR
 fun ErrorDialog(
     content: String,
     modifier: Modifier = Modifier,
-    title: String = stringResource(id = StringR.string.dialog_title_error),
-    submitText: String = stringResource(id = StringR.string.ok),
+    title: String = ErrorDialogDefaults.title,
+    submitText: String = ErrorDialogDefaults.submitText,
     onDismiss: () -> Unit = {},
     shape: Shape = AlertDialogDefaults.shape,
     containerColor: Color = AlertDialogDefaults.containerColor,
@@ -69,6 +68,11 @@ fun ErrorDialog(
     )
 }
 
+object ErrorDialogDefaults {
+    val title: String @Composable get() = stringResource(id = StringR.string.dialog_title_error)
+    val submitText: String @Composable get() = stringResource(id = StringR.string.ok)
+}
+
 @Preview
 @Composable
 internal fun ErrorDialogLightPreview() = ElementPreviewLight { ContentToPreview() }

libraries/ui-strings/src/main/res/values/strings_eax.xml

@@ -12,12 +12,13 @@
     <string name="login_hide_password">Hide password</string>
     <string name="ex_choose_server_subtitle">What is the address of your server?</string>
     <string name="server_selection_server_footer">You can only connect to an existing server that supports sliding sync. Your homeserver admin will need to configure it.</string>
-    <string name="server_selection_server_footer_action">Learn more</string>
     <string name="server_selection_sliding_sync_alert_title">Server not supported</string>
     <string name="server_selection_sliding_sync_alert_message">This server currently doesn\'t support sliding sync.</string>
+    <string name="server_selection_invalid_homeserver_error">We couldn\'t reach this homeserver. Please check that you have entered the homeserver URL correctly. If the URL is correct, contact your homeserver administrator for further help.</string>
     <!-- Create room -->
     <string name="search_for_someone">Search for someone</string>
     <string name="new_room">New room</string>
+
     <!-- Room list -->
     <string name="notice_room_invite_accepted_by_you">You accepted the invite</string>
     <string name="notice_room_invite_accepted">%1$s accepted the invite</string>
@@ -58,5 +59,4 @@
     <string name="session_verification_start">Continue</string>
 
     <string name="verification_conclusion_ok_self_notice_title">Verification complete</string>
-
 </resources>