From 95e551697fb17518a945899a90ddd1af8e7ef36f Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Wed, 16 Sep 2026 08:22:49 +0000 Subject: [PATCH] :bug: Report environment failures as compact audit events Connectivity and gateway failures (network, offline, 502/503 and nitrate configuration) are not application defects, but offline fell through to :default and 502/503 rendered exception-page, so they reached the internal error reports and alerts with the full payload (stack plus the last events). They are now classified as environment failures and reported as audit-only handled-exception events. generate-report accepts an explicit :format, as keyword arguments or as a trailing map. :compact keeps the context header plus type, code and uri, and skips the stack, the ex-data dump (which may contain request headers) and the last-events list. flash derives the payload format from the cause, so environment failures get a compact report; the audit event name stays the canonical one requested by the caller (handled-exception/unhandled-exception) because external tooling filters on those names. Environment fingerprints drop the stack frame, so grouping does not depend on the internal call site. submit-report now requires an exception cause: a report without one is ignored instead of using a separate fallback fingerprint, so a single fingerprint format governs every report. :offline gets its own handler and both connectivity handlers show the new errors.connection-error message instead of the generic toast. Closes #11743 AI-assisted-by: deepseek-v4.1-flash --- .../frontend/routing-app-shell-subtleties.md | 7 +- .serena/memories/frontend/testing.md | 2 +- frontend/src/app/main/errors.cljs | 178 +++-- frontend/src/app/main/ui/static.cljs | 17 +- .../frontend_tests/data/persistence_test.cljs | 628 ++++++++++-------- .../test/frontend_tests/main_errors_test.cljs | 232 ++++++- frontend/translations/en.po | 4 + 7 files changed, 700 insertions(+), 368 deletions(-) diff --git a/.serena/memories/frontend/routing-app-shell-subtleties.md b/.serena/memories/frontend/routing-app-shell-subtleties.md index 3408c98328..537c9591a9 100644 --- a/.serena/memories/frontend/routing-app-shell-subtleties.md +++ b/.serena/memories/frontend/routing-app-shell-subtleties.md @@ -9,9 +9,10 @@ - The root app renders an exception page from `:exception` state before the normal error boundary. `rt/navigated` clears `:exception`. - Frontend error handling treats stale cross-build JS chunk failures specially: messages containing `$cljs$cst$` or `$cljs$core$I` plus undefined/null/not-a-function signatures trigger throttled reload. - Plugin-originated uncaught errors are identified through the plugin runtime hook and logged rather than turning into the global exception page. -- `app.main.errors/submit-report` is governed by a dedup governor: each report carries a fingerprint (`report-name|type|code|hint|first stack frame`; the report name is part of it so a handled report never coalesces with an unhandled/exception-page one), the first occurrence is always emitted, repeats within 2 minutes are counted and included in the next emitted report as `:occurrences`, and the fingerprint cache is bounded (first-inserted entry evicted, FIFO, via `:order` queue) so memory stays fixed. It applies to `handled-exception`, `unhandled-exception` and `exception-page`. -- `generate-report` is total: if formatting fails it returns a minimal fallback string instead of nil, so an already reserved emission is never dropped. -- `flash` reserves the report before generating it, so suppressed occurrences do not pay the `generate-report` cost; the toast is unchanged. +- `app.main.errors/submit-report` is governed by a dedup governor: each report carries a fingerprint (`report-name|type|code|hint|first stack frame`; the report name is part of it so a handled report never coalesces with an unhandled/exception-page one), the first occurrence is always emitted, repeats within 2 minutes are counted and included in the next emitted report as `:occurrences`, and the fingerprint cache is bounded (first-inserted entry evicted, FIFO, via `:order` queue) so memory stays fixed. It applies to `handled-exception`, `unhandled-exception` and `exception-page`; a report without an exception cause is ignored and does not consume a reservation. +- Errors caused by the environment (`environment-error-types`: `:network`, `:offline`, `:bad-gateway`, `:service-unavailable`, `:nitrate-unavailable`, `:nitrate-not-configured`) are not application defects: they are reported as audit-only `handled-exception` (never `unhandled-exception`/`exception-page`, so they skip internal reports and alerts), with a compact report and a fingerprint that drops the stack frame. `:offline` has its own handler and no longer falls through to `:default`; `:network`/`:offline` show the `errors.connection-error` toast. +- `generate-report` accepts an explicit `{:format :compact|:full}` (default `:full`) chosen by its caller (`flash`, `exception-section*`): `:compact` keeps the context header plus type/code/uri, and skips the stack, the `ex-data` dump and the last-events list. It is total: if formatting fails it returns a minimal fallback string instead of nil, so an already reserved emission is never dropped. +- `flash` reserves the report before generating it, so suppressed occurrences do not pay the `generate-report` cost; the toast is unchanged. `flash` derives only the payload format from the cause (`environment-error?` → `:compact`); the audit event name is the canonical one requested by `:type` (`handled-exception`/`unhandled-exception`) and is never reclassified, because external tools filter on those names. `exception-section*` picks `handled-exception`/`exception-page` explicitly per cause; `flash-persistence` only adds the toast hint. ## Store and websocket diff --git a/.serena/memories/frontend/testing.md b/.serena/memories/frontend/testing.md index 1065d48659..d657214ea5 100644 --- a/.serena/memories/frontend/testing.md +++ b/.serena/memories/frontend/testing.md @@ -6,7 +6,7 @@ Frontend validation: CLJS + React/Rumext + RxJS/Potok; SCSS modules; shared CLJC READ `mem:testing` FIRST — it defines the execution discipline (no piping, tee to file, preferred commands) that applies to all CLJS/JS test runs. -Frontend unit tests live under `frontend/test/frontend_tests/` and use `cljs.test`. They should be deterministic, avoid DOM/UI integration where possible, and mock side effects such as RPC, storage, timers, or network access. +Frontend unit tests live under `frontend/test/frontend_tests/` and use `cljs.test`. They should be deterministic, avoid DOM/UI integration where possible, and mock side effects such as RPC, storage, timers, or network access. Mock through `frontend-tests.helpers.mock`: prefer `mock/with-mocks` (installs with `set!`, so it survives async boundaries) over `with-redefs`. The `:esm` test build dispatches calls to multi-arity vars as `cljs$core$IFn$_invoke$arity$N`, so stub multi-arity vars with `mock/stub` (arities 0-6); for variadic call sites with more than 6 args use a plain variadic `fn` instead. A mock must not call the mocked var again (self-delegation inside a multi-arity function recurses). Async tests wrap the body in `t/async` and thread its `done` into `mock/with-mocks` as the outer callback; `done'` must be called exactly once (calling it twice only prints a warning; not calling it stalls the run and leaks the mocks). From `frontend/`: - Full unit test run (always builds, suppressed output): `pnpm run test:quiet`. diff --git a/frontend/src/app/main/errors.cljs b/frontend/src/app/main/errors.cljs index 460dd59e12..c4f109f6f7 100644 --- a/frontend/src/app/main/errors.cljs +++ b/frontend/src/app/main/errors.cljs @@ -127,35 +127,91 @@ ;; Set the main potok error handler (reset! st/on-error on-error) -(defn generate-report +;; --- Environment failures +;; +;; Failures caused by the user's environment (connectivity, a degraded or +;; misconfigured service) are not application defects. They are reported to the +;; audit log as compact events and must never reach the internal error reports +;; (`unhandled-exception`/`exception-page`), which trigger alerts. + +(def environment-error-types + "Error types produced by the environment rather than by an application + defect." + #{:network + :offline + :bad-gateway + :service-unavailable + :nitrate-unavailable + :nitrate-not-configured}) + +(defn environment-error? [cause] + (contains? environment-error-types (:type (ex-data cause)))) + +(defn- report-context + "Common context header for every report format." + [cause] + (let [team-id (:current-team-id @st/state) + file-id (:current-file-id @st/state) + profile-id (:profile-id @st/state) + data (ex-data cause)] + (with-out-str + (println "Context:") + (println "--------------------") + (println "Timestamp:" (ct/format-inst (ct/now) :rfc1123)) + (println "Hint: " (or (:hint data) (ex-message cause) "--")) + (println "Prof ID: " (str (or profile-id "--"))) + (println "Team ID: " (str (or team-id "--"))) + (when-let [file-id (or (:file-id data) file-id)] + (println "File ID: " (str file-id))) + (println "Version: " (:full cf/version)) + (println "HREF: " (rt/get-current-href))))) + +(defn- generate-full-report + "Complete report: context, formatted throwable (including `ex-data`) and the + last events." + [cause] + (with-out-str + (print (report-context cause)) + (println) + + (println + (ex/format-throwable cause)) + (println) + + (println "Last events:") + (println "--------------------") + (println (st/format-last-events)) + (println))) + +(defn- generate-compact-report + "Reduced report for environment failures: context plus the error type, code + and uri. It skips the stack trace, the `ex-data` dump (which may contain + request headers) and the last-events list." + [cause] + (let [data (ex-data cause)] + (with-out-str + (print (report-context cause)) + (println) + (println "Error:") + (println "--------------------") + (println "Type: " (or (:type data) "--")) + (println "Code: " (or (:code data) "--")) + (when-let [uri (:uri data)] + (println "URI: " uri))))) + +(defn generate-report + "Build the report string for `cause`. + + `:format` selects the payload: `:full` (default) includes the formatted + throwable and the last events; `:compact` keeps only the context and the + error type/code/uri, for environment failures. The option is accepted both + as keyword arguments and as a trailing map." + [cause & {:keys [format] :or {format :full}}] (try - (let [team-id (:current-team-id @st/state) - file-id (:current-file-id @st/state) - profile-id (:profile-id @st/state) - data (ex-data cause)] - - (with-out-str - (println "Context:") - (println "--------------------") - (println "Timestamp:" (ct/format-inst (ct/now) :rfc1123)) - (println "Hint: " (or (:hint data) (ex-message cause) "--")) - (println "Prof ID: " (str (or profile-id "--"))) - (println "Team ID: " (str (or team-id "--"))) - (when-let [file-id (or (:file-id data) file-id)] - (println "File ID: " (str file-id))) - (println "Version: " (:full cf/version)) - (println "HREF: " (rt/get-current-href)) - (println) - - (println - (ex/format-throwable cause)) - (println) - - (println "Last events:") - (println "--------------------") - (println (st/format-last-events)) - (println))) + (case format + :compact (generate-compact-report cause) + (generate-full-report cause)) (catch :default err (.error js/console "error on generating report" err) ;; Keep this function total: `flash` reserves a report slot before @@ -212,23 +268,23 @@ The report name is part of the identity, so a `handled-exception` report never coalesces with an `unhandled-exception`/`exception-page` report of - the same cause (those two do reach the error reports and alerts)." + the same cause (those two do reach the error reports and alerts). + + Environment failures drop the stack frame: their internal call site is an + implementation detail, and keeping it would fragment the grouping." [event-name cause] (let [data (ex-data cause) ftype (or (:type data) :unknown) code (or (:code data) :unknown) hint (or (ex/get-hint cause) "") - ;; A JS stack string starts with "Error: "; the first - ;; actual frame is the second line. - frame (or (some-> (.-stack cause) (str/lines) (second)) "")] - (str (label event-name) "|" (label ftype) "|" (label code) "|" - (str/prune hint 120) "|" (str/prune frame 120)))) - -(defn fallback-fingerprint - "Fingerprint for reports submitted without a `cause` (e.g. the exception - page or a stalled save)." - [event-name hint] - (str (label event-name) "|" (str/prune (or hint "") 120))) + base (str (label event-name) "|" (label ftype) "|" (label code) "|" + (str/prune hint 120))] + (if (environment-error? cause) + base + (let [;; A JS stack string starts with "Error: "; the first + ;; actual frame is the second line. + frame (or (some-> (.-stack cause) (str/lines) (second)) "")] + (str base "|" (str/prune frame 120)))))) (defn- evict-oldest "Drops the fingerprint inserted first. `:order` mirrors the insertion @@ -297,15 +353,18 @@ (defn submit-report "Report the error report to the audit log subsystem, subject to the - report governor." + report governor. + + `cause` must be the exception the report describes: a report without a + cause is ignored (and does not consume a governor reservation), so every + report shares the same fingerprint format." [& {:keys [event-name report hint cause] :or {event-name "unhandled-exception"}}] - (when (and (not (str/empty? hint)) + (when (and (ex/exception? cause) + (not (str/empty? hint)) (string? report) (string? event-name)) - (let [state (reserve-report! (if (ex/exception? cause) - (error-fingerprint event-name cause) - (fallback-fingerprint event-name hint)) + (let [state (reserve-report! (error-fingerprint event-name cause) (inst-ms (ct/now)))] (when (::emit state) (emit-report! event-name report hint (::occurrences state)))))) @@ -322,6 +381,11 @@ "Show error notification banner and emit error report. A nil timeout keeps the notification visible until dismissed or replaced. + The payload format is derived from the cause: environment failures get a + compact report. The audit event name is the canonical one requested by + `:type` (`handled-exception`/`unhandled-exception`); it is an external + contract, so flash never reclassifies it. + The report is reserved before being generated, so repeated errors that fall inside the governor window do not pay the report-building cost. @@ -338,11 +402,12 @@ :handled "handled-exception" :unhandled "unhandled-exception" :silent nil)] - (let [report-hint (ex/get-hint cause)] + (let [format (if (environment-error? cause) :compact :full) + report-hint (ex/get-hint cause)] (when (and (string? report-hint) (not (str/empty? report-hint))) (let [state (reserve-report! (error-fingerprint event-name cause) (inst-ms (ct/now)))] (when (::emit state) - (let [generated (generate-report cause)] + (let [generated (generate-report cause {:format format})] (emit-report! event-name generated report-hint @@ -360,14 +425,29 @@ (assoc :links [{:label (tr "labels.download" "report.txt") :callback (partial download-report! report)}]))))))) +(defn- handle-connectivity-error + "Report a failure caused by the user's connectivity. These are audit-only + telemetry with a compact payload: they never reach the internal error + reports and a stack trace adds nothing for a network condition." + [error prefix] + (when-let [cause (::instance error)] + (ex/print-throwable cause :prefix prefix)) + (flash :cause (::instance error) + :type :handled + :hint (tr "errors.connection-error"))) + (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)) + (handle-connectivity-error error "Network Error")) + +(defmethod ptk/handle-error :offline + [error] + ;; Status 0 (browser offline) must not fall through to `:default`: + ;; that would report it as an unhandled application error. + (handle-connectivity-error error "Offline Error")) (def ^:private delegated-persistence-types "Save failure causes routed to their own error handler: retaining the diff --git a/frontend/src/app/main/ui/static.cljs b/frontend/src/app/main/ui/static.cljs index c9a78d5bcd..9a7dca3090 100644 --- a/frontend/src/app/main/ui/static.cljs +++ b/frontend/src/app/main/ui/static.cljs @@ -551,19 +551,22 @@ (mf/defc exception-section* {::mf/private true} [{:keys [data] :as props}] - (let [type (get data :type) - cause (get data ::errors/instance) + (let [type (get data :type) + cause (get data ::errors/instance) + environment (errors/environment-error? cause) - report (mf/with-memo [cause] - (when (ex/exception? cause) - (errors/generate-report cause))) + report (mf/with-memo [cause] + (when (ex/exception? cause) + (errors/generate-report cause {:format (if environment :compact :full)}))) - props (mf/spread-props props {:report report})] + props (mf/spread-props props {:report report})] (mf/with-effect [report type cause] (when (and (ex/exception? cause) (not (contains? #{:not-found :authentication} type))) - (errors/submit-report :event-name "exception-page" + ;; Environment pages are audit-only: they use the canonical + ;; `handled-exception` event instead of `exception-page`. + (errors/submit-report :event-name (if environment "handled-exception" "exception-page") :report report :hint (ex/get-hint cause) :cause cause))) diff --git a/frontend/test/frontend_tests/data/persistence_test.cljs b/frontend/test/frontend_tests/data/persistence_test.cljs index 9b8fea18b0..9a6fe53f10 100644 --- a/frontend/test/frontend_tests/data/persistence_test.cljs +++ b/frontend/test/frontend_tests/data/persistence_test.cljs @@ -33,37 +33,43 @@ :undo-changes []})) (t/deftest queued-edits-save-during-temporary-read-only-mode - (doseq [read-only-event [(drw/context-lost) - #(assoc-in % [:workspace-global :read-only?] true) - #(assoc % :workspace-global {:read-only? true - :preview-id (uuid/next)})]] - (let [file-id (uuid/next) - response (rx/subject) - errors (atom []) - store (ptk/store {:state {:permissions {:can-edit true} - :files {file-id {:id file-id :revn 0}}} - :on-error #(swap! errors conj %)})] - (with-redefs [rp/cmd! (mock/stub (fn [_ _] (rx/take 1 response)))] - (try - (ptk/emit! store (dps/initialize-persistence) - (local-commit file-id) - read-only-event - ::dps/force-persist) - (rx/push! response {:revn 1}) - (t/is (= :saved (get-in @store [:persistence :status]))) - (t/is (empty? (get-in @store [:persistence :queue]))) + (t/async done + (doseq [read-only-event [(drw/context-lost) + #(assoc-in % [:workspace-global :read-only?] true) + #(assoc % :workspace-global {:read-only? true + :preview-id (uuid/next)})]] + (let [file-id (uuid/next) + response (rx/subject) + errors (atom []) + store (ptk/store {:state {:permissions {:can-edit true} + :files {file-id {:id file-id :revn 0}}} + :on-error #(swap! errors conj %)})] + (mock/with-mocks + {rp/cmd! (mock/stub (fn [_ _] (rx/take 1 response)))} + (fn [section-done] + (try + (ptk/emit! store (dps/initialize-persistence) + (local-commit file-id) + read-only-event + ::dps/force-persist) + (rx/push! response {:revn 1}) + (t/is (= :saved (get-in @store [:persistence :status]))) + (t/is (empty? (get-in @store [:persistence :queue]))) - (ptk/emit! store (drw/context-restored) - #(assoc-in % [:workspace-global :read-only?] false) - (local-commit file-id) - ::dps/force-persist) - (rx/push! response {:revn 2}) - (t/is (= :saved (get-in @store [:persistence :status]))) - (t/is (empty? (get-in @store [:persistence :queue]))) - (t/is (empty? @errors)) - (finally - (rx/dispose! store) - (rx/end! response))))))) + (ptk/emit! store (drw/context-restored) + #(assoc-in % [:workspace-global :read-only?] false) + (local-commit file-id) + ::dps/force-persist) + (rx/push! response {:revn 2}) + (t/is (= :saved (get-in @store [:persistence :status]))) + (t/is (empty? (get-in @store [:persistence :queue]))) + (t/is (empty? @errors)) + (finally + (rx/dispose! store) + (rx/end! response) + (section-done)))) + (fn [])))) + (done))) (t/deftest historical-preview-cannot-create-local-commits (let [file-id (uuid/next) @@ -78,128 +84,155 @@ (t/is (empty? @output)))) (defn- with-watchdog - [f] + "Async fixture for the persistence watchdog tests: mocks the clock, timers + and RPC transport, and runs `f` with persistence initialized. + + Mocks are installed through `mock/with-mocks` and restored when the test + completes; `done'` chains to the `t/async` `done` and is always called + exactly once after teardown, so a failing body can neither leak the mocks + nor stall the test run." + [f done] (let [clock (atom 0) ticks (rx/subject) response (rx/subject) reports (atom []) causes (atom []) - render errors/generate-report file-id (uuid/next) store (ptk/store {:state {:current-file-id file-id :permissions {:can-edit true} :files {file-id {:id file-id :revn 0}}} :on-error #(t/is false (str %))})] - (with-redefs [ct/now (mock/stub #(ct/inst @clock)) - rx/interval (mock/stub (fn [_] ticks)) - rp/cmd! (mock/stub (fn [_ _] (rx/take 1 response))) - st/state store - errors/generate-report (fn [cause] - (swap! causes conj cause) - (render cause)) - errors/submit-report (fn [& params] - (swap! reports conj (apply hash-map params)))] - (try - (ptk/emit! store (dps/initialize-persistence)) - (f {:clock clock :ticks ticks :response response :causes causes - :reports reports :store store :file-id file-id}) - (finally - (rx/dispose! store) - (rx/end! ticks) - (rx/end! response)))))) + (mock/with-mocks + {ct/now (mock/stub #(ct/inst @clock)) + rx/interval (mock/stub (fn [_] ticks)) + rp/cmd! (mock/stub (fn [_ _] (rx/take 1 response))) + st/state store + errors/generate-report (fn [cause & _] + (swap! causes conj cause) + "report") + errors/submit-report (fn [& params] + (swap! reports conj (apply hash-map params)))} + (fn [done'] + (try + (ptk/emit! store (dps/initialize-persistence)) + (f {:clock clock :ticks ticks :response response :causes causes + :reports reports :store store :file-id file-id}) + (finally + (rx/dispose! store) + (rx/end! ticks) + (rx/end! response) + (done')))) + done))) (t/deftest stalled-request-is-reported-once-without-discarding-edits - (with-watchdog - (fn [{:keys [clock ticks reports causes store file-id]}] - (ptk/emit! store (local-commit file-id) ::dps/force-persist) - (reset! clock 300000) - (rx/push! ticks :tick) - (t/is (empty? @reports) "Five minutes must elapse before reporting") + (t/async done + (with-watchdog + (fn [{:keys [clock ticks reports causes store file-id]}] + (ptk/emit! store (local-commit file-id) ::dps/force-persist) + (reset! clock 300000) + (rx/push! ticks :tick) + (t/is (empty? @reports) "Five minutes must elapse before reporting") - ;; More local edits must not reset the stalled request's clock. - (reset! clock 300001) - (ptk/emit! store (drw/context-lost) - (local-commit file-id) ::dps/force-persist) - (rx/push! ticks :tick) - (t/is (= 1 (count @reports))) - (t/is (= "handled-exception" (:event-name (first @reports)))) - (let [data (ex-data (first @causes))] - (t/is (= :saving-stalled (:code data))) - (t/is (= file-id (:file-id data))) - (t/is (true? (:render-context-lost? data)))) + ;; More local edits must not reset the stalled request's clock. + (reset! clock 300001) + (ptk/emit! store (drw/context-lost) + (local-commit file-id) ::dps/force-persist) + (rx/push! ticks :tick) + (t/is (= 1 (count @reports))) + (t/is (= "handled-exception" (:event-name (first @reports)))) + (let [data (ex-data (first @causes))] + (t/is (= :saving-stalled (:code data))) + (t/is (= file-id (:file-id data))) + (t/is (true? (:render-context-lost? data)))) - (reset! clock 900000) - (rx/push! ticks :tick) - (t/is (= 1 (count @reports)) "Do not repeat a report for the same stall") - (t/is (= :saving (get-in @store [:persistence :status]))) - (t/is (= 2 (count (get-in @store [:persistence :queue]))))))) + (reset! clock 900000) + (rx/push! ticks :tick) + (t/is (= 1 (count @reports)) "Do not repeat a report for the same stall") + (t/is (= :saving (get-in @store [:persistence :status]))) + (t/is (= 2 (count (get-in @store [:persistence :queue]))))) + done))) (t/deftest successful-saves-reset-the-stall-clock-and-allow-a-new-report - (with-watchdog - (fn [{:keys [clock ticks response reports store file-id]}] - (ptk/emit! store (local-commit file-id) ::dps/force-persist - (local-commit file-id) ::dps/force-persist) - (reset! clock 290000) - (rx/push! response {:revn 1}) - (reset! clock 300001) - (rx/push! ticks :tick) - (t/is (empty? @reports) "The queue is making progress") + (t/async done + (with-watchdog + (fn [{:keys [clock ticks response reports store file-id]}] + (ptk/emit! store (local-commit file-id) ::dps/force-persist + (local-commit file-id) ::dps/force-persist) + (reset! clock 290000) + (rx/push! response {:revn 1}) + (reset! clock 300001) + (rx/push! ticks :tick) + (t/is (empty? @reports) "The queue is making progress") - (reset! clock 590001) - (rx/push! ticks :tick) - (t/is (= 1 (count @reports)) "The second request has now stalled") + (reset! clock 590001) + (rx/push! ticks :tick) + (t/is (= 1 (count @reports)) "The second request has now stalled") - (rx/push! response {:revn 2}) - (t/is (= :saved (get-in @store [:persistence :status]))) - (reset! clock 1000000) - (rx/push! ticks :tick) - (t/is (= 1 (count @reports)) "A saved file must not be reported") + (rx/push! response {:revn 2}) + (t/is (= :saved (get-in @store [:persistence :status]))) + (reset! clock 1000000) + (rx/push! ticks :tick) + (t/is (= 1 (count @reports)) "A saved file must not be reported") - (ptk/emit! store (local-commit file-id) ::dps/force-persist) - (reset! clock 1300001) - (rx/push! ticks :tick) - (t/is (= 2 (count @reports)) "A later stall gets its own report")))) + (ptk/emit! store (local-commit file-id) ::dps/force-persist) + (reset! clock 1300001) + (rx/push! ticks :tick) + (t/is (= 2 (count @reports)) "A later stall gets its own report")) + done))) (t/deftest pending-edits-are-monitored-without-extending-the-deadline - (with-watchdog - (fn [{:keys [clock ticks reports store]}] - (rx/push! ticks :tick) - (t/is (empty? @reports) "An idle file must not be reported") - (ptk/emit! store (#'dps/update-status :pending)) - (reset! clock 300001) - (ptk/emit! store (#'dps/update-status :pending)) - (rx/push! ticks :tick) - (t/is (= 1 (count @reports))) - (ptk/emit! store (#'dps/update-status :error)) - (reset! clock 900000) - (rx/push! ticks :tick) - (t/is (= 1 (count @reports)) "Do not report an already failed save")))) + (t/async done + (with-watchdog + (fn [{:keys [clock ticks reports store]}] + (rx/push! ticks :tick) + (t/is (empty? @reports) "An idle file must not be reported") + (ptk/emit! store (#'dps/update-status :pending)) + (reset! clock 300001) + (ptk/emit! store (#'dps/update-status :pending)) + (rx/push! ticks :tick) + (t/is (= 1 (count @reports))) + (ptk/emit! store (#'dps/update-status :error)) + (reset! clock 900000) + (rx/push! ticks :tick) + (t/is (= 1 (count @reports)) "Do not report an already failed save")) + done))) (t/deftest reinitializing-persistence-replaces-the-watchdog - (let [active-timers (atom 0) - ticks (rx/subject) - store (ptk/store {:state {} :on-error #(t/is false (str %))})] - (with-redefs [rx/interval (mock/stub - (fn [_] - (rx/create - (fn [subscriber] - (swap! active-timers inc) - (let [subscription (.subscribe ticks subscriber)] - (fn [] - (rx/dispose! subscription) - (swap! active-timers dec)))))))] - (try - (ptk/emit! store (dps/initialize-persistence)) - (t/is (= 1 @active-timers)) - (ptk/emit! store (dps/initialize-persistence)) - (t/is (= 1 @active-timers)) - (finally - (rx/dispose! store) - (rx/end! ticks)))) - (t/is (zero? @active-timers)))) + (t/async done + (let [active-timers (atom 0) + ticks (rx/subject) + store (ptk/store {:state {} :on-error #(t/is false (str %))})] + (mock/with-mocks + {rx/interval (mock/stub + (fn [_] + (rx/create + (fn [subscriber] + (swap! active-timers inc) + (let [subscription (.subscribe ticks subscriber)] + (fn [] + (rx/dispose! subscription) + (swap! active-timers dec)))))))} + (fn [section-done] + (try + (ptk/emit! store (dps/initialize-persistence)) + (t/is (= 1 @active-timers)) + (ptk/emit! store (dps/initialize-persistence)) + (t/is (= 1 @active-timers)) + (finally + (rx/dispose! store) + (rx/end! ticks) + (t/is (zero? @active-timers)) + (section-done)))) + done)))) (defn- with-persistence - [f] + "Async fixture for the persistence tests: mocks the transport and flash, and + runs `f` with persistence initialized. + + Like `with-watchdog`, `done'` chains to the `t/async` `done` and is called + exactly once after teardown. Inside a `doseq`, pass a no-op completion + (`(fn [])`) and call the test's `done` once at the end." + [f done] (let [file-id (uuid/next) response (rx/subject) failures (atom []) @@ -208,171 +241,214 @@ :permissions {:can-edit true} :files {file-id {:id file-id :revn 0}}} :on-error #(t/is false (str %))})] - (with-redefs [rp/cmd! (mock/stub (fn [cmd params] - (swap! requests conj [cmd params]) - (rx/take 1 response))) - errors/flash (fn [& {:keys [cause]}] - (swap! failures conj cause))] - (try - (ptk/emit! store (dps/initialize-persistence)) - (f {:file-id file-id :response response :failures failures - :requests requests :store store}) - (finally - (rx/dispose! store) - (rx/end! response)))))) + (mock/with-mocks + {rp/cmd! (mock/stub (fn [cmd params] + (swap! requests conj [cmd params]) + (rx/take 1 response))) + errors/flash (fn [& {:keys [cause]}] + (swap! failures conj cause))} + (fn [done'] + (try + (ptk/emit! store (dps/initialize-persistence)) + (f {:file-id file-id :response response :failures failures + :requests requests :store store}) + (finally + (rx/dispose! store) + (rx/end! response) + (done')))) + done))) (t/deftest permission-loss-fails-without-discarding-queued-edits - (with-persistence - (fn [{:keys [file-id requests failures store]}] - (ptk/emit! store (local-commit file-id) - #(assoc-in % [:permissions :can-edit] false) - ::dps/force-persist) - (t/is (= :error (get-in @store [:persistence :status]))) - (t/is (= 1 (count (get-in @store [:persistence :queue])))) - (t/is (empty? @requests)) - (t/is (= 1 (count @failures))) - (ptk/emit! store (local-commit file-id) ::dps/force-persist - (#'dps/update-status :pending)) - (t/is (= :error (get-in @store [:persistence :status]))) - (t/is (= 2 (count (get-in @store [:persistence :queue]))))))) + (t/async done + (with-persistence + (fn [{:keys [file-id requests failures store]}] + (ptk/emit! store (local-commit file-id) + #(assoc-in % [:permissions :can-edit] false) + ::dps/force-persist) + (t/is (= :error (get-in @store [:persistence :status]))) + (t/is (= 1 (count (get-in @store [:persistence :queue])))) + (t/is (empty? @requests)) + (t/is (= 1 (count @failures))) + (ptk/emit! store (local-commit file-id) ::dps/force-persist + (#'dps/update-status :pending)) + (t/is (= :error (get-in @store [:persistence :status]))) + (t/is (= 2 (count (get-in @store [:persistence :queue]))))) + done))) (t/deftest failed-request-retains-the-queue-and-is-not-retried-on-initialization - (with-persistence - (fn [{:keys [file-id response requests store]}] - (ptk/emit! store (local-commit file-id) ::dps/force-persist - (local-commit file-id) ::dps/force-persist) - (.error response (ex-info "Connection lost" {:type :network})) - (ptk/emit! store (dps/initialize-persistence)) - (t/is (= :error (get-in @store [:persistence :status]))) - (t/is (= 2 (count (get-in @store [:persistence :queue])))) - (t/is (= 1 (count @requests)))))) - -(t/deftest save-failures-use-a-translated-warning-except-for-authentication - (doseq [cause-type [:network :offline :authentication]] - (with-persistence - (fn [{:keys [file-id response store]}] - (let [notifications (atom [])] - (with-redefs [errors/flash (fn [& params] - (swap! notifications conj (apply hash-map params))) - i18n/tr (mock/stub #(str "translated:" %))] - (ptk/emit! store (local-commit file-id) ::dps/force-persist) - (.error response (ex-info "Raw transport details" {:type cause-type})) - (t/is (= :error (get-in @store [:persistence :status]))) - (let [data (get-in @store [:persistence :error])] - (ptk/handle-error (assoc data ::errors/instance (ex-info "Save failed" data)))) - (if (= cause-type :authentication) - (t/is (empty? @notifications)) - (t/is (= ["translated:errors.save-failed"] - (mapv :hint @notifications)))))))))) - -(t/deftest missing-commit-is-an-error-instead-of-skipping-changes - (with-persistence - (fn [{:keys [requests store]}] - (let [id (uuid/next)] - (ptk/emit! store - #(assoc % :persistence {:queue (conj #queue [] id) - :index {} :run-id (uuid/next) - :status :saving}) - (dps/initialize-persistence)) - (t/is (= :error (get-in @store [:persistence :status]))) - (t/is (= [id] (vec (get-in @store [:persistence :queue])))) - (t/is (empty? @requests)))))) - -(t/deftest initialization-recovers-an-unsent-commit-with-a-dangling-run-id - (with-persistence - (fn [{:keys [file-id response requests store]}] - (let [commit (assoc @(local-commit file-id) :changes []) - id (:id commit)] - (ptk/emit! store - #(assoc % :persistence {:queue (conj #queue [] id) - :index {id commit} :run-id (uuid/next) - :status :saving}) - (dps/initialize-persistence)) - (t/is (= 1 (count @requests))) - (rx/push! response {:revn 1}) - (t/is (= :saved (get-in @store [:persistence :status]))) - (t/is (empty? (get-in @store [:persistence :queue]))))))) - -(t/deftest an-active-request-is-never-sent-twice - (doseq [interrupt [[(dps/initialize-persistence)] - [(ptk/data-event ::dps/error)]]] + (t/async done (with-persistence (fn [{:keys [file-id response requests store]}] - (apply ptk/emit! store (local-commit file-id) ::dps/force-persist interrupt) + (ptk/emit! store (local-commit file-id) ::dps/force-persist + (local-commit file-id) ::dps/force-persist) + (.error response (ex-info "Connection lost" {:type :network})) (ptk/emit! store (dps/initialize-persistence)) - (t/is (= 1 (count @requests))) - (rx/push! response {:revn 1}) - (t/is (= :saved (get-in @store [:persistence :status]))) - (t/is (empty? (get-in @store [:persistence :queue]))))))) + (t/is (= :error (get-in @store [:persistence :status]))) + (t/is (= 2 (count (get-in @store [:persistence :queue])))) + (t/is (= 1 (count @requests)))) + done))) + +(t/deftest save-failures-use-a-translated-warning-except-for-authentication + (t/async done + (doseq [cause-type [:network :offline :authentication]] + (with-persistence + (fn [{:keys [file-id response store]}] + (let [notifications (atom [])] + (mock/with-mocks + {errors/flash (fn [& params] + (swap! notifications conj (apply hash-map params))) + i18n/tr (mock/stub #(str "translated:" %))} + (fn [section-done] + (ptk/emit! store (local-commit file-id) ::dps/force-persist) + (.error response (ex-info "Raw transport details" {:type cause-type})) + (t/is (= :error (get-in @store [:persistence :status]))) + (let [data (get-in @store [:persistence :error])] + (ptk/handle-error (assoc data ::errors/instance (ex-info "Save failed" data)))) + (if (= cause-type :authentication) + (t/is (empty? @notifications)) + (t/is (= ["translated:errors.save-failed"] + (mapv :hint @notifications)))) + (section-done)) + (fn [])))) + (fn []))) + (done))) + +(t/deftest missing-commit-is-an-error-instead-of-skipping-changes + (t/async done + (with-persistence + (fn [{:keys [requests store]}] + (let [id (uuid/next)] + (ptk/emit! store + #(assoc % :persistence {:queue (conj #queue [] id) + :index {} :run-id (uuid/next) + :status :saving}) + (dps/initialize-persistence)) + (t/is (= :error (get-in @store [:persistence :status]))) + (t/is (= [id] (vec (get-in @store [:persistence :queue])))) + (t/is (empty? @requests)))) + done))) + +(t/deftest initialization-recovers-an-unsent-commit-with-a-dangling-run-id + (t/async done + (with-persistence + (fn [{:keys [file-id response requests store]}] + (let [commit (assoc @(local-commit file-id) :changes []) + id (:id commit)] + (ptk/emit! store + #(assoc % :persistence {:queue (conj #queue [] id) + :index {id commit} :run-id (uuid/next) + :status :saving}) + (dps/initialize-persistence)) + (t/is (= 1 (count @requests))) + (rx/push! response {:revn 1}) + (t/is (= :saved (get-in @store [:persistence :status]))) + (t/is (empty? (get-in @store [:persistence :queue]))))) + done))) + +(t/deftest an-active-request-is-never-sent-twice + (t/async done + (doseq [interrupt [[(dps/initialize-persistence)] + [(ptk/data-event ::dps/error)]]] + (with-persistence + (fn [{:keys [file-id response requests store]}] + (apply ptk/emit! store (local-commit file-id) ::dps/force-persist interrupt) + (ptk/emit! store (dps/initialize-persistence)) + (t/is (= 1 (count @requests))) + (rx/push! response {:revn 1}) + (t/is (= :saved (get-in @store [:persistence :status]))) + (t/is (empty? (get-in @store [:persistence :queue])))) + (fn []))) + (done))) (t/deftest permission-restoration-resumes-only-unsent-edits - (with-persistence - (fn [{:keys [file-id response requests store]}] - (ptk/emit! store (local-commit file-id) ::dps/force-persist - (local-commit file-id) ::dps/force-persist - #(assoc-in % [:permissions :can-edit] false)) - (rx/push! response {:revn 1}) - (t/is (= :error (get-in @store [:persistence :status]))) - (t/is (= 1 (count (get-in @store [:persistence :queue])))) - (ptk/emit! store - #(assoc-in % [:permissions :can-edit] true) - (ptk/data-event :app.main.data.common/change-team-role)) - (t/is (= 2 (count @requests))) - (rx/push! response {:revn 2}) - (t/is (= :saved (get-in @store [:persistence :status]))) - (t/is (empty? (get-in @store [:persistence :queue])))))) + (t/async done + (with-persistence + (fn [{:keys [file-id response requests store]}] + (ptk/emit! store (local-commit file-id) ::dps/force-persist + (local-commit file-id) ::dps/force-persist + #(assoc-in % [:permissions :can-edit] false)) + (rx/push! response {:revn 1}) + (t/is (= :error (get-in @store [:persistence :status]))) + (t/is (= 1 (count (get-in @store [:persistence :queue])))) + (ptk/emit! store + #(assoc-in % [:permissions :can-edit] true) + (ptk/data-event :app.main.data.common/change-team-role)) + (t/is (= 2 (count @requests))) + (rx/push! response {:revn 2}) + (t/is (= :saved (get-in @store [:persistence :status]))) + (t/is (empty? (get-in @store [:persistence :queue])))) + done))) (t/deftest recovery-keeps-an-acknowledgment-received-without-a-runner - (with-persistence - (fn [{:keys [file-id response requests store]}] - (ptk/emit! store (local-commit file-id) ::dps/force-persist - (ptk/data-event ::dps/error)) - (rx/push! response {:revn 1}) - (t/is (= 1 (count (get-in @store [:persistence :queue])))) - (ptk/emit! store (dps/initialize-persistence)) - (t/is (= 1 (count @requests)) "The acknowledged changes must not be sent again") - (t/is (= :saved (get-in @store [:persistence :status]))) - (t/is (empty? (get-in @store [:persistence :queue])))))) + (t/async done + (with-persistence + (fn [{:keys [file-id response requests store]}] + (ptk/emit! store (local-commit file-id) ::dps/force-persist + (ptk/data-event ::dps/error)) + (rx/push! response {:revn 1}) + (t/is (= 1 (count (get-in @store [:persistence :queue])))) + (ptk/emit! store (dps/initialize-persistence)) + (t/is (= 1 (count @requests)) "The acknowledged changes must not be sent again") + (t/is (= :saved (get-in @store [:persistence :status]))) + (t/is (empty? (get-in @store [:persistence :queue])))) + done))) (t/deftest recovery-reports-an-unknown-request-outcome-without-replaying-it - (with-persistence - (fn [{:keys [file-id requests store]}] - (let [commit (assoc @(local-commit file-id) ::dps/request-id (uuid/next)) - id (:id commit)] - (ptk/emit! store - #(assoc % :persistence {:queue (conj #queue [] id) - :index {id commit} :status :saving}) - (dps/initialize-persistence)) - (t/is (= :error (get-in @store [:persistence :status]))) - (t/is (= :save-outcome-unknown (get-in @store [:persistence :error :code]))) - (t/is (= [id] (vec (get-in @store [:persistence :queue])))) - (t/is (empty? @requests)))))) + (t/async done + (with-persistence + (fn [{:keys [file-id requests store]}] + (let [commit (assoc @(local-commit file-id) ::dps/request-id (uuid/next)) + id (:id commit)] + (ptk/emit! store + #(assoc % :persistence {:queue (conj #queue [] id) + :index {id commit} :status :saving}) + (dps/initialize-persistence)) + (t/is (= :error (get-in @store [:persistence :status]))) + (t/is (= :save-outcome-unknown (get-in @store [:persistence :error :code]))) + (t/is (= [id] (vec (get-in @store [:persistence :queue])))) + (t/is (empty? @requests)))) + done))) (t/deftest initialization-flushes-buffered-edits-without-duplicating-them - (with-persistence - (fn [{:keys [file-id response requests store]}] - (ptk/emit! store (local-commit file-id) - (dps/initialize-persistence) - ::dps/force-persist) - (t/is (= 1 (count @requests))) - (t/is (= 1 (count (get-in @store [:persistence :queue])))) - (rx/push! response {:revn 1}) - (t/is (= :saved (get-in @store [:persistence :status])))))) + (t/async done + (with-persistence + (fn [{:keys [file-id response requests store]}] + (ptk/emit! store (local-commit file-id) + (dps/initialize-persistence) + ::dps/force-persist) + (t/is (= 1 (count @requests))) + (t/is (= 1 (count (get-in @store [:persistence :queue])))) + (rx/push! response {:revn 1}) + (t/is (= :saved (get-in @store [:persistence :status])))) + done))) (t/deftest synchronous-save-results-do-not-leave-a-dangling-runner - (with-persistence - (fn [{:keys [file-id store]}] - (with-redefs [rp/cmd! (mock/stub (fn [_ _] (rx/of {:revn 1})))] - (ptk/emit! store (local-commit file-id) ::dps/force-persist) - (t/is (= :saved (get-in @store [:persistence :status]))) - (t/is (empty? (get-in @store [:persistence :queue]))))))) - -(t/deftest empty-or-invalid-save-responses-preserve-the-queue-as-failed - (doseq [result [(rx/empty) (rx/of nil) (rx/of {:revn -1})]] + (t/async done (with-persistence (fn [{:keys [file-id store]}] - (with-redefs [rp/cmd! (mock/stub (fn [_ _] result))] - (ptk/emit! store (local-commit file-id) ::dps/force-persist) - (t/is (= :error (get-in @store [:persistence :status]))) - (t/is (= :invalid-save-response (get-in @store [:persistence :error :code]))) - (t/is (= 1 (count (get-in @store [:persistence :queue]))))))))) + (mock/with-mocks + {rp/cmd! (mock/stub (fn [_ _] (rx/of {:revn 1})))} + (fn [section-done] + (ptk/emit! store (local-commit file-id) ::dps/force-persist) + (t/is (= :saved (get-in @store [:persistence :status]))) + (t/is (empty? (get-in @store [:persistence :queue]))) + (section-done)) + (fn []))) + done))) + +(t/deftest empty-or-invalid-save-responses-preserve-the-queue-as-failed + (t/async done + (doseq [result [(rx/empty) (rx/of nil) (rx/of {:revn -1})]] + (with-persistence + (fn [{:keys [file-id store]}] + (mock/with-mocks + {rp/cmd! (mock/stub (fn [_ _] result))} + (fn [section-done] + (ptk/emit! store (local-commit file-id) ::dps/force-persist) + (t/is (= :error (get-in @store [:persistence :status]))) + (t/is (= :invalid-save-response (get-in @store [:persistence :error :code]))) + (t/is (= 1 (count (get-in @store [:persistence :queue])))) + (section-done)) + (fn []))) + (fn []))) + (done))) diff --git a/frontend/test/frontend_tests/main_errors_test.cljs b/frontend/test/frontend_tests/main_errors_test.cljs index 64b274e797..cdb8b84ec9 100644 --- a/frontend/test/frontend_tests/main_errors_test.cljs +++ b/frontend/test/frontend_tests/main_errors_test.cljs @@ -18,6 +18,7 @@ - delegated save failures – causes handled by the handler for their type" (:require [app.common.uuid :as uuid] + [app.main.data.event :as ev] [app.main.data.persistence :as dps] [app.main.data.workspace :as-alias dw] [app.main.errors :as errors] @@ -155,13 +156,21 @@ (t/is (string? fingerprint)) (t/is (str/starts-with? fingerprint "handled-exception|unknown|unknown|")))) -(t/deftest fallback-fingerprint-is-stable-and-discriminating - (t/is (= (errors/fallback-fingerprint "exception-page" "boom") - (errors/fallback-fingerprint "exception-page" "boom"))) - (t/is (not= (errors/fallback-fingerprint "exception-page" "boom") - (errors/fallback-fingerprint "handled-exception" "boom"))) - (t/is (not= (errors/fallback-fingerprint "exception-page" "boom") - (errors/fallback-fingerprint "exception-page" "other")))) +(t/deftest environment-fingerprints-ignore-the-stack-frame + (let [cause-a (doto (ex-info "http error" {:type :offline :hint "http error"}) + (unchecked-set "stack" "Error: http error\n at call-site-a (app.js:1)")) + cause-b (doto (ex-info "http error" {:type :offline :hint "http error"}) + (unchecked-set "stack" "Error: http error\n at call-site-b (app.js:2)")) + defect-a (doto (ex-info "boom" {:type :internal :hint "boom"}) + (unchecked-set "stack" "Error: boom\n at call-site-a (app.js:1)")) + defect-b (doto (ex-info "boom" {:type :internal :hint "boom"}) + (unchecked-set "stack" "Error: boom\n at call-site-b (app.js:2)"))] + (t/testing "environment failures group across internal call sites" + (t/is (= (errors/error-fingerprint "handled-exception" cause-a) + (errors/error-fingerprint "handled-exception" cause-b)))) + (t/testing "application defects keep the stack frame in their identity" + (t/is (not= (errors/error-fingerprint "handled-exception" defect-a) + (errors/error-fingerprint "handled-exception" defect-b)))))) (t/deftest governor-emits-first-occurrence-and-suppresses-repeats (let [d1 (errors/reserve-report* (errors/initial-report-state) "fp" 1000) @@ -243,17 +252,29 @@ (let [events (capture-reports! (fn [] (doseq [event-name ["handled-exception" "unhandled-exception" "exception-page"]] - (errors/submit-report :event-name event-name :report "report" :hint event-name) - (errors/submit-report :event-name event-name :report "report" :hint event-name))))] + (let [cause (error-cause :type :internal :hint event-name)] + (errors/submit-report :event-name event-name + :report "report" + :hint event-name + :cause cause) + (errors/submit-report :event-name event-name + :report "report" + :hint event-name + :cause cause)))))] (t/is (= 3 (count events))))) -(t/deftest submit-report-without-cause-dedups-by-fallback-fingerprint +(t/deftest submit-report-without-cause-is-ignored (let [events (capture-reports! (fn [] (errors/submit-report :event-name "exception-page" :report "report" :hint "boom") - (errors/submit-report :event-name "exception-page" :report "report" :hint "boom") - (errors/submit-report :event-name "exception-page" :report "report" :hint "other")))] - (t/is (= 2 (count events))))) + (errors/submit-report :event-name "exception-page" + :report "report" + :hint "boom" + :cause (error-cause :type :internal :hint "boom"))))] + ;; The cause-less call is ignored and must not consume the reservation + ;; of the cause-based report. + (t/is (= 1 (count events))) + (t/is (= 1 (:occurrences (deref (first events))))))) (t/deftest governor-bounds-an-incident-like-loop (let [cause (error-cause :type :network :hint "unable to perform fetch operation") @@ -268,29 +289,37 @@ (t/deftest flash-suppressed-occurrence-does-not-build-a-report (let [generated (atom 0) - cause (error-cause :type :network :hint "unable to perform fetch operation") + cause (error-cause :type :internal :hint "unable to perform fetch operation") events (atom [])] - (with-redefs [errors/generate-report (fn [_] (swap! generated inc) "report") - st/emit! (mock/stub (fn [& emitted] (swap! events into emitted))) - rt/get-current-href (constantly "https://penpot.example.com/#/workspace") - tm/schedule mock/noop] - (dotimes [_ 3] - (errors/flash :cause cause :type :handled)) - (t/is (= 1 (count @events))) - (t/is (= 1 @generated))))) + (mock/with-mocks + {st/format-last-events (mock/stub (fn [& _] (swap! generated inc) "report")) + st/emit! (mock/stub (fn [& emitted] (swap! events into emitted))) + rt/get-current-href (constantly "https://penpot.example.com/#/workspace") + tm/schedule mock/noop} + (fn [done'] + (dotimes [_ 3] + (errors/flash :cause cause :type :handled)) + (t/is (= 1 (count @events))) + (t/is (= 1 @generated)) + (done')) + (fn [])))) (t/deftest flash-bounds-an-incident-like-loop (let [generated (atom 0) - cause (error-cause :type :network :hint "unable to perform fetch operation") + cause (error-cause :type :internal :hint "unable to perform fetch operation") events (atom [])] - (with-redefs [errors/generate-report (fn [_] (swap! generated inc) "report") - st/emit! (mock/stub (fn [& emitted] (swap! events into emitted))) - rt/get-current-href (constantly "https://penpot.example.com/#/workspace") - tm/schedule mock/noop] - (dotimes [_ 10000] - (errors/flash :cause cause :type :handled)) - (t/is (= 1 (count @events))) - (t/is (= 1 @generated))))) + (mock/with-mocks + {st/format-last-events (mock/stub (fn [& _] (swap! generated inc) "report")) + st/emit! (mock/stub (fn [& emitted] (swap! events into emitted))) + rt/get-current-href (constantly "https://penpot.example.com/#/workspace") + tm/schedule mock/noop} + (fn [done'] + (dotimes [_ 10000] + (errors/flash :cause cause :type :handled)) + (t/is (= 1 (count @events))) + (t/is (= 1 @generated)) + (done')) + (fn [])))) (t/deftest generate-report-is-total-when-formatting-fails (with-redefs [st/format-last-events (mock/stub (fn [& _] (throw (ex-info "formatting failed" {}))))] @@ -303,10 +332,149 @@ st/emit! (mock/stub (fn [& emitted] (swap! events into emitted))) rt/get-current-href (constantly "https://penpot.example.com/#/workspace") tm/schedule mock/noop] - (errors/flash :cause (error-cause :type :network :hint "boom") :type :handled) + (errors/flash :cause (error-cause :type :internal :hint "boom") :type :handled) (t/is (= 1 (count @events))) (t/is (string? (:report (deref (first @events)))))))) +;; --------------------------------------------------------------------------- +;; Environment failures +;; +;; Connectivity and service failures are not application defects: they are +;; audit-only telemetry with a compact report and a dedicated toast. +;; --------------------------------------------------------------------------- + +(t/deftest environment-error-classification + (t/testing "environment failures are recognised" + (doseq [type [:network :offline :bad-gateway :service-unavailable + :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" + (doseq [type [:internal :assertion :persistence :validation :authentication]] + (t/is (false? (errors/environment-error? (error-cause :type type))) + (str "did not expect environment error: " type)))) + (t/testing "causes without ex-data are not environment failures" + (t/is (false? (errors/environment-error? (js/Error. "plain failure")))) + (t/is (false? (errors/environment-error? nil))))) + +(t/deftest generate-report-compact-omits-stack-data-and-last-events + (mock/with-mocks + {st/format-last-events (mock/stub (fn [& _] (throw (ex-info "must not be called" {})))) + rt/get-current-href (constantly "https://penpot.example.com/#/workspace")} + (fn [done'] + (let [cause (ex-info "http error" {:type :offline + :hint "http error" + :uri "/api/rpc/command/update-file" + :headers {"x-session-id" "secret"}}) + report (errors/generate-report cause {:format :compact})] + (t/is (string? report)) + (t/is (str/includes? report "Hint:")) + (t/is (str/includes? report "http error")) + (t/is (str/includes? report ":offline")) + (t/is (str/includes? report "/api/rpc/command/update-file")) + (t/is (not (str/includes? report "Last events:"))) + (t/is (not (str/includes? report "Data:"))) + (t/is (not (str/includes? report "===="))) + (t/is (not (str/includes? report "secret")))) + (done')) + (fn []))) + +(t/deftest generate-report-defaults-to-the-full-format + (mock/with-mocks + {st/format-last-events (mock/stub (fn [& _] "(stub last events)")) + rt/get-current-href (constantly "https://penpot.example.com/#/workspace")} + (fn [done'] + (let [report (errors/generate-report (error-cause :type :internal :hint "boom"))] + (t/is (str/includes? report "Last events:")) + (t/is (str/includes? report "(stub last events)"))) + (done')) + (fn []))) + +(t/deftest connectivity-handlers-report-governed-compact-audit-events + (doseq [type [:network :offline]] + (errors/reset-report-governor!) + (let [events (atom []) + cause (ex-info "http error" {:type type :hint "http error"})] + (mock/with-mocks + {st/emit! (mock/stub (fn [& emitted] (swap! events into emitted))) + rt/get-current-href (constantly "https://penpot.example.com/#/workspace") + tm/schedule (mock/stub (fn [f] (f))) + st/format-last-events (mock/stub (fn [& _] (throw (ex-info "must not be called" {}))))} + (fn [done'] + (errors/on-error cause) + ;; `flash` emits the report first and schedules the toast right after. + (t/is (= 2 (count @events)) (str "unexpected event count for " type)) + (let [report-event (first @events) + toast-event (second @events) + props (deref report-event)] + (t/is (= "handled-exception" (::ev/name props))) + (t/is (not (str/includes? (:report props) "Last events:"))) + (t/is (= (i18n/tr "errors.connection-error") + (get-in (ptk/update toast-event {}) [:notification :content])))) + (done')) + (fn []))))) + +(t/deftest flash-keeps-the-canonical-event-name-and-derives-the-format + (let [cause (ex-info "http error" {:type :network + :hint "http error" + :headers {"x-session-id" "secret"}})] + (doseq [[type event-name] [[:handled "handled-exception"] + [:unhandled "unhandled-exception"]]] + (errors/reset-report-governor!) + (let [events (atom [])] + (mock/with-mocks + {st/emit! (mock/stub (fn [& emitted] (swap! events into emitted))) + rt/get-current-href (constantly "https://penpot.example.com/#/workspace") + tm/schedule mock/noop + st/format-last-events (mock/stub (fn [& _] (throw (ex-info "must not be called" {}))))} + (fn [done'] + ;; The event name is the canonical one requested by the caller; + ;; only the payload format is derived from the cause. + (errors/flash :cause cause :type type) + (t/is (= 1 (count @events)) (str "unexpected event count for " type)) + (let [props (deref (first @events)) + report (:report props)] + (t/is (= event-name (::ev/name props)) (str "event name for " type)) + (t/is (not (str/includes? report "Last events:"))) + (t/is (not (str/includes? report "secret")))) + (done')) + (fn [])))))) + +(t/deftest offline-loop-is-governed-and-never-unhandled + (let [events (atom []) + cause (ex-info "http error" {:type :offline :hint "http error"})] + (mock/with-mocks + {st/emit! (mock/stub (fn [& emitted] (swap! events into emitted))) + rt/get-current-href (constantly "https://penpot.example.com/#/workspace") + tm/schedule mock/noop + st/format-last-events (mock/stub (fn [& _] (throw (ex-info "must not be called" {}))))} + (fn [done'] + (dotimes [_ 10000] + (errors/on-error cause)) + (t/is (= 1 (count @events))) + (t/is (= "handled-exception" (::ev/name (deref (first @events))))) + (done')) + (fn [])))) + +(t/deftest flash-persistence-uses-compact-reports-for-environment-failures + (let [events (atom [])] + (mock/with-mocks + {st/emit! (mock/stub (fn [& emitted] (swap! events into emitted))) + rt/get-current-href (constantly "https://penpot.example.com/#/workspace") + tm/schedule mock/noop + st/format-last-events (mock/stub (fn [& _] (throw (ex-info "must not be called" {}))))} + (fn [done'] + (errors/flash-persistence (ex-info "http error" {:type :offline + :hint "http error" + :headers {"x-session-id" "secret"}})) + (t/is (= 1 (count @events))) + (let [props (deref (first @events))] + (t/is (= "handled-exception" (::ev/name props))) + (t/is (not (str/includes? (:report props) "Last events:"))) + (t/is (not (str/includes? (:report props) "secret")))) + (done')) + (fn [])))) + (t/deftest exception-page-reports-dedup-by-cause (let [cause-a (error-cause :type :internal :code :unable-to-process-repository-response :hint "boom") cause-b (error-cause :type :internal :code :other :hint "other") diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 378bb3aed2..ff228ae248 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -1674,6 +1674,10 @@ msgstr "" msgid "errors.comment-error" msgstr "There was an error with the comment" +#: src/app/main/errors.cljs:429 +msgid "errors.connection-error" +msgstr "Cannot reach the server. Check your connection and try again." + #: src/app/main/errors.cljs:520 msgid "errors.deprecated" msgstr ""