mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 22:10:34 +00:00
🐛 Report raw fetch failures as connectivity errors (#12624)
* 🐛 Report raw fetch failures as connectivity errors A fetch that fails outside app.util.http (zip.js, plugins, schema loads) rejects with a bare TypeError such as "Failed to fetch". It reached the global handlers and was reported as an unhandled crash, with no stack and no URL. The global handlers now wrap the browser's network TypeErrors as :network (or :offline when the browser says so), so they get the connection toast and a compact handled report. Response body reads in app.util.http also wrap a TypeError as :network with the request uri, as fetch itself already did. Relates to #12581 AI-assisted-by: claude-opus-5-5 * 🐛 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
3cc91e8ba2
commit
a3fe43c1ec
@ -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,6 +1124,15 @@
|
||||
(and (= (.-name ^js cause) "NotFoundError")
|
||||
(str/includes? message "removeChild")))))
|
||||
|
||||
(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."
|
||||
[cause]
|
||||
(ex-info (ex-message cause)
|
||||
{:type (if (false? (.-onLine ^js g/navigator)) :offline :network)
|
||||
:hint "unable to perform fetch operation"}
|
||||
cause))
|
||||
|
||||
(defn- from-plugin?
|
||||
"Check if the error is marked as originating from plugin code. The
|
||||
plugin runtime tracks plugin errors in a WeakMap, which works even
|
||||
@ -1149,6 +1159,10 @@
|
||||
(is-ignorable-exception? cause)
|
||||
nil
|
||||
|
||||
;; Raw fetch failures: connectivity, not a crash
|
||||
(http/network-failure? cause)
|
||||
(on-error (network-failure->error cause))
|
||||
|
||||
;; All other errors: show exception page
|
||||
:else
|
||||
|
||||
@ -1176,6 +1190,10 @@
|
||||
(is-ignorable-exception? cause)
|
||||
nil
|
||||
|
||||
;; Raw fetch failures: connectivity, not a crash
|
||||
(http/network-failure? cause)
|
||||
(on-error (network-failure->error cause))
|
||||
|
||||
;; All other errors: show exception page
|
||||
:else
|
||||
(let [data (ex-data cause)
|
||||
|
||||
@ -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)
|
||||
@ -156,7 +176,19 @@
|
||||
:blob (.blob ^js native-response))]
|
||||
(->> (rx/from body)
|
||||
(rx/map (fn [body]
|
||||
(assoc response :body body))))))
|
||||
(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. Other errors, such as parse failures
|
||||
;; (SyntaxError), pass as is.
|
||||
(rx/catch (fn [cause]
|
||||
(if (network-failure? cause)
|
||||
(rx/throw (ex-info (ex-message cause)
|
||||
{:type :network
|
||||
:hint "unable to read response body"
|
||||
:uri (:uri response)}
|
||||
cause))
|
||||
(rx/throw cause)))))))
|
||||
|
||||
(defn send!
|
||||
[{:keys [response-type] :or {response-type :text} :as params}]
|
||||
|
||||
@ -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
|
||||
@ -148,3 +149,33 @@
|
||||
;; V8 gives native rejections such as a failed fetch no frames at all.
|
||||
(let [cause (make-error "TypeError" "Failed to fetch" :stack "TypeError: Failed to fetch")]
|
||||
(t/is (false? (errors/is-ignorable-exception? cause))))))
|
||||
|
||||
;; ---------------------------------------------------------------------------
|
||||
;; network-failure->error tests
|
||||
;; ---------------------------------------------------------------------------
|
||||
|
||||
(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 (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))))))
|
||||
|
||||
@ -132,6 +132,7 @@
|
||||
[frontend-tests.ui.stroke-menu-test]
|
||||
[frontend-tests.ui.workspace-libraries-test]
|
||||
[frontend-tests.util-clipboard-test]
|
||||
[frontend-tests.util-http-test]
|
||||
[frontend-tests.util-object-test]
|
||||
[frontend-tests.util-queue-test]
|
||||
[frontend-tests.util-range-tree-test]
|
||||
@ -283,6 +284,7 @@
|
||||
'frontend-tests.ui.stroke-menu-test
|
||||
'frontend-tests.ui.workspace-libraries-test
|
||||
'frontend-tests.util-clipboard-test
|
||||
'frontend-tests.util-http-test
|
||||
'frontend-tests.util-object-test
|
||||
'frontend-tests.util-queue-test
|
||||
'frontend-tests.util-range-tree-test
|
||||
|
||||
75
frontend/test/frontend_tests/util_http_test.cljs
Normal file
75
frontend/test/frontend_tests/util_http_test.cljs
Normal file
@ -0,0 +1,75 @@
|
||||
;; 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 SUBSIDIARY SL
|
||||
|
||||
(ns frontend-tests.util-http-test
|
||||
(:require
|
||||
[app.util.http :as http]
|
||||
[cljs.test :as t :include-macros true]
|
||||
[frontend-tests.helpers.async :as async]))
|
||||
|
||||
(def ^:private uri "https://example.com/assets/file.json")
|
||||
|
||||
(defn- failing-response
|
||||
"A response map whose native body read rejects with `cause`."
|
||||
[cause]
|
||||
{:status 200
|
||||
:uri uri
|
||||
:headers {}
|
||||
::http/response #js {:text (fn [] (js/Promise.reject cause))}})
|
||||
|
||||
(defn- ^:async read-error
|
||||
"Read the body of `response` as text and resolve with the error raised."
|
||||
[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")
|
||||
error (await (read-error (failing-response cause)))
|
||||
data (ex-data error)]
|
||||
(t/is (= :network (:type data)))
|
||||
(t/is (= uri (:uri data)))
|
||||
(t/is (identical? cause (ex-cause error)))))
|
||||
|
||||
(t/deftest ^:async body-parse-failure-passes-unchanged
|
||||
(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