[Fixes #5130] Select mailserver round-robin on connection failure

Signed-off-by: Andrea Maria Piana <andrea.maria.piana@gmail.com>

src/status_im/models/mailserver.cljs

@@ -59,15 +59,39 @@
 (defn fetch-current [{:keys [db] :as cofx}]
   (fetch (:inbox/current-id db) cofx))
 
+(defn preferred-mailserver-id [{:keys [db] :as cofx}]
+  (let [chain (models.network/get-chain cofx)]
+    (get-in db [:account/account :settings :wnode chain])))
+
+(defn- round-robin
+  "Find the choice and pick the next one, default to first if not found"
+  [choices current-id]
+  (let [next-index (reduce
+                    (fn [index choice]
+                      (if (= current-id choice)
+                        (reduced (inc index))
+                        (inc index)))
+                    0
+                    choices)]
+    (nth choices
+         (mod
+          next-index
+          (count choices)))))
+
 (defn selected-or-random-id
-  "Use the preferred mailserver if set & exists, otherwise picks one randomly"
+  "Use the preferred mailserver if set & exists, otherwise picks one randomly
+  if current-id is not set, else round-robin"
   [{:keys [db] :as cofx}]
-  (let [chain (models.network/get-chain cofx)
+  (let [chain      (models.network/get-chain cofx)
-        preference (get-in db [:account/account :settings :wnode chain])]
+        current-id (:inbox/current-id db)
+        preference (preferred-mailserver-id cofx)
+        choices    (-> db :inbox/wnodes chain keys)]
     (if (and preference
              (fetch preference cofx))
       preference
-      (-> db :inbox/wnodes chain keys rand-nth))))
+      (if current-id
+        (round-robin choices current-id)
+        (rand-nth choices)))))
 
 (def default? (comp not :user-defined fetch))
 

src/status_im/transport/inbox.cljs

@@ -39,7 +39,7 @@
 
 (def connection-timeout
   "Time after which mailserver connection is considered to have failed"
-  60000)
+  15000)
 
 (def fetching-timeout
   "Time we should wait after last message was fetch from mailserver before we
@@ -314,9 +314,13 @@
 
 (handlers/register-handler-fx
  :inbox/check-connection
- (fn [{:keys [db] :as cofx} [_ _]]
+ (fn [{:keys [db] :as cofx} _]
    (when (= :connecting (:mailserver-status db))
-     (update-mailserver-status :error cofx))))
+     (if (models.mailserver/preferred-mailserver-id cofx)
+       (update-mailserver-status :error cofx)
+       (handlers-macro/merge-fx cofx
+                                (models.mailserver/set-current-mailserver)
+                                (connect-to-mailserver))))))
 
 (defn update-last-request [last-request {:keys [db]}]
   (let [chats         (:transport/chats db)

test/cljs/status_im/test/models/mailserver.cljs

@@ -146,7 +146,6 @@
     (let [cofx {:db {:network "mainnet_rpc"
                      :account/account {:networks {"mainnet_rpc"
                                                   {:config {:NetworkId 1}}}}
-                     :inbox/current-id "a"
                      :inbox/wnodes {:mainnet {"a" {}
                                               "b" {}
                                               "c" {}
@@ -167,10 +166,31 @@
                                :db
                                :inbox/current-id))))))))
       (testing "the user has not set an explicit preference"
-        (testing "it sets a random mailserver"
+        (testing "current-id is not set"
-          (is (= "d" (-> (model/set-current-mailserver cofx)
+          (testing "it sets a random mailserver"
-                         :db
+            (is (= "d" (-> (model/set-current-mailserver cofx)
-                         :inbox/current-id))))))))
+                           :db
+                           :inbox/current-id)))))
+        (testing "current-id is set"
+          (testing "it sets the next mailserver"
+            (is (= "c" (-> (model/set-current-mailserver (assoc-in
+                                                          cofx
+                                                          [:db :inbox/current-id]
+                                                          "b"))
+                           :db
+                           :inbox/current-id)))
+            (is (= "a" (-> (model/set-current-mailserver (assoc-in
+                                                          cofx
+                                                          [:db :inbox/current-id]
+                                                          "d"))
+                           :db
+                           :inbox/current-id)))
+            (is (= "a" (-> (model/set-current-mailserver (assoc-in
+                                                          cofx
+                                                          [:db :inbox/current-id]
+                                                          "non-existing"))
+                           :db
+                           :inbox/current-id)))))))))
 
 (deftest delete-mailserver
   (testing "the user is not connected to the mailserver"