diff --git a/frontend/src/app/main/errors.cljs b/frontend/src/app/main/errors.cljs index 6c0fc30666..5125730363 100644 --- a/frontend/src/app/main/errors.cljs +++ b/frontend/src/app/main/errors.cljs @@ -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 diff --git a/frontend/src/app/util/http.cljs b/frontend/src/app/util/http.cljs index aec75d9f41..97638f7231 100644 --- a/frontend/src/app/util/http.cljs +++ b/frontend/src/app/util/http.cljs @@ -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" diff --git a/frontend/test/frontend_tests/errors_test.cljs b/frontend/test/frontend_tests/errors_test.cljs index e8ba2bd228..baa5da1f96 100644 --- a/frontend/test/frontend_tests/errors_test.cljs +++ b/frontend/test/frontend_tests/errors_test.cljs @@ -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)))))) diff --git a/frontend/test/frontend_tests/util_http_test.cljs b/frontend/test/frontend_tests/util_http_test.cljs index b20df426d7..e686d4d3a9 100644 --- a/frontend/test/frontend_tests/util_http_test.cljs +++ b/frontend/test/frontend_tests/util_http_test.cljs @@ -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"))))))