From a8c39370d505f335ed6c155cecb8abe4915bef9e Mon Sep 17 00:00:00 2001 From: Andrea Maria Piana Date: Fri, 24 Jan 2020 09:45:45 +0100 Subject: [PATCH] Fire request once network status changes & periodically check connection This commit fixes a couple of issues. The first one is that when we go from offline->cellular-network->wifi historical requests were not triggered (only when going from offline->online, but if they are disabled in cellular network they won't fire). The second is due to the fact that it is possible that the connection status gets into a bad state, (this happens for example if get-latency returns all errors, but we are online, and it won't be retried), and no connection attempt is made anymore. I have changed the logic to periodically check the connection state, and try to connect if too much time has passed since last error (10s), similarly to what we did before, but in a tick, so we are less reliant on having the right state in the db. Signed-off-by: yenda --- src/status_im/mailserver/core.cljs | 35 ++++++++----------- .../mobile_network_settings/events.cljs | 3 +- src/status_im/utils/publisher.cljs | 4 ++- test/cljs/status_im/test/mailserver/core.cljs | 4 +-- 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/status_im/mailserver/core.cljs b/src/status_im/mailserver/core.cljs index d4a573c411..13231eda90 100644 --- a/src/status_im/mailserver/core.cljs +++ b/src/status_im/mailserver/core.cljs @@ -193,21 +193,19 @@ (generate-mailserver-symkey mailserver))))) (fx/defn add-peer - [{:keys [db] :as cofx}] + [{:keys [db now] :as cofx}] (let [{:keys [address sym-key-id generating-sym-key?] :as mailserver} (fetch-current db)] (fx/merge cofx - {:db (-> db - (update-mailserver-state :connecting) - (update :mailserver/connection-checks inc)) + {:db (assoc + (update-mailserver-state db :connecting) + :mailserver/last-connection-attempt now) :mailserver/add-peer address ;; Any message sent before this takes effect will not be marked as sent ;; probably we should improve the UX so that is more transparent to the ;; user - :mailserver/update-mailservers [address] - :utils/dispatch-later [{:ms constants/connection-timeout - :dispatch [:mailserver/check-connection-timeout]}]} + :mailserver/update-mailservers [address]} (when-not (or sym-key-id generating-sym-key?) (generate-mailserver-symkey mailserver))))) @@ -521,23 +519,20 @@ {:mailserver/remove-peer address} (set-current-mailserver)))))) +(defn check-connection! [] + (re-frame/dispatch [:mailserver/check-connection-timeout])) + (fx/defn check-connection - "connection-checks counter is used to prevent changing - mailserver on flaky connections - if there is more than one connection check pending - decrement the connection check counter - else - change mailserver if mailserver is connected" - [{:keys [db] :as cofx}] + "Check connection checks that the connection is successfully connected, + otherwise it will try to change mailserver and connect again" + [{:keys [db now] :as cofx}] ;; check if logged into multiaccount (when (contains? db :multiaccount) - (let [connection-checks (dec (:mailserver/connection-checks db))] - (if (>= 0 connection-checks) + (let [last-connection-attempt (:mailserver/last-connection-attempt db)] + (when (<= (- now last-connection-attempt)) (fx/merge cofx - {:db (dissoc db :mailserver/connection-checks)} - (when (= :connecting (:mailserver/state db)) - (change-mailserver))) - {:db (update db :mailserver/connection-checks dec)})))) + (when (not= :connected (:mailserver/state db)) + (change-mailserver))))))) (fx/defn reset-request-to [{:keys [db]}] diff --git a/src/status_im/ui/screens/mobile_network_settings/events.cljs b/src/status_im/ui/screens/mobile_network_settings/events.cljs index 720617e8cd..93157870e3 100644 --- a/src/status_im/ui/screens/mobile_network_settings/events.cljs +++ b/src/status_im/ui/screens/mobile_network_settings/events.cljs @@ -33,7 +33,8 @@ (sheet-defaults)] logged-in? - [(bottom-sheet/hide-bottom-sheet)])))) + [(mailserver/process-next-messages-request) + (bottom-sheet/hide-bottom-sheet)])))) (defn apply-settings ([sync?] (apply-settings sync? :default)) diff --git a/src/status_im/utils/publisher.cljs b/src/status_im/utils/publisher.cljs index eb15885d18..a562ee8f30 100644 --- a/src/status_im/utils/publisher.cljs +++ b/src/status_im/utils/publisher.cljs @@ -3,11 +3,12 @@ [re-frame.db] [status-im.multiaccounts.update.publisher :as multiaccounts] [status-im.utils.async :as async-util] + [status-im.mailserver.core :as mailserver] [status-im.utils.datetime :as datetime] [status-im.utils.fx :as fx])) (defonce polling-executor (atom nil)) -(def sync-interval-ms 120000) +(def sync-interval-ms 10000) (def sync-timeout-ms 20000) (defn- start-publisher! [] @@ -18,6 +19,7 @@ (fn [done-fn] (let [cofx {:now (datetime/timestamp) :db @re-frame.db/app-db}] + (mailserver/check-connection!) (multiaccounts/publish-update! cofx) (done-fn))) sync-interval-ms diff --git a/test/cljs/status_im/test/mailserver/core.cljs b/test/cljs/status_im/test/mailserver/core.cljs index 98f356503d..7d0efeff86 100644 --- a/test/cljs/status_im/test/mailserver/core.cljs +++ b/test/cljs/status_im/test/mailserver/core.cljs @@ -571,13 +571,13 @@ (testing "Mailserver disconnected, sym-key exists" (let [result (peers-summary-change-result true false true)] (is (= (into #{} (keys result)) - #{:db :mailserver/add-peer :utils/dispatch-later :mailserver/update-mailservers})) + #{:db :mailserver/add-peer :mailserver/update-mailservers})) (is (= (get-in result [:db :mailserver/state]) :connecting)))) (testing "Mailserver disconnected, sym-key doesn't exists (unlikely situation in practice)" (let [result (peers-summary-change-result false false true)] (is (= (into #{} (keys result)) - #{:db :mailserver/add-peer :utils/dispatch-later :shh/generate-sym-key-from-password :mailserver/update-mailservers})) + #{:db :mailserver/add-peer :shh/generate-sym-key-from-password :mailserver/update-mailservers})) (is (= (get-in result [:db :mailserver/state]) :connecting)))) (testing "Mailserver isn't concerned by peer summary changes"