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 <eric@status.im>
This commit is contained in:
Andrea Maria Piana 2020-01-24 09:45:45 +01:00 committed by yenda
parent ea02f8e3cf
commit a8c39370d5
No known key found for this signature in database
GPG Key ID: 0095623C0069DCE6
4 changed files with 22 additions and 24 deletions

View File

@ -193,21 +193,19 @@
(generate-mailserver-symkey mailserver))))) (generate-mailserver-symkey mailserver)))))
(fx/defn add-peer (fx/defn add-peer
[{:keys [db] :as cofx}] [{:keys [db now] :as cofx}]
(let [{:keys [address sym-key-id generating-sym-key?] :as mailserver} (let [{:keys [address sym-key-id generating-sym-key?] :as mailserver}
(fetch-current db)] (fetch-current db)]
(fx/merge (fx/merge
cofx cofx
{:db (-> db {:db (assoc
(update-mailserver-state :connecting) (update-mailserver-state db :connecting)
(update :mailserver/connection-checks inc)) :mailserver/last-connection-attempt now)
:mailserver/add-peer address :mailserver/add-peer address
;; Any message sent before this takes effect will not be marked as sent ;; 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 ;; probably we should improve the UX so that is more transparent to the
;; user ;; user
:mailserver/update-mailservers [address] :mailserver/update-mailservers [address]}
:utils/dispatch-later [{:ms constants/connection-timeout
:dispatch [:mailserver/check-connection-timeout]}]}
(when-not (or sym-key-id generating-sym-key?) (when-not (or sym-key-id generating-sym-key?)
(generate-mailserver-symkey mailserver))))) (generate-mailserver-symkey mailserver)))))
@ -521,23 +519,20 @@
{:mailserver/remove-peer address} {:mailserver/remove-peer address}
(set-current-mailserver)))))) (set-current-mailserver))))))
(defn check-connection! []
(re-frame/dispatch [:mailserver/check-connection-timeout]))
(fx/defn check-connection (fx/defn check-connection
"connection-checks counter is used to prevent changing "Check connection checks that the connection is successfully connected,
mailserver on flaky connections otherwise it will try to change mailserver and connect again"
if there is more than one connection check pending [{:keys [db now] :as cofx}]
decrement the connection check counter
else
change mailserver if mailserver is connected"
[{:keys [db] :as cofx}]
;; check if logged into multiaccount ;; check if logged into multiaccount
(when (contains? db :multiaccount) (when (contains? db :multiaccount)
(let [connection-checks (dec (:mailserver/connection-checks db))] (let [last-connection-attempt (:mailserver/last-connection-attempt db)]
(if (>= 0 connection-checks) (when (<= (- now last-connection-attempt))
(fx/merge cofx (fx/merge cofx
{:db (dissoc db :mailserver/connection-checks)} (when (not= :connected (:mailserver/state db))
(when (= :connecting (:mailserver/state db)) (change-mailserver)))))))
(change-mailserver)))
{:db (update db :mailserver/connection-checks dec)}))))
(fx/defn reset-request-to (fx/defn reset-request-to
[{:keys [db]}] [{:keys [db]}]

View File

@ -33,7 +33,8 @@
(sheet-defaults)] (sheet-defaults)]
logged-in? logged-in?
[(bottom-sheet/hide-bottom-sheet)])))) [(mailserver/process-next-messages-request)
(bottom-sheet/hide-bottom-sheet)]))))
(defn apply-settings (defn apply-settings
([sync?] (apply-settings sync? :default)) ([sync?] (apply-settings sync? :default))

View File

@ -3,11 +3,12 @@
[re-frame.db] [re-frame.db]
[status-im.multiaccounts.update.publisher :as multiaccounts] [status-im.multiaccounts.update.publisher :as multiaccounts]
[status-im.utils.async :as async-util] [status-im.utils.async :as async-util]
[status-im.mailserver.core :as mailserver]
[status-im.utils.datetime :as datetime] [status-im.utils.datetime :as datetime]
[status-im.utils.fx :as fx])) [status-im.utils.fx :as fx]))
(defonce polling-executor (atom nil)) (defonce polling-executor (atom nil))
(def sync-interval-ms 120000) (def sync-interval-ms 10000)
(def sync-timeout-ms 20000) (def sync-timeout-ms 20000)
(defn- start-publisher! [] (defn- start-publisher! []
@ -18,6 +19,7 @@
(fn [done-fn] (fn [done-fn]
(let [cofx {:now (datetime/timestamp) (let [cofx {:now (datetime/timestamp)
:db @re-frame.db/app-db}] :db @re-frame.db/app-db}]
(mailserver/check-connection!)
(multiaccounts/publish-update! cofx) (multiaccounts/publish-update! cofx)
(done-fn))) (done-fn)))
sync-interval-ms sync-interval-ms

View File

@ -571,13 +571,13 @@
(testing "Mailserver disconnected, sym-key exists" (testing "Mailserver disconnected, sym-key exists"
(let [result (peers-summary-change-result true false true)] (let [result (peers-summary-change-result true false true)]
(is (= (into #{} (keys result)) (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]) (is (= (get-in result [:db :mailserver/state])
:connecting)))) :connecting))))
(testing "Mailserver disconnected, sym-key doesn't exists (unlikely situation in practice)" (testing "Mailserver disconnected, sym-key doesn't exists (unlikely situation in practice)"
(let [result (peers-summary-change-result false false true)] (let [result (peers-summary-change-result false false true)]
(is (= (into #{} (keys result)) (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]) (is (= (get-in result [:db :mailserver/state])
:connecting)))) :connecting))))
(testing "Mailserver isn't concerned by peer summary changes" (testing "Mailserver isn't concerned by peer summary changes"