CalvinBackup/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-03-31 00:29:37 +02:00
Code Issues Packages Projects Releases Wiki Activity

✨ Add retry mechanism for idenpotent get repo requests on frontend (#8792)

Browse Source 
* ♻️ Handle fetch-error gracefully with toast instead of full-page error

Network-level failures (lost connectivity, DNS failure, etc.) on RPC
calls were propagating as :internal/:fetch-error to the global error
handler, which replaced the entire UI with a full-page error screen.

Now the :internal handler distinguishes :fetch-error from other internal
errors and shows a non-intrusive toast notification instead, allowing
the user to continue working.

* ✨ Add automatic retry with backoff for idempotent RPC requests

Idempotent (GET) RPC requests are now automatically retried up to 3
times with exponential back-off (1s, 2s, 4s) when a transient error
occurs.  Retryable errors include: network-level failures
(:fetch-error), 502 Bad Gateway, 503 Service Unavailable, and browser
offline (status 0).

Mutation (POST) requests are never retried to avoid unintended
side-effects.  Non-transient errors (4xx client errors, auth errors,
validation errors) propagate immediately without retry.

* ♻️ Make retry helpers public with configurable parameters

Make retryable-error? and with-retry public functions, and replace
private constants with a default-retry-config map.  with-retry now
accepts an optional config map (:max-retries, :base-delay-ms) enabling
callers and tests to customize retry behavior.

* ✨ Add tests for RPC retry mechanism

Comprehensive tests for the retry helpers in app.main.repo:
- retryable-error? predicate: covers all retryable types (fetch-error,
  bad-gateway, service-unavailable, offline) and non-retryable types
  (validation, authentication, authorization, plain errors)
- with-retry observable wrapper: verifies immediate success, recovery
  after transient failures, max-retries exhaustion, no retry for
  non-retryable errors, fetch-error retry, custom config, and mixed
  error scenarios

* ♻️ Introduce :network error type for fetch-level failures

Replace the awkward {:type :internal :code :fetch-error} combination
with a proper {:type :network} type in app.util.http/fetch.  This makes
the error taxonomy self-explanatory and removes the special-case branch
in the :internal handler.

Consequences:
- http.cljs: emit {:type :network} instead of {:type :internal :code :fetch-error}
- errors.cljs: add a dedicated ptk/handle-error :network method (toast);
  restore :internal handler to its original unconditional full-page error form
- repo.cljs: simplify retryable-types and retryable-error? — :network
  replaces the former :internal special-case, no code check needed
- repo_test.cljs: update tests to use {:type :network}

* 📚 Add comment explaining the use of bit-shift-left
This commit is contained in:
Andrey Antukh
2026-03-27 11:10:26 +01:00
committed by GitHub
parent 8db63c9770
commit d67c7f1c8e
5 changed files with 315 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
frontend/src/app/main/errors.cljs
View File

@@ -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))

108
frontend/src/app/main/repo.cljs
View File

@@ -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))

3
frontend/src/app/util/http.cljs
View File

@@ -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}