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

🐛 Classify the responses the backend never wrote

Browse Source
This commit is contained in:
alonso.torres 2026-09-24 10:10:02 +02:00
parent dd163a696d
commit 67466d7406
4 changed files with 235 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -498,6 +498,15 @@
:type :handled
:hint (tr "errors.connection-error")))
(defn- handle-transport-error
"Reports a failure whose answer did not come from the backend. These affect
one request and pass on their own, so the user keeps the page and gets a
toast."
[error prefix]
(when-let [cause (::instance error)]
(ex/print-throwable cause :prefix prefix))
(flash :cause (::instance error) :type :handled))
(defmethod ptk/handle-error :network
[error]
;; Transient network errors (e.g. lost connectivity, DNS failure)
@ -511,6 +520,18 @@
;; that would report it as an unhandled application error.
(handle-connectivity-error error "Offline Error"))
(defmethod ptk/handle-error :gateway-error
[error]
(handle-connectivity-error error "Gateway Error"))
(defmethod ptk/handle-error :rate-limit
[error]
(handle-transport-error error "Rate Limit Error"))
(defmethod ptk/handle-error :unexpected-response
[error]
(handle-transport-error error "Unexpected Response"))
(def ^:private delegated-persistence-types
"Save failure causes routed to their own error handler: retaining the
changes cannot resolve them."

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

@ -32,6 +32,7 @@
#{:network ; js/fetch network-level failure
:bad-gateway ; 502
:service-unavailable ; 503
:gateway-error ; 504 and the Cloudflare 52x family
:offline}) ; status 0 (browser offline)
(defn retryable-error?
@ -79,6 +80,28 @@
;; -- Response handling -------------------------------------------------------
(def ^:private gateway-statuses
"Statuses raised by an edge proxy when the request never reached the
backend or its answer never came back."
#{504 ; gateway timeout
520 ; cloudflare: unknown origin error
521 ; cloudflare: origin is down
522 ; cloudflare: connection timed out
523 ; cloudflare: origin unreachable
524}) ; cloudflare: origin timed out
(def ^:private max-body-excerpt
"Number of characters kept from a body that we could not interpret."
500)
(defn- body-excerpt
"Keeps the head of an uninterpretable body. Error pages from proxies run
into tens of kilobytes and the excerpt is enough to recognize them."
[body]
(if (and (string? body) (> (count body) max-body-excerpt))
(str (subs body 0 max-body-excerpt) "...")
body))
(defn handle-response
[{:keys [status body headers uri] :as response}]
(cond
@ -121,6 +144,35 @@
(and (>= status 400) (map? body))
(rx/throw (ex-info "http error" (assoc body :uri uri :status status)))
;; The branches below handle responses the backend never wrote: the
;; body carries no error code, so the status is all we can read.
(contains? gateway-statuses status)
(rx/throw
(ex/error :type :gateway-error
:code :gateway-error
:hint "the request did not reach the backend"
:uri uri
:status status))
(= 429 status)
(rx/throw
(ex/error :type :rate-limit
:code :rate-limited
:hint "too many requests"
:uri uri
:status status))
(>= status 400)
(rx/throw
(ex/error :type :unexpected-response
:code :unexpected-error-response
:hint "unexpected error response"
:uri uri
:status status
:headers headers
:data (body-excerpt body)))
:else
(rx/throw
(ex/error :type :internal
@ -129,7 +181,7 @@
:uri uri
:status status
:headers headers
:data body))))
:data (body-excerpt body)))))
(def default-options
{:update-file {:query-params [:id]}

128
frontend/test/frontend_tests/data/repo_test.cljs
View File

@ -8,7 +8,8 @@
(:require
[app.main.repo :as repo]
[beicon.v2.core :as rx]
[cljs.test :as t :include-macros true]))
[cljs.test :as t :include-macros true]
[cuerdas.core :as str]))
;; ---------------------------------------------------------------------------
;; retryable-error? tests (synchronous)
@ -29,6 +30,11 @@
(let [err (ex-info "service unavailable" {:type :service-unavailable})]
(t/is (true? (repo/retryable-error? err))))))
(t/deftest retryable-error-gateway-error
(t/testing "504 and the Cloudflare 52x family are retryable"
(let [err (ex-info "http error" {:type :gateway-error})]
(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})]
@ -215,3 +221,123 @@
(t/is (= 2 @call-count))
(t/is (= :authentication (:type (ex-data err))))
(done))))))))
;; ---------------------------------------------------------------------------
;; handle-response classification tests
;; ---------------------------------------------------------------------------
(defn- capture-error
"Subscribes to `handle-response` for `response` and calls `k` with the
`ex-data` of the raised error."
[response k]
(->> (repo/handle-response response)
(rx/subs!
(fn [_] (t/is false "expected an error response") (k nil))
(fn [cause] (k (ex-data cause))))))
(def ^:private html-error-page
"<html><head><title>Web Page Blocked</title></head><body>blocked</body></html>")
(t/deftest handle-response-gateway-statuses
(t/testing "every edge status that never reached the backend is a gateway error"
(t/async done
(let [statuses [504 520 521 522 523 524]
pending (atom (count statuses))]
(doseq [status statuses]
(capture-error {:status status
:body html-error-page
:headers {"content-type" "text/html"}
:uri "https://design.penpot.app/api/main/methods/get-teams"}
(fn [data]
(t/is (= :gateway-error (:type data))
(str "status " status " should be a gateway error"))
(t/is (= status (:status data))
(str "status " status " should be carried through"))
(when (zero? (swap! pending dec))
(done)))))))))
(t/deftest handle-response-rate-limit
(t/testing "a 429 with no interpretable body is a rate-limit error"
(t/async done
(capture-error {:status 429
:body ""
:headers {}
:uri "https://design.penpot.app/api/main/methods/get-teams"}
(fn [data]
(t/is (= :rate-limit (:type data)))
(t/is (= 429 (:status data)))
(done))))))
(t/deftest handle-response-intercepted-by-proxy
(t/testing "an error page from a filtering proxy is not an internal error"
(t/async done
(capture-error {:status 403
:body html-error-page
:headers {"content-type" "text/html"
"server" "Zscaler/6.2"}
:uri "https://design.penpot.app/api/main/methods/update-file"}
(fn [data]
(t/is (= :unexpected-response (:type data)))
(t/is (= 403 (:status data)))
(done))))))
(t/deftest handle-response-keeps-body-excerpt
(t/testing "a body too long to keep is cut short and marked as cut"
(t/async done
(capture-error {:status 403
:body (apply str (repeat 5000 "x"))
:headers {"content-type" "text/html"}
:uri "https://design.penpot.app/api/main/methods/update-file"}
(fn [data]
(t/is (= 503 (count (:data data))) "500 characters and an ellipsis")
(t/is (str/ends-with? (:data data) "..."))
(done))))))
(t/deftest handle-response-keeps-a-short-body-whole
(t/testing "a body short enough to read is passed through untouched"
(t/async done
(capture-error {:status 403
:body html-error-page
:headers {"content-type" "text/html"}
:uri "https://design.penpot.app/api/main/methods/update-file"}
(fn [data]
(t/is (= html-error-page (:data data)))
(done))))))
(t/deftest handle-response-challenge-still-authorization
(t/testing "a Cloudflare challenge keeps its own type"
(t/async done
(capture-error {:status 403
:body html-error-page
:headers {"server" "cloudflare"
"cf-mitigated" "challenge"}
:uri "https://design.penpot.app/api/main/methods/get-teams"}
(fn [data]
(t/is (= :authorization (:type data)))
(t/is (= :challenge-required (:code data)))
(done))))))
(t/deftest handle-response-keeps-backend-error
(t/testing "an error the backend itself reported is passed through untouched"
(t/async done
(capture-error {:status 400
:body {:type :validation :code :missing-param}
:headers {"content-type" "application/transit+json"}
:uri "https://design.penpot.app/api/main/methods/update-file"}
(fn [data]
(t/is (= :validation (:type data)))
(t/is (= :missing-param (:code data)))
(t/is (= 400 (:status data)))
(done))))))
(t/deftest handle-response-undecodable-non-error-is-internal
(t/testing "a non-error response we cannot interpret is still an internal error"
(t/async done
(capture-error {:status 301
:body html-error-page
:headers {"content-type" "text/html"}
:uri "https://design.penpot.app/api/main/methods/get-teams"}
(fn [data]
(t/is (= :internal (:type data)))
(t/is (= :unable-to-process-repository-response (:code data)))
(done))))))

