mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 05:50:35 +00:00
🐛 Treat only network TypeErrors on body read as connectivity errors
process-response-type turned every TypeError from a body read into a :network error, so a bug such as "body stream already read" showed a connection toast. It now uses network-failure?, which moves to app.util.http so that both the http client and the global error handlers share it. Add Firefox's "Error in body stream" message. Tests now check :offline and :network separately by setting navigator.onLine, and read-error builds on async/->promise. AI-assisted-by: claude-opus-5-5
This commit is contained in:
parent
933d5e31f2
commit
988b8606f7
@ -22,6 +22,7 @@
|
||||
[app.main.worker]
|
||||
[app.util.dom :as dom]
|
||||
[app.util.globals :as g]
|
||||
[app.util.http :as http]
|
||||
[app.util.i18n :refer [tr]]
|
||||
[app.util.timers :as ts]
|
||||
[app.util.webapi :as wapi]
|
||||
@ -1123,26 +1124,6 @@
|
||||
(and (= (.-name ^js cause) "NotFoundError")
|
||||
(str/includes? message "removeChild")))))
|
||||
|
||||
;; Messages of the TypeError a failed `fetch` rejects with, or a failed
|
||||
;; read of its response body.
|
||||
(def ^:private network-failure-messages
|
||||
#{"Failed to fetch" ; Chrome
|
||||
"NetworkError when attempting to fetch resource." ; Firefox
|
||||
"Load failed" ; Safari
|
||||
"network error"}) ; Chrome, body read
|
||||
|
||||
(defn network-failure?
|
||||
"True when the error is the browser's TypeError for a request that never
|
||||
got an answer. `app.util.http` wraps its own failures as `:network`; this
|
||||
catches the raw ones from third-party code (zip.js, plugins, schema
|
||||
loads) that reach the global handlers."
|
||||
[cause]
|
||||
(and (= (.-name ^js cause) "TypeError")
|
||||
(let [message (ex-message cause)]
|
||||
(or (contains? network-failure-messages message)
|
||||
;; Newer Chrome versions append the host in parentheses
|
||||
(str/starts-with? message "Failed to fetch (")))))
|
||||
|
||||
(defn network-failure->error
|
||||
"Wrap a raw network TypeError as a connectivity error, so it is reported
|
||||
as an environment failure instead of an application crash."
|
||||
@ -1179,7 +1160,7 @@
|
||||
nil
|
||||
|
||||
;; Raw fetch failures: connectivity, not a crash
|
||||
(network-failure? cause)
|
||||
(http/network-failure? cause)
|
||||
(on-error (network-failure->error cause))
|
||||
|
||||
;; All other errors: show exception page
|
||||
@ -1210,7 +1191,7 @@
|
||||
nil
|
||||
|
||||
;; Raw fetch failures: connectivity, not a crash
|
||||
(network-failure? cause)
|
||||
(http/network-failure? cause)
|
||||
(on-error (network-failure->error cause))
|
||||
|
||||
;; All other errors: show exception page
|
||||
|
||||
@ -146,6 +146,26 @@
|
||||
:body (.-body ^js response)
|
||||
::response response})
|
||||
|
||||
;; Messages of the TypeError a failed `fetch` rejects with, or a failed
|
||||
;; read of its response body.
|
||||
(def ^:private network-failure-messages
|
||||
#{"Failed to fetch" ; Chrome
|
||||
"NetworkError when attempting to fetch resource." ; Firefox
|
||||
"Load failed" ; Safari
|
||||
"network error" ; Chrome, body read
|
||||
"Error in body stream"}) ; Firefox, body read
|
||||
|
||||
(defn network-failure?
|
||||
"True when the error is the browser's TypeError for a request that never
|
||||
got an answer. Other TypeErrors, such as reading a body twice, are bugs
|
||||
and do not match."
|
||||
[cause]
|
||||
(and (= (.-name ^js cause) "TypeError")
|
||||
(let [message (ex-message cause)]
|
||||
(or (contains? network-failure-messages message)
|
||||
;; Newer Chrome versions append the host in parentheses
|
||||
(str/starts-with? message "Failed to fetch (")))))
|
||||
|
||||
(defn process-response-type
|
||||
[response-type response]
|
||||
(let [native-response (::response response)
|
||||
@ -159,9 +179,10 @@
|
||||
(assoc response :body body)))
|
||||
;; The connection can drop while the body streams in, after
|
||||
;; `fetch` already resolved; the browser then rejects the read
|
||||
;; with a bare TypeError. Parse failures (SyntaxError) pass as is.
|
||||
;; with a bare TypeError. Other errors, such as parse failures
|
||||
;; (SyntaxError), pass as is.
|
||||
(rx/catch (fn [cause]
|
||||
(if (= (.-name ^js cause) "TypeError")
|
||||
(if (network-failure? cause)
|
||||
(rx/throw (ex-info (ex-message cause)
|
||||
{:type :network
|
||||
:hint "unable to read response body"
|
||||
|
||||
@ -7,6 +7,7 @@
|
||||
(ns frontend-tests.errors-test
|
||||
(:require
|
||||
[app.main.errors :as errors]
|
||||
[app.util.globals :as g]
|
||||
[cljs.test :as t :include-macros true]))
|
||||
|
||||
(defn- make-error
|
||||
@ -150,30 +151,31 @@
|
||||
(t/is (false? (errors/is-ignorable-exception? cause))))))
|
||||
|
||||
;; ---------------------------------------------------------------------------
|
||||
;; network-failure? tests
|
||||
;; network-failure->error tests
|
||||
;; ---------------------------------------------------------------------------
|
||||
|
||||
(t/deftest test-network-failure-browser-messages
|
||||
(t/testing "The TypeError each browser gives a failed fetch is a network failure"
|
||||
(doseq [message ["Failed to fetch"
|
||||
"Failed to fetch (design.penpot.app)"
|
||||
"NetworkError when attempting to fetch resource."
|
||||
"Load failed"
|
||||
"network error"]]
|
||||
(t/is (true? (errors/network-failure? (make-error "TypeError" message)))
|
||||
message))))
|
||||
|
||||
(t/deftest test-network-failure-needs-type-error
|
||||
(t/testing "Other errors with the same message are not network failures"
|
||||
(t/is (false? (errors/network-failure? (make-error "Error" "Failed to fetch")))))
|
||||
(t/testing "Other TypeErrors are not network failures"
|
||||
(t/is (false? (errors/network-failure? (make-error "TypeError" "x is not a function"))))))
|
||||
(defn- with-online
|
||||
"Call `f` while `navigator.onLine` reads `online?`."
|
||||
[online? f]
|
||||
(js/Object.defineProperty g/navigator "onLine"
|
||||
#js {:value online? :configurable true})
|
||||
(try
|
||||
(f)
|
||||
(finally
|
||||
(js-delete g/navigator "onLine"))))
|
||||
|
||||
(t/deftest test-network-failure-becomes-environment-error
|
||||
(t/testing "A raw fetch failure is wrapped as an environment error"
|
||||
(let [cause (make-error "TypeError" "Failed to fetch")
|
||||
error (errors/network-failure->error cause)]
|
||||
(t/is (contains? #{:network :offline} (:type (ex-data error))))
|
||||
(t/is (true? (errors/environment-error? error)))
|
||||
(t/is (= "Failed to fetch" (ex-message error)))
|
||||
(t/is (identical? cause (ex-cause error))))))
|
||||
|
||||
(t/deftest test-network-failure-type-follows-connection
|
||||
(let [cause (make-error "TypeError" "Failed to fetch")
|
||||
type #(:type (ex-data (errors/network-failure->error cause)))]
|
||||
(t/testing "Offline browsers report :offline"
|
||||
(t/is (= :offline (with-online false type))))
|
||||
(t/testing "Online browsers report :network"
|
||||
(t/is (= :network (with-online true type))))))
|
||||
|
||||
@ -7,8 +7,8 @@
|
||||
(ns frontend-tests.util-http-test
|
||||
(:require
|
||||
[app.util.http :as http]
|
||||
[beicon.v2.core :as rx]
|
||||
[cljs.test :as t :include-macros true]))
|
||||
[cljs.test :as t :include-macros true]
|
||||
[frontend-tests.helpers.async :as async]))
|
||||
|
||||
(def ^:private uri "https://example.com/assets/file.json")
|
||||
|
||||
@ -20,14 +20,14 @@
|
||||
:headers {}
|
||||
::http/response #js {:text (fn [] (js/Promise.reject cause))}})
|
||||
|
||||
(defn- read-error
|
||||
(defn- ^:async read-error
|
||||
"Read the body of `response` as text and resolve with the error raised."
|
||||
[response]
|
||||
(js/Promise.
|
||||
(fn [resolve]
|
||||
(rx/subs! (fn [_] (resolve nil))
|
||||
resolve
|
||||
(http/process-response-type :text response)))))
|
||||
(try
|
||||
(await (async/->promise (http/process-response-type :text response)))
|
||||
nil
|
||||
(catch :default cause
|
||||
cause)))
|
||||
|
||||
(t/deftest ^:async body-read-network-failure-is-a-network-error
|
||||
(let [cause (js/TypeError. "network error")
|
||||
@ -41,3 +41,35 @@
|
||||
(let [cause (js/SyntaxError. "Unexpected token < in JSON")
|
||||
error (await (read-error (failing-response cause)))]
|
||||
(t/is (identical? cause error))))
|
||||
|
||||
(t/deftest ^:async body-read-programming-error-passes-unchanged
|
||||
(let [cause (js/TypeError. "body stream already read")
|
||||
error (await (read-error (failing-response cause)))]
|
||||
(t/is (identical? cause error))))
|
||||
|
||||
;; ---------------------------------------------------------------------------
|
||||
;; network-failure? tests
|
||||
;; ---------------------------------------------------------------------------
|
||||
|
||||
(defn- make-error
|
||||
[error-name message]
|
||||
(let [err (js/Error. message)]
|
||||
(set! (.-name err) error-name)
|
||||
err))
|
||||
|
||||
(t/deftest test-network-failure-browser-messages
|
||||
(t/testing "The TypeError each browser gives a failed fetch is a network failure"
|
||||
(doseq [message ["Failed to fetch"
|
||||
"Failed to fetch (design.penpot.app)"
|
||||
"NetworkError when attempting to fetch resource."
|
||||
"Load failed"
|
||||
"network error"
|
||||
"Error in body stream"]]
|
||||
(t/is (true? (http/network-failure? (make-error "TypeError" message)))
|
||||
message))))
|
||||
|
||||
(t/deftest test-network-failure-needs-type-error
|
||||
(t/testing "Other errors with the same message are not network failures"
|
||||
(t/is (false? (http/network-failure? (make-error "Error" "Failed to fetch")))))
|
||||
(t/testing "Other TypeErrors are not network failures"
|
||||
(t/is (false? (http/network-failure? (make-error "TypeError" "x is not a function"))))))
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user