diff --git a/frontend/src/app/main/errors.cljs b/frontend/src/app/main/errors.cljs index 63402ae3f2..b1df018682 100644 --- a/frontend/src/app/main/errors.cljs +++ b/frontend/src/app/main/errors.cljs @@ -139,6 +139,15 @@ :level :error :timeout 5000}))) +(defmethod ptk/handle-error :network + [error] + ;; Transient network errors (e.g. lost connectivity, DNS failure) + ;; should not replace the entire page with an error screen. Show a + ;; non-intrusive toast instead and let the user continue working. + (when-let [cause (::instance error)] + (ex/print-throwable cause :prefix "Network Error")) + (flash :cause (::instance error) :type :handled)) + (defmethod ptk/handle-error :internal [error] (st/emit! (rt/assign-exception error)) diff --git a/frontend/src/app/main/repo.cljs b/frontend/src/app/main/repo.cljs index ad252e2a04..d2a0ff5068 100644 --- a/frontend/src/app/main/repo.cljs +++ b/frontend/src/app/main/repo.cljs @@ -21,6 +21,61 @@ (log/set-level! :info) +;; -- Retry helpers ----------------------------------------------------------- + +(def ^:private retryable-types + "Set of error types that are considered transient and safe to retry + for idempotent (GET) requests." + #{:network ; js/fetch network-level failure + :bad-gateway ; 502 + :service-unavailable ; 503 + :offline}) ; status 0 (browser offline) + +(defn retryable-error? + "Return true when `error` represents a transient failure that is safe + to retry. Only errors whose `ex-data` `:type` belongs to + `retryable-types` qualify." + [error] + (contains? retryable-types (:type (ex-data error)))) + +(def default-retry-config + "Default configuration for the retry mechanism on idempotent requests." + {:max-retries 3 + :base-delay-ms 1000}) + +(defn with-retry + "Wrap `observable-fn` (a zero-arg function returning an Observable) so + that retryable errors are retried up to `:max-retries` times with + exponential back-off. Non-retryable errors propagate immediately. + + Accepts an optional `config` map with: + :max-retries – maximum number of retries (default 3) + :base-delay-ms – base delay in ms; doubles each attempt (default 1000)" + ([observable-fn] + (with-retry observable-fn default-retry-config)) + ([observable-fn config] + (with-retry observable-fn config 0)) + ([observable-fn config attempt] + (let [{:keys [max-retries base-delay-ms]} (merge default-retry-config config)] + (->> (observable-fn) + (rx/catch + (fn [cause] + (if (and (retryable-error? cause) + (< attempt max-retries)) + ;; bit-shift-left 1 N is equivalent to 2^N: shift the bits of the + ;; number 1 to the left N positions (e.g. 1 -> 2 -> 4 -> 8 -> 16), + ;; producing exponential backoff delays of 1x, 2x, 4x, 8x, 16x. + (let [delay-ms (* base-delay-ms (bit-shift-left 1 attempt))] + (log/wrn :hint "retrying request" + :attempt (inc attempt) + :delay delay-ms + :error (ex-message cause)) + (->> (rx/timer delay-ms) + (rx/mapcat (fn [_] (with-retry observable-fn config (inc attempt)))))) + (rx/throw cause)))))))) + +;; -- Response handling ------------------------------------------------------- + (defn handle-response [{:keys [status body headers uri] :as response}] (cond @@ -146,32 +201,41 @@ (log/trc :hint "make request" :id id) - (->> (http/fetch request) - (rx/map http/response->map) - (rx/mapcat (fn [{:keys [headers body] :as response}] - (log/trc :hint "response received" :id id :elapsed (tpoint)) + (let [make-request + (fn [] + (->> (http/fetch request) + (rx/map http/response->map) + (rx/mapcat (fn [{:keys [headers body] :as response}] + (log/trc :hint "response received" :id id :elapsed (tpoint)) - (let [ctype (get headers "content-type") - response-stream? (str/starts-with? ctype "text/event-stream") - tpoint (ct/tpoint-ms)] + (let [ctype (get headers "content-type") + response-stream? (str/starts-with? ctype "text/event-stream") + tpoint (ct/tpoint-ms)] - (when (and response-stream? (not stream?)) - (ex/raise :type :assertion - :code :unexpected-response - :hint "expected normal response, received sse stream" - :uri (:uri response) - :status (:status response))) + (when (and response-stream? (not stream?)) + (ex/raise :type :assertion + :code :unexpected-response + :hint "expected normal response, received sse stream" + :uri (:uri response) + :status (:status response))) - (if response-stream? - (-> (sse/create-stream body) - (sse/read-stream t/decode-str)) + (if response-stream? + (-> (sse/create-stream body) + (sse/read-stream t/decode-str)) - (->> response - (http/process-response-type response-type) - (rx/map decode-fn) - (rx/tap (fn [_] - (log/trc :hint "response decoded" :id id :elapsed (tpoint)))) - (rx/mapcat handle-response))))))))) + (->> response + (http/process-response-type response-type) + (rx/map decode-fn) + (rx/tap (fn [_] + (log/trc :hint "response decoded" :id id :elapsed (tpoint)))) + (rx/mapcat handle-response))))))))] + + ;; Idempotent (GET) requests are automatically retried on + ;; transient network / server errors. Mutations are never + ;; retried to avoid unintended side-effects. + (if (= :get method) + (with-retry make-request) + (make-request))))) (defmulti cmd! (fn [id _] id)) diff --git a/frontend/src/app/util/http.cljs b/frontend/src/app/util/http.cljs index 8b9c590895..f2810d4ac0 100644 --- a/frontend/src/app/util/http.cljs +++ b/frontend/src/app/util/http.cljs @@ -108,8 +108,7 @@ (vreset! abortable? false) (when-not (or @unsubscribed? (= (.-name ^js cause) "AbortError")) (let [error (ex-info (ex-message cause) - {:type :internal - :code :fetch-error + {:type :network :hint "unable to perform fetch operation" :uri uri :headers headers} diff --git a/frontend/test/frontend_tests/data/repo_test.cljs b/frontend/test/frontend_tests/data/repo_test.cljs new file mode 100644 index 0000000000..d4ac101086 --- /dev/null +++ b/frontend/test/frontend_tests/data/repo_test.cljs @@ -0,0 +1,217 @@ +;; This Source Code Form is subject to the terms of the Mozilla Public +;; License, v. 2.0. If a copy of the MPL was not distributed with this +;; file, You can obtain one at http://mozilla.org/MPL/2.0/. +;; +;; Copyright (c) KALEIDOS INC + +(ns frontend-tests.data.repo-test + (:require + [app.main.repo :as repo] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true])) + +;; --------------------------------------------------------------------------- +;; retryable-error? tests (synchronous) +;; --------------------------------------------------------------------------- + +(t/deftest retryable-error-network + (t/testing "network error (js/fetch failure) is retryable" + (let [err (ex-info "network" {:type :network})] + (t/is (true? (repo/retryable-error? err)))))) + +(t/deftest retryable-error-bad-gateway + (t/testing "502 bad-gateway is retryable" + (let [err (ex-info "bad gateway" {:type :bad-gateway})] + (t/is (true? (repo/retryable-error? err)))))) + +(t/deftest retryable-error-service-unavailable + (t/testing "503 service-unavailable is retryable" + (let [err (ex-info "service unavailable" {:type :service-unavailable})] + (t/is (true? (repo/retryable-error? err)))))) + +(t/deftest retryable-error-offline + (t/testing "offline (status 0) is retryable" + (let [err (ex-info "offline" {:type :offline})] + (t/is (true? (repo/retryable-error? err)))))) + +(t/deftest retryable-error-internal + (t/testing "internal error (genuine bug) is NOT retryable" + (let [err (ex-info "internal" {:type :internal :code :something})] + (t/is (not (repo/retryable-error? err)))))) + +(t/deftest retryable-error-validation + (t/testing "validation error is NOT retryable" + (let [err (ex-info "validation" {:type :validation :code :request-body-too-large})] + (t/is (not (repo/retryable-error? err)))))) + +(t/deftest retryable-error-authentication + (t/testing "authentication error is NOT retryable" + (let [err (ex-info "auth" {:type :authentication})] + (t/is (not (repo/retryable-error? err)))))) + +(t/deftest retryable-error-authorization + (t/testing "authorization/challenge error is NOT retryable" + (let [err (ex-info "auth" {:type :authorization :code :challenge-required})] + (t/is (not (repo/retryable-error? err)))))) + +(t/deftest retryable-error-no-ex-data + (t/testing "plain error without ex-data is NOT retryable" + (let [err (js/Error. "plain")] + (t/is (not (repo/retryable-error? err)))))) + +;; --------------------------------------------------------------------------- +;; with-retry tests (async, using zero-delay config for speed) +;; --------------------------------------------------------------------------- + +(def ^:private fast-config + "Retry config with zero delay for fast tests." + {:max-retries 3 :base-delay-ms 0}) + +(t/deftest with-retry-succeeds-immediately + (t/testing "returns value when observable succeeds on first try" + (t/async done + (let [call-count (atom 0) + obs-fn (fn [] + (swap! call-count inc) + (rx/of :ok))] + (->> (repo/with-retry obs-fn fast-config) + (rx/subs! + (fn [val] + (t/is (= :ok val)) + (t/is (= 1 @call-count)) + (done)) + (fn [err] + (t/is false (str "unexpected error: " (ex-message err))) + (done)))))))) + +(t/deftest with-retry-retries-on-retryable-error + (t/testing "retries and eventually succeeds after transient failures" + (t/async done + (let [call-count (atom 0) + obs-fn (fn [] + (let [n (swap! call-count inc)] + (if (< n 3) + ;; First two calls fail with retryable error + (rx/throw (ex-info "bad gateway" {:type :bad-gateway})) + ;; Third call succeeds + (rx/of :recovered))))] + (->> (repo/with-retry obs-fn fast-config) + (rx/subs! + (fn [val] + (t/is (= :recovered val)) + (t/is (= 3 @call-count)) + (done)) + (fn [err] + (t/is false (str "unexpected error: " (ex-message err))) + (done)))))))) + +(t/deftest with-retry-exhausts-retries + (t/testing "propagates error after max retries exhausted" + (t/async done + (let [call-count (atom 0) + obs-fn (fn [] + (swap! call-count inc) + (rx/throw (ex-info "offline" {:type :offline})))] + (->> (repo/with-retry obs-fn fast-config) + (rx/subs! + (fn [_val] + (t/is false "should not succeed") + (done)) + (fn [err] + ;; 1 initial + 3 retries = 4 total calls + (t/is (= 4 @call-count)) + (t/is (= :offline (:type (ex-data err)))) + (done)))))))) + +(t/deftest with-retry-no-retry-on-non-retryable + (t/testing "non-retryable errors propagate immediately without retry" + (t/async done + (let [call-count (atom 0) + obs-fn (fn [] + (swap! call-count inc) + (rx/throw (ex-info "auth" {:type :authentication})))] + (->> (repo/with-retry obs-fn fast-config) + (rx/subs! + (fn [_val] + (t/is false "should not succeed") + (done)) + (fn [err] + (t/is (= 1 @call-count)) + (t/is (= :authentication (:type (ex-data err)))) + (done)))))))) + +(t/deftest with-retry-network-error-retried + (t/testing "network error (js/fetch failure) is retried" + (t/async done + (let [call-count (atom 0) + obs-fn (fn [] + (let [n (swap! call-count inc)] + (if (= n 1) + (rx/throw (ex-info "net" {:type :network})) + (rx/of :ok))))] + (->> (repo/with-retry obs-fn fast-config) + (rx/subs! + (fn [val] + (t/is (= :ok val)) + (t/is (= 2 @call-count)) + (done)) + (fn [err] + (t/is false (str "unexpected error: " (ex-message err))) + (done)))))))) + +(t/deftest with-retry-internal-not-retried + (t/testing "internal error (genuine bug) is not retried" + (t/async done + (let [call-count (atom 0) + obs-fn (fn [] + (swap! call-count inc) + (rx/throw (ex-info "bug" {:type :internal + :code :something})))] + (->> (repo/with-retry obs-fn fast-config) + (rx/subs! + (fn [_val] + (t/is false "should not succeed") + (done)) + (fn [err] + (t/is (= 1 @call-count)) + (t/is (= :internal (:type (ex-data err)))) + (done)))))))) + +(t/deftest with-retry-respects-max-retries-config + (t/testing "respects custom max-retries setting" + (t/async done + (let [call-count (atom 0) + config {:max-retries 1 :base-delay-ms 0} + obs-fn (fn [] + (swap! call-count inc) + (rx/throw (ex-info "offline" {:type :offline})))] + (->> (repo/with-retry obs-fn config) + (rx/subs! + (fn [_val] + (t/is false "should not succeed") + (done)) + (fn [err] + ;; 1 initial + 1 retry = 2 total + (t/is (= 2 @call-count)) + (t/is (= :offline (:type (ex-data err)))) + (done)))))))) + +(t/deftest with-retry-mixed-errors + (t/testing "retries retryable errors, then stops on non-retryable" + (t/async done + (let [call-count (atom 0) + obs-fn (fn [] + (let [n (swap! call-count inc)] + (case n + 1 (rx/throw (ex-info "gw" {:type :bad-gateway})) + 2 (rx/throw (ex-info "auth" {:type :authentication})) + (rx/of :should-not-reach))))] + (->> (repo/with-retry obs-fn fast-config) + (rx/subs! + (fn [_val] + (t/is false "should not succeed") + (done)) + (fn [err] + (t/is (= 2 @call-count)) + (t/is (= :authentication (:type (ex-data err)))) + (done)))))))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index ad84056110..b39ba239d5 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -2,6 +2,7 @@ (:require [cljs.test :as t] [frontend-tests.basic-shapes-test] + [frontend-tests.data.repo-test] [frontend-tests.data.workspace-colors-test] [frontend-tests.helpers-shapes-test] [frontend-tests.logic.comp-remove-swap-slots-test] @@ -35,6 +36,7 @@ [] (t/run-tests 'frontend-tests.basic-shapes-test + 'frontend-tests.data.repo-test 'frontend-tests.data.workspace-colors-test 'frontend-tests.helpers-shapes-test 'frontend-tests.logic.comp-remove-swap-slots-test