36
frontend/test/frontend_tests/main_errors_test.cljs
View File

@ -141,7 +141,7 @@
(t/deftest environment-error-classification
(t/testing "environment failures are recognised"
(doseq [type [:network :offline :bad-gateway :service-unavailable
:nitrate-unavailable :nitrate-not-configured]]
:gateway-error :nitrate-unavailable :nitrate-not-configured]]
(t/is (true? (errors/environment-error? (error-cause :type type)))
(str "expected environment error: " type))))
(t/testing "application defects are not environment failures"
@ -192,7 +192,7 @@
;; handler. Each is reported as a governed compact audit event plus its
;; connection toast. Proves: per type, report first and toast right after,
;; with no stack in the report and the dedicated toast message.
(doseq [type [:network :offline]]
(doseq [type [:network :offline :gateway-error]]
(errors/reset-report-governor!)
(let [events (atom [])
cause (ex-info "http error" {:type type :hint "http error"})]
@ -786,3 +786,35 @@
(let [state (ptk/update (last @events) {})]
(t/is (nil? (get-in state [:notification :timeout])))
(t/is (some? (get-in state [:notification :links 0])))))))))
;; ---------------------------------------------------------------------------
;; transport errors
;; ---------------------------------------------------------------------------
(t/deftest ^:async transport-errors-are-handled-without-replacing-the-page
;; Scenario: a rate limit and an error page the backend did not write,
;; through the global handler. Proves: neither replaces the page, each is
;; a handled report followed by one toast.
(doseq [type [:rate-limit :unexpected-response]]
(errors/reset-report-governor!)
(let [events (atom [])
assigned (atom [])
cause (ex-info "http error"
{:type type
:hint "http error"
:status 429
:uri "https://design.penpot.app/api/main/methods/get-teams"})]
(await
(mock/with-mocks*
{st/emit! (mock/stub (fn [& emitted] (swap! events into emitted)))
rt/get-current-href (constantly "https://penpot.example.com/#/workspace")
rt/assign-exception (fn [error] (swap! assigned conj error) error)
tm/schedule (mock/stub (fn [f] (f)))
st/format-last-events (mock/stub (fn [& _] "(stub last events)"))}
(errors/on-error cause)
(await (async/settle))
(t/is (empty? @assigned)
(str type " must not replace the page with an error screen"))
(t/is (= ["handled-exception"]
(map (comp ::ev/name deref) (report-events @events)))
(str type " is reported as handled, not unhandled"))
(t/is (= 2 (count @events)) (str type " must show one notification")))))))