From 5a97d322c4aaea786ba5732fdf5fd6134af20a3c Mon Sep 17 00:00:00 2001
From: Benoit Marty <benoit@matrix.org>
Date: Tue, 2 Apr 2024 16:40:07 +0200
Subject: [PATCH] Code clarity: use withTimeout with combination of runCatching
 instead of withTimeoutOrNull

---
 .../impl/troubleshoot/NotificationTest.kt     | 38 ++++++++++---------
 .../impl/troubleshoot/PushLoopbackTest.kt     | 36 ++++++++++--------
 2 files changed, 41 insertions(+), 33 deletions(-)

diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/NotificationTest.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/NotificationTest.kt
index d7e3641e22..26b7161fca 100644
--- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/NotificationTest.kt
+++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/NotificationTest.kt
@@ -29,7 +29,7 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.launch
-import kotlinx.coroutines.withTimeoutOrNull
+import kotlinx.coroutines.withTimeout
 import timber.log.Timber
 import javax.inject.Inject
 import kotlin.time.Duration.Companion.seconds
@@ -72,22 +72,26 @@ class NotificationTest @Inject constructor(
             notificationClickHandler.state.first()
             Timber.d("Notification clicked!")
         }
-        val s = withTimeoutOrNull(30.seconds) {
-            job.join()
-        }
-        job.cancel()
-        if (s == null) {
-            notificationDisplayer.dismissDiagnosticNotification()
-            delegate.updateState(
-                description = stringProvider.getString(R.string.troubleshoot_notifications_test_display_notification_failure),
-                status = NotificationTroubleshootTestState.Status.Failure(false)
-            )
-        } else {
-            delegate.updateState(
-                description = stringProvider.getString(R.string.troubleshoot_notifications_test_display_notification_success),
-                status = NotificationTroubleshootTestState.Status.Success
-            )
-        }
+        runCatching {
+            withTimeout(30.seconds) {
+                job.join()
+            }
+        }.fold(
+            onSuccess = {
+                delegate.updateState(
+                    description = stringProvider.getString(R.string.troubleshoot_notifications_test_display_notification_success),
+                    status = NotificationTroubleshootTestState.Status.Success
+                )
+            },
+            onFailure = {
+                job.cancel()
+                notificationDisplayer.dismissDiagnosticNotification()
+                delegate.updateState(
+                    description = stringProvider.getString(R.string.troubleshoot_notifications_test_display_notification_failure),
+                    status = NotificationTroubleshootTestState.Status.Failure(false)
+                )
+            }
+        )
     }.invokeOnCompletion {
         // Ensure that the notification is cancelled when the screen is left
         notificationDisplayer.dismissDiagnosticNotification()
diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTest.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTest.kt
index 2ecec61996..861554bef8 100644
--- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTest.kt
+++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/troubleshoot/PushLoopbackTest.kt
@@ -31,7 +31,7 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.launch
-import kotlinx.coroutines.withTimeoutOrNull
+import kotlinx.coroutines.withTimeout
 import timber.log.Timber
 import javax.inject.Inject
 import kotlin.time.Duration.Companion.seconds
@@ -84,21 +84,25 @@ class PushLoopbackTest @Inject constructor(
             job.cancel()
             return
         }
-        val result = withTimeoutOrNull(10.seconds) {
-            completable.await()
-        }
-        job.cancel()
-        if (result == null) {
-            delegate.updateState(
-                description = stringProvider.getString(R.string.troubleshoot_notifications_test_push_loop_back_failure_4),
-                status = NotificationTroubleshootTestState.Status.Failure(false)
-            )
-        } else {
-            delegate.updateState(
-                description = stringProvider.getString(R.string.troubleshoot_notifications_test_push_loop_back_success, result),
-                status = NotificationTroubleshootTestState.Status.Success
-            )
-        }
+        runCatching {
+            withTimeout(10.seconds) {
+                completable.await()
+            }
+        }.fold(
+            onSuccess = { duration ->
+                delegate.updateState(
+                    description = stringProvider.getString(R.string.troubleshoot_notifications_test_push_loop_back_success, duration),
+                    status = NotificationTroubleshootTestState.Status.Success
+                )
+            },
+            onFailure = {
+                job.cancel()
+                delegate.updateState(
+                    description = stringProvider.getString(R.string.troubleshoot_notifications_test_push_loop_back_failure_4),
+                    status = NotificationTroubleshootTestState.Status.Failure(false)
+                )
+            }
+        )
     }
 
     override suspend fun reset() = delegate.reset()