From 84d4db0ec855a7db1ac56508d0c368d3ca8cd5b7 Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Tue, 20 Oct 2026 16:00:58 +0200 Subject: [PATCH] :sparkles: Keep autosave and the workspace alive when saves fail - A save that failed on a short platform failure (a lost or refused database connection, lack of resources, a server shutting down, a serialization failure or deadlock, a socket or HTTP timeout, a host that did not resolve, an interruption) answered 500, which stops autosave. It now answers 503, which the frontend retries. Other I/O and database errors keep answering 500, timeouts 504. - A save answered not-found no longer replaces the workspace with the access page. It is a terminal save error: the workspace stays open with the failed save notice, report and reload link. When edit rights are lost, the notice says how many changes could not be saved. - A library sync marked the library synced three seconds later, whatever happened to its save. A rejected sync was undone but stayed marked: copies stayed out of date and the "updates available" notice never came back. The mark now waits for the save, a rollback of the sync brings the notice back, and a failed mark (e.g. the user just became a viewer) is logged instead of leaving the workspace. - While saves fail the workspace is read-only. The view-mode banner says editing is disabled because changes can't be saved, with the Inspect banner's size and bold lead and no Done button. Users can still switch pages, pan, zoom and inspect. The lock lifts when a save lands and brings back the mode the user had or picked meanwhile; the WebGL context lock keeps the workspace read-only if it lasts longer. The lock lives in app.main.data.persistence.lock. - A viewer is never locked. A role change that leaves the user a viewer keeps the edits that lost permission, queued and on screen, and shows the count notice again. The role change that brings edit rights back releases them and rolls them back on screen, as a rejected save would. Any other terminal failure is released by the next role change or file opened. The debug faults `io` (socket timeout, 503) and `eof` (500) cover both sides of the I/O rule. --- .serena/memories/backend/subtleties.md | 2 +- .../workspace-state-persistence-subtleties.md | 3 +- backend/src/app/http/errors.clj | 53 +++++- backend/src/app/rpc/commands/files_update.clj | 6 +- .../test/backend_tests/http_errors_test.clj | 43 +++++ frontend/src/app/main/data/persistence.cljs | 112 +++++++++--- .../app/main/data/persistence/commits.cljs | 8 + .../src/app/main/data/persistence/lock.cljs | 77 ++++++++ frontend/src/app/main/data/workspace.cljs | 3 +- .../src/app/main/data/workspace/common.cljs | 7 +- .../app/main/data/workspace/libraries.cljs | 164 ++++++++++++------ frontend/src/app/main/errors.cljs | 17 +- frontend/src/app/main/refs.cljs | 3 + .../main/ui/workspace/viewport/top_bar.cljs | 29 +++- .../main/ui/workspace/viewport/top_bar.scss | 20 +++ frontend/src/debug.cljs | 10 +- .../data/persistence_retry_test.cljs | 67 +++++++ .../frontend_tests/data/persistence_test.cljs | 140 ++++++++++++++- .../data/workspace_libraries_sync_test.cljs | 114 ++++++++++++ .../test/frontend_tests/main_errors_test.cljs | 30 +++- frontend/test/frontend_tests/runner.cljs | 4 + .../ui/viewport_top_bar_test.cljs | 58 +++++++ frontend/translations/en.po | 20 ++- frontend/translations/es.po | 16 ++ 24 files changed, 898 insertions(+), 108 deletions(-) create mode 100644 frontend/src/app/main/data/persistence/lock.cljs create mode 100644 frontend/test/frontend_tests/data/workspace_libraries_sync_test.cljs create mode 100644 frontend/test/frontend_tests/ui/viewport_top_bar_test.cljs diff --git a/.serena/memories/backend/subtleties.md b/.serena/memories/backend/subtleties.md index 5708f0b677..885d12cd92 100644 --- a/.serena/memories/backend/subtleties.md +++ b/.serena/memories/backend/subtleties.md @@ -91,4 +91,4 @@ - A deletion date is assigned once and never moved: `restore-files` (`app.rpc.commands.files`) unmarks only durable history, the `:file` method of `app.tasks.delete-object` dates only rows that have no date yet, and `objects-gc` collects only rows that carry one. Unmarking a revision leaves it invisible to `objects-gc` for ever, because nothing sets that date again. - Binfile v1/v3 import and the v3 export `get-file` step run inside one long transaction. They raise `idle_in_transaction_session_timeout` to 30 min (`app.binfile.common/transaction-idle-timeout-ms`) instead of disabling it, so a connection left idle while the ZIP is decoded or the file data is realized is bounded; `statement_timeout` stays at 300 s. - Automatic snapshots (`take-snapshot?`, every `auto-file-snapshot-every` revns or after `auto-file-snapshot-timeout`): `attach-snapshot` keeps the stored bytes as they are (pointers present → `realize-pointers` only, migrated → the migrated file) and flags `::objects-map` (passed to `fsnap/create!` as `:objects-map`) from the decoded data, never from `:features` (`update-file*` merges team and request features first, so they can name objects-map/pointer-map the data lacks). `fsnap/create!` runs in its own `tx-run!` (callers such as `restore-file-snapshot` pass a pool-only cfg); data with objects-maps is stored as it is and it queues `compact-file-snapshot` in that tx, which locks the `file_data` row and rewrites it with plain objects (`fsnap/compact!` via `fdata/realize-page-objects`, which skips pages behind a pointer since the task binds no `*load-fn*`; keeps backend/metadata/dates; no-op if gone or already plain; storage backend uploads a new object and touches the old one for gc). legacy-db compacts inline. Readers (preview, restore, file-gc) read both formats. Tests enable `fdata/pointer-map` for teams, so a saved file ends up behind pointers: test the raw-bytes path with files created without pointer-map, or call `fsnap/create!` directly. -- `update-file` with `:changes-with-metadata` groups that all carry `:commit-id` is partial: fast path applies and validates everything once; on a change failure it bisects the commits for the longest prefix that applies and validates (each attempt applies a prefix from the original data and runs referential integrity + file schema; ~log2 K attempts, none for a single commit, the first attempt's error reused), keeping that prefix (a commit that repairs an earlier one can keep both) and answering 200 with `:rejected {:commit-ids :failed-commit-id :error}`. Only accepted changes reach `file_change`, persistence and the msgbus; a fully rejected request writes and broadcasts nothing (revn unchanged). Flat `:changes` or groups without ids stay all-or-nothing. Before each attempt (`apply-prefix`, which returns `{:file :tracked :refs}` or the cause) `pmap/*tracked*` and the `cpc/*state*` atom are reset to their pre-attempt values (at the end `apply-commits` sets those of the kept prefix): a pointer map already `modified?` keeps its id when changed again, so the failed version would be persisted. DB/IO/interrupt errors and `:not-found` fragments (`change-failure?`) fail the whole request, never reject a commit. A rejection whose ex-info has no `:type` reports `:internal :unexpected-error`. `app.http.errors` answers 503 (`:server-error` + code) for transient SQLSTATEs (classes 08/53, 57P01–3, 40001, 40P01), `SQLTransientException`, `IOException` and `InterruptedException`, which the client's save retries; other SQL errors stay 500. With the `:debug-save-faults` flag, a `:set-plugin-data` file change in namespace `:penpot-debug` keyed `save-fault:` raises that fault in `apply-changes` (used by `debug.failNextSave`/`commitInvalidChange`). With a schema check running (hard schema validation without skip-validate, or soft) and 2+ commits, the touched pages' objects are decoded on the original file first (`decode-objects`; referential validation alone reads few objects, so no pre-decode then): objects-map caches decoded values per instance and a derived map copies the cache, so otherwise every attempt re-decodes the page (~260 ms at 10k shapes). Validation step timings (`with-validation-timing`, upstream `penpot_rpc_update_file_validation_timing` + the `update-file` debug log) come from the first attempt only (retries run with a cfg without metrics and timings); the whole bisection is one `retry` step sample, and the rejection log carries `:attempts`/`:elapsed`. +- `update-file` with `:changes-with-metadata` groups that all carry `:commit-id` is partial: fast path applies and validates everything once; on a change failure it bisects the commits for the longest prefix that applies and validates (each attempt applies a prefix from the original data and runs referential integrity + file schema; ~log2 K attempts, none for a single commit, the first attempt's error reused), keeping that prefix (a commit that repairs an earlier one can keep both) and answering 200 with `:rejected {:commit-ids :failed-commit-id :error}`. Only accepted changes reach `file_change`, persistence and the msgbus; a fully rejected request writes and broadcasts nothing (revn unchanged). Flat `:changes` or groups without ids stay all-or-nothing. Before each attempt (`apply-prefix`, which returns `{:file :tracked :refs}` or the cause) `pmap/*tracked*` and the `cpc/*state*` atom are reset to their pre-attempt values (at the end `apply-commits` sets those of the kept prefix): a pointer map already `modified?` keeps its id when changed again, so the failed version would be persisted. DB/IO/interrupt errors and `:not-found` fragments (`change-failure?`) fail the whole request, never reject a commit. A rejection whose ex-info has no `:type` reports `:internal :unexpected-error`. `app.http.errors` answers 503 (`:server-error` + code) for transient SQLSTATEs (classes 08/53, 57P01–3, 40001, 40P01), `SQLTransientException`, `InterruptedException` and only the `IOException`s that pass on their own (`transient-io-exception?`: `SocketException`, `InterruptedIOException` incl. socket timeouts, `UnknownHostException`, `HttpTimeoutException`), which the client's save retries (and GETs retry by themselves); any other `IOException` (EOF from a corrupt blob, Jackson parse errors, missing files) stays 500 `:io-exception`, as do other SQL errors. With the `:debug-save-faults` flag, a `:set-plugin-data` file change in namespace `:penpot-debug` keyed `save-fault:` raises that fault in `apply-changes` (used by `debug.failNextSave`/`commitInvalidChange`). With a schema check running (hard schema validation without skip-validate, or soft) and 2+ commits, the touched pages' objects are decoded on the original file first (`decode-objects`; referential validation alone reads few objects, so no pre-decode then): objects-map caches decoded values per instance and a derived map copies the cache, so otherwise every attempt re-decodes the page (~260 ms at 10k shapes). Validation step timings (`with-validation-timing`, upstream `penpot_rpc_update_file_validation_timing` + the `update-file` debug log) come from the first attempt only (retries run with a cfg without metrics and timings); the whole bisection is one `retry` step sample, and the rejection log carries `:attempts`/`:elapsed`. diff --git a/.serena/memories/frontend/workspace-state-persistence-subtleties.md b/.serena/memories/frontend/workspace-state-persistence-subtleties.md index b16875237f..6c0cd341b9 100644 --- a/.serena/memories/frontend/workspace-state-persistence-subtleties.md +++ b/.serena/memories/frontend/workspace-state-persistence-subtleties.md @@ -25,9 +25,10 @@ - A save answered with `:rejected` (backend kept the commits before the first failing one, see `mem:backend/subtleties`) is not a failure: `rollback-rejected` runs before `commit-persisted`. The unit is the undo group (one user action, which can commit to several files, e.g. `update-component` writes the copy's file and the library, and `component-changed` syncs inherit the edit's group; multi-file actions must share a non-nil group): the pure bookkeeping lives in `app.main.data.persistence.commits` (`pcm`): `pcm/rollback-plan` reverts, in the saved file, the pending commits from the first rejected one on, and in any file the pending commits from the first one of a reverted group on, iterating until no new group appears; other files' unrelated commits stay. `pcm/without-rolled-back` drops pending reverted commits from queue entries (partial entries trimmed, empty ones leave the queue) and the buffer; saved commits of a reverted group (the head's kept ones, and `:saved`, which `keep-saved-commits` fills on `discard-commit` with saved commits whose group a pending commit still shares) are undone by a new `save-undo? false` commit (`compensate-commits`, flushed like any edit); for a file whose data is not loaded the undo goes straight to the queue as an entry, nothing applied locally. Per file, `dch/rebase-commits` undoes every pending commit newest first, re-applies the remote changes kept since the oldest pending commit, and redoes the pending commits that stay. Undo changes hold absolute values from commit time, so undoing only the rejected commits would overwrite a peer's later change to the same attr. Remote changes are numbered (`record-remote-changes`, `[:persistence :remote-seq]`) and kept per file under `:remote-changes` only while the file has pending commits; `prune-remote-changes` (discard, release) forgets those at or below the oldest pending stamp, since re-applying them would undo saved edits. Then `dwu/forget-undo-groups` (cut history before the first entry of a reverted commit's undo-group; no match clears it; open transaction emptied), selection pruned and edition mode left when the edited shape is gone (`prune-removed-shapes`; `v2-update-text-shape-content` also skips a missing shape, so the editor's exit commits nothing), `errors/flash` `:unhandled` toast tagged `:persistence-rejected`, hidden after `rejected-timeout-ms` (10 s). It ends with `(ptk/data-event ::rolled-back {:file-ids})`: `dwl/sync-file` waits for it or `wait-persisted` and marks the library synced only on a landed save (on rollback it restores `:synced-at` and re-checks updates after the rollback's own save). `update-status` ignores `:pending` when buffer and queue are empty (`nothing-to-save?`): a dropped commit's 200 ms pending debounce would otherwise leave the status stuck and `wait-persisted-or-error` waiting forever. `prune-removed-shapes` also closes a `:colorpicker` modal. `rebase-commits` is not a commit (not saved/broadcast/undoable, commit watchers such as thumbnails do not see it) and falls back to `::dw/reload-current-file` if applying throws. `debug.commitInvalidChange({kind, before, after})` forces such a rejection live. - Persistence sends revn as the max of the commit revn and locally tracked latest revn; remote commits update that revn tracker. - Persistence is skipped in version preview/read-only mode or without edit permission. +- Failing saves lock the workspace (`app.main.data.persistence.lock`, alias `plk`): `plk/failing?` (status `:error`, or `:recovering` while a slow-cycle attempt is in flight as `:saving`) drives `sync-failure-lock` (also on `change-team-role`), which locks only while `[:permissions :can-edit]`: a viewer, already read-only, gets the `:save-permission-denied` toast and never the save-lock banner; `lock-on-failure` stores the previous read-only value in `[:workspace-global :persistence-lock]` and sets `:read-only?`; `unlock-after-failure` restores it (into `[:render-state :pre-context-lost-read-only?]` instead while the WebGL context is lost, which keeps its own save/restore). `set-workspace-read-only` writes into the lock while it is held, so an Inspect/Design switch is not lost. `view-only-bar*` (`ui/workspace/viewport/top_bar.cljs`, shared by both renderers) shows the save error while the lock exists (text per `dps/save-retrying?`), hides its Done/exit button and takes priority over the context-loss message. No save-failure popup: page navigation, pan, zoom and inspection stay available. Once unlocked, the usual Inspect/context-loss message and action return. Plugins still write while read-only (never as a viewer: `commit-changes` drops commits without `:can-edit`). A peer's `:file-restored` reloads the file and opens the `:file-restored` modal named after the restoring session's profile. - A terminal failure halts the queue on purpose: the edits stay queued so `wait-persisted-or-error` keeps rejecting, and `workspace/versions.cljs` relies on that to avoid snapshotting a file whose edits were never saved. `recover-persistence` lifts the halt when a file is opened again or the role changes. A transport failure still inside its retry window (`slow-retry-pending?`) is resent once (`resume-persistence true`), never released; any other halt emits `release-failed-file` (drops every queued commit of the file that failed) then `resume-persistence`. Opening a file also replays the queued, then buffered, edits of its version onto the loaded data (`pending-file-changes` → `restore-pending-changes` in `bundle-fetched`); a replay that throws keeps the server data and the queue. That is what unblocks the editor without a reload; because the queue is shared, dropping the failed file is also what lets other files' pending edits drain. `release-failed-file` runs at recovery time, never at failure time — clearing the error eagerly would make waiters resolve. - The split between `initialize-persistence-state` and `recover-persistence` is deliberate: initialization deliberately does NOT lift a halt, so undeliverable edits are released when the user acts, not when the view mounts. -- **Every non-transport terminal save failure is permanent** (a backend without commit-id groups still answers an invalid batch with a terminal `:validation`). There is no "stale but valid" category: permission lost, file gone, and server-rejected changes all fail again on a resend. Losing edit permission is not temporary — the backend raises `:not-found` for it (`check-edition-permissions!` → `perms/make-check-fn`), and a save's `:not-found` is a terminal save error that keeps the workspace open (`errors/delegated-persistence-types` is only `#{:authentication}`; a missing data fragment answers it too). So `:save-permission-denied` (a client-side `can-edit` check) must never be resumed: release it. Its toast `errors.save-permission-lost` is plural over `:unsaved-commits` (`count-unsaved-commits`); plural keys need `(tr k (i18n/c n))`, a bare number renders the fallback text. +- **Every non-transport terminal save failure is permanent** (a backend without commit-id groups still answers an invalid batch with a terminal `:validation`). There is no "stale but valid" category: permission lost, file gone, and server-rejected changes all fail again on a resend. Losing edit permission is not temporary — the backend raises `:not-found` for it (`check-edition-permissions!` → `perms/make-check-fn`), and a save's `:not-found` is a terminal save error that keeps the workspace open (`errors/delegated-persistence-types` is only `#{:authentication}`; a missing data fragment answers it too). So `:save-permission-denied` (a client-side `can-edit` check) must never be resumed. A role change that leaves `can-edit` false keeps it halted (the edits stay queued and on screen; `notify-kept-failure` shows the count toast again, since the single-toast store lets the role-change info toast replace it); the role change that brings edit rights back releases it. A release on a role change (`release-failed-file` with `revert?`, i.e. not on opening a file, whose data comes fresh from the server) also rolls the released commits back locally with `pcm/release-plan` (a `rollback-plan` rejecting every pending commit of the file) through `apply-rollback-plan`: rebase, compensation of saved commits of the same groups, undo-history cut, `prune-removed-shapes`, `::rolled-back`. Without it the canvas keeps showing edits the server never got. Its toast `errors.save-permission-lost` is plural over `:unsaved-commits` (`count-unsaved-commits`); plural keys need `(tr k (i18n/c n))`, a bare number renders the fallback text. - Save failures split transient vs terminal (`transient-error?`: the repo retryable types `:network`/`:offline`/`:bad-gateway`/`:service-unavailable`/`:gateway-error`, plus `:invalid-save-response`, plus save-only `busy-server-error?`: `:rate-limit` (429 from rlimit/climit — a save parked on another save's file lock holds the profile's single `:update-file/by-profile` permit, so the user's saves to other files 429 after 60 s) and backend-written 504 `:server-error` with `:statement-timeout`/`:idle-in-transaction-timeout` (Postgres cancels the lock wait at 300 s, before nginx; rolled back, so resend is safe). Kept out of `rp/retryable-types` on purpose so GETs do not retry them; everything else is terminal). Terminal keeps the `:error` halt + `flash-persistence` path; transient enters a `:retrying` episode: the head commit stays queued and resends with backoff 2s/8s/20s (3 retries, then the `:error` path). A transient failure that outlasts the backoff arms `slow-retry-cycle`: one silent resend (`:recovering`) every `slow-retry-delay-ms` (30s) until a save lands, `retry-give-up-ms` (24h since `:failing-since`) passes, or the head commit collects `max-slow-failures` (3) attempts that failed after `slow-failure-ms` (90s) or more (`:slow-failures`, counted in `persistence-transient-failure` from the attempt's elapsed time): a gateway timeout may leave the backend applying the save, so a save that keeps failing slowly must not be replayed for a day, while fast failures (offline) never count; the cycle stops on `::resume-persistence`, `::initialize-persistence` and `finalize-workspace`. Leaving the workspace also emits `suspend-retries` (`:suspended` in `:persistence`, drops `:retry-token` so a pending backoff timer goes stale): no backoff, slow cycle or `online` resend runs outside the workspace; only `workspace-initialized` (not a role change) lifts it via `recover-persistence` (`opened-file?`), which also resends a `:retrying` episode whose backoff was cut short. Its toast is `errors.save-still-retrying` without a reload link; a landing save hides the failure toast (`errors/persistence-failed-tag`). Replaying a save is safe because changes are idempotent, so there is no unknown-outcome guard: a commit whose earlier request left `active-requests` is simply sent again under the same `:commit-id`; only a request still in flight blocks a resend (`:in-flight` stays silent). Tests hold the slow cycle by stubbing `rx/timer` to never fire for `slow-retry-delay-ms`, or instant timers loop forever. Retry timers carry the episode token; a superseded token stays silent. Status stays `:retrying` through re-entries (`next-status` refuses `:pending`/`:saving` from it); waiters (`wait-persisted-or-error`) wait through it and reject only on `:error`. `terminal-status?` releases waiters only with an empty queue and an empty buffer: `run-persistence-task` sets `:saved` as soon as the queue empties, even with commits still buffered (a slow save landing while later edits wait for the 3 s flush). - One reconnect notice per episode: sticky toast tagged `:persistence-reconnecting` (single-toast store, re-show replaces), hidden by tag on drain (`:saved`) and on terminal failure; recovery is silent. Header indicator has a `:retrying` state (`workspace.header.retrying`). - Resume triggers: backoff timer and the browser `online` event (guarded by `exists? js/window`; re-enters the runner only for a live `:retrying` episode). New local edits during `:retrying` only join the queue: the episode keeps its `:run-id`, so `append-commit` does not re-enter, and the live runner (still waiting on the head's `commit-persisted`) sends them after the head. Re-entering on edits would bypass the backoff and spend an attempt per batched commit. diff --git a/backend/src/app/http/errors.clj b/backend/src/app/http/errors.clj index b8ef12eb74..7acad1f9f4 100644 --- a/backend/src/app/http/errors.clj +++ b/backend/src/app/http/errors.clj @@ -17,6 +17,7 @@ [app.http.session :as-alias session] [app.util.inet :as inet] [clojure.spec.alpha :as s] + [cuerdas.core :as str] [yetti.request :as yreq] [yetti.response :as yres])) @@ -223,6 +224,25 @@ "25P03" "The transaction was idle too long and was cancelled" "A database error occurred")) +(defn- transient-sql-state? + "True for SQLSTATE codes of failures that pass on their own: lost or + refused connections, lack of resources, a server shutting down, and + serialization failures or deadlocks of concurrent transactions." + [state] + (and (string? state) + (or (str/starts-with? state "08") + (str/starts-with? state "53") + (contains? #{"57P01" "57P02" "57P03" "40001" "40P01"} state)))) + +(defn- unavailable + "Response for a failure that passes on its own: the request can be sent + again." + [code hint] + {::yres/status 503 + ::yres/body {:type :server-error + :code code + :hint hint}}) + (defmethod handle-exception org.postgresql.util.PSQLException [error request parent-cause] (let [state (.getSQLState ^java.sql.SQLException error) @@ -231,6 +251,9 @@ (l/error :hint "postgresql error" :cause cause) (cond + (transient-sql-state? state) + (unavailable :database-unavailable "The database is not available right now") + (= state "57014") {::yres/status 504 ::yres/body {:type :server-error @@ -271,13 +294,37 @@ (assoc :type :server-error) (update :code #(or % :unhandled)))})))) +(defmethod handle-exception java.sql.SQLTransientException + [cause request _] + (binding [l/*context* (request->context request)] + (l/wrn :hint "transient database error" :cause cause) + (unavailable :database-unavailable "The database is not available right now"))) + +(defn- transient-io-exception? + "True for I/O failures that pass on their own: a lost, refused or timed + out connection, or a host name that did not resolve. A parse error or + data that cannot be read fails again on every attempt." + [cause] + (or (instance? java.net.SocketException cause) + (instance? java.io.InterruptedIOException cause) + (instance? java.net.UnknownHostException cause) + (instance? java.net.http.HttpTimeoutException cause))) + (defmethod handle-exception java.io.IOException [cause request _] (binding [l/*context* (request->context request)] (l/wrn :hint "io exception" :cause cause) - {::yres/status 500 - ::yres/body {:type :server-error - :code :io-exception}})) + (if (transient-io-exception? cause) + (unavailable :io-exception "An input/output operation failed") + {::yres/status 500 + ::yres/body {:type :server-error + :code :io-exception}}))) + +(defmethod handle-exception InterruptedException + [cause request _] + (binding [l/*context* (request->context request)] + (l/wrn :hint "interrupted request" :cause cause) + (unavailable :interrupted "The request was interrupted"))) (defmethod handle-exception java.util.concurrent.CompletionException [cause request _] diff --git a/backend/src/app/rpc/commands/files_update.clj b/backend/src/app/rpc/commands/files_update.clj index 63411a34d4..9537aab460 100644 --- a/backend/src/app/rpc/commands/files_update.clj +++ b/backend/src/app/rpc/commands/files_update.clj @@ -611,8 +611,10 @@ "runtime" (throw (RuntimeException. message)) "ex-info" (throw (ex-info message {})) "assertion" (throw (AssertionError. message)) - "sql" (throw (java.sql.SQLException. message)) - "io" (throw (java.io.IOException. message)) + "sql" (throw (org.postgresql.util.PSQLException. + message org.postgresql.util.PSQLState/CONNECTION_FAILURE)) + "io" (throw (java.net.SocketTimeoutException. message)) + "eof" (throw (java.io.EOFException. message)) "interrupted" (throw (InterruptedException. message)) "not-found" (ex/raise :type :not-found :code :object-not-found :hint message) "stack-overflow" (throw (StackOverflowError. message)) diff --git a/backend/test/backend_tests/http_errors_test.clj b/backend/test/backend_tests/http_errors_test.clj index 9fbf6e0345..52b23722c8 100644 --- a/backend/test/backend_tests/http_errors_test.clj +++ b/backend/test/backend_tests/http_errors_test.clj @@ -68,3 +68,46 @@ headers (::yres/headers response)] (t/is (= 429 (::yres/status response))) (t/is (nil? (get headers "retry-after"))))) + +(defn- psql-exception + [state] + (org.postgresql.util.PSQLException. "database failure" state)) + +(t/deftest transient-database-failures-answer-503 + (doseq [state [org.postgresql.util.PSQLState/CONNECTION_FAILURE + org.postgresql.util.PSQLState/CONNECTION_UNABLE_TO_CONNECT + org.postgresql.util.PSQLState/SERIALIZATION_FAILURE + org.postgresql.util.PSQLState/DEADLOCK_DETECTED]] + (let [response (http-errors/handle (psql-exception state) {})] + (t/is (= 503 (::yres/status response)) (str state)) + (t/is (= :server-error (-> response ::yres/body :type)))))) + +(t/deftest other-database-failures-answer-500 + (let [response (http-errors/handle (psql-exception org.postgresql.util.PSQLState/UNIQUE_VIOLATION) {})] + (t/is (= 500 (::yres/status response))) + (t/is (= :database-error (-> response ::yres/body :code))))) + +(t/deftest database-timeouts-keep-answering-504 + (let [response (http-errors/handle (psql-exception org.postgresql.util.PSQLState/QUERY_CANCELED) {})] + (t/is (= 504 (::yres/status response))) + (t/is (= :statement-timeout (-> response ::yres/body :code))))) + +(t/deftest transient-connection-failures-answer-503 + (doseq [cause [(java.sql.SQLTransientConnectionException. "pool exhausted") + (java.net.SocketException. "broken pipe") + (java.net.ConnectException. "connection refused") + (java.net.SocketTimeoutException. "read timed out") + (java.net.UnknownHostException. "storage.example.com") + (InterruptedException. "shutting down")]] + (let [response (http-errors/handle cause {})] + (t/is (= 503 (::yres/status response)) (str (class cause))) + (t/is (= :server-error (-> response ::yres/body :type)))))) + +(t/deftest permanent-io-failures-answer-500 + (doseq [cause [(java.io.IOException. "generic") + (java.io.EOFException. "truncated blob") + (java.nio.file.NoSuchFileException. "/data/missing") + (com.fasterxml.jackson.core.JsonParseException. nil "bad json")]] + (let [response (http-errors/handle cause {})] + (t/is (= 500 (::yres/status response)) (str (class cause))) + (t/is (= :io-exception (-> response ::yres/body :code)))))) diff --git a/frontend/src/app/main/data/persistence.cljs b/frontend/src/app/main/data/persistence.cljs index 3086280976..948f699c7a 100644 --- a/frontend/src/app/main/data/persistence.cljs +++ b/frontend/src/app/main/data/persistence.cljs @@ -18,6 +18,7 @@ [app.main.data.helpers :as dsh] [app.main.data.notifications :as ntf] [app.main.data.persistence.commits :as pcm] + [app.main.data.persistence.lock :as plk] [app.main.data.workspace :as-alias dw] [app.main.data.workspace.edition :as dwe] [app.main.data.workspace.undo :as dwu] @@ -152,6 +153,13 @@ (and (= from :retrying) (#{:pending :saving} to)) from :else to)) +(defn- count-unsaved-commits + "Local commits not saved yet, queued or buffered, of every file." + [state] + (let [{:keys [queue index buffer]} (:persistence state)] + (+ (count (mapcat #(get-in index [% :commits]) queue)) + (count buffer)))) + (defn- nothing-to-save? [{:keys [buffer queue]}] (and (empty? buffer) (empty? queue))) @@ -234,6 +242,13 @@ (retry-window-open? pstate) (< (or slow-failures 0) max-slow-failures))) +(defn save-retrying? + "True while the slow cycle keeps resending a failed queue, including + while one of its attempts is in flight." + [pstate] + (or (true? (:recovering pstate)) + (slow-retry-pending? pstate))) + (defn- report-stalled-persistence [now] (ptk/reify ::report-stalled-persistence @@ -567,6 +582,21 @@ :features features :remote-seq (dm/get-in state [:persistence :remote-seq] 0))])))))) +(defn- apply-rollback-plan + "Events that revert the local data as `plan` says: pending commits are + undone, saved ones undone by a new commit, and their groups leave the + undo history." + [origin state {:keys [files compensate groups]}] + (rx/concat + (rx/from (map (fn [[fid plan]] (dch/rebase-commits fid plan)) files)) + (rx/from (map (fn [[fid commits]] (compensate-commits origin state fid commits)) + (group-by :file-id compensate))) + (rx/of (dwu/forget-undo-groups groups) + (prune-removed-shapes) + ;; Tells listeners (e.g. a library sync) which files lost commits + (ptk/data-event ::rolled-back + {:file-ids (into (set (keys files)) (map :file-id) compensate)})))) + (defn- rollback-rejected "Reverts the commits `rejected` in the save of `entry`, the later commits of its file and the rest of their undo groups. Pending commits are undone @@ -583,13 +613,7 @@ ptk/WatchEvent (watch [it state _] - (let [{:keys [files compensate groups]} @plan] - (rx/concat - (rx/from (map (fn [[fid plan]] (dch/rebase-commits fid plan)) files)) - (rx/from (map (fn [[fid commits]] (compensate-commits it state fid commits)) - (group-by :file-id compensate))) - (rx/of (dwu/forget-undo-groups groups) - (prune-removed-shapes))))) + (apply-rollback-plan it state @plan)) ptk/EffectEvent (effect [_ _ _] @@ -720,7 +744,8 @@ (ex-info hint {:type :persistence :code code :commit-id commit-id - :file-id (:file-id commit)}))))] + :file-id (:file-id commit) + :unsaved-commits (count-unsaved-commits state)}))))] (if (and (some? token) (not= token (dm/get-in state [:persistence :retry-token]))) ;; Stale retry timer: its episode was superseded. Stay silent. @@ -832,18 +857,35 @@ rejecting: callers use it to avoid snapshotting a file whose edits were never saved. What must not survive is the halt. Left in place, the failed commits sit at the head of the queue forever and, because the queue is - shared, they also block the pending edits of every other file." - [] - (ptk/reify ::release-failed-file - ptk/UpdateEvent - (update [_ state] - (let [commit-id (get-in state [:persistence :error :commit-id]) - file-id (get-in state [:persistence :index commit-id :file-id])] - (update state :persistence - #(-> % + shared, they also block the pending edits of every other file. + + With `revert?` the file is still open with the released edits applied, + so they are rolled back as a rejection would be: undone on screen and + cut from the undo history." + [revert?] + (let [plan (volatile! nil)] + (ptk/reify ::release-failed-file + ptk/UpdateEvent + (update [_ state] + (let [pstate (:persistence state) + commit-id (get-in pstate [:error :commit-id]) + file-id (get-in pstate [:index commit-id :file-id]) + plan' (when (and revert? file-id) + (pcm/release-plan pstate file-id))] + (vreset! plan plan') + (assoc state :persistence + (-> (if plan' + (pcm/without-rolled-back pstate {:file-id file-id} plan') + pstate) (drop-file-commits file-id) (pcm/prune-remote-changes file-id) - (dissoc :failing-since :slow-failures))))))) + (dissoc :failing-since :slow-failures))))) + + ptk/WatchEvent + (watch [it state _] + (if-let [plan @plan] + (apply-rollback-plan it state plan) + (rx/empty)))))) (defn- queue-needs-a-runner? "True when the queue still holds commits that nobody is sending. @@ -914,6 +956,16 @@ (rx/of (flush-buffer)) (rx/empty))))))) +(defn- notify-kept-failure + "Shows the save failure the queue keeps again, so the count of unsaved + changes replaces the notice the role change shows." + [] + (ptk/reify ::notify-kept-failure + ptk/EffectEvent + (effect [_ state _] + (let [error (dm/get-in state [:persistence :error])] + (errors/flash-persistence (ex-info (:hint error) error)))))) + (defn- recover-persistence "Lifts a halted queue when a file is opened again or the role changes. @@ -931,7 +983,8 @@ A lost permission is not a temporary condition. The backend raises `:not-found` for it, and replaying the save to find that out routes the - user to the exception page." + user to the exception page. A role change that leaves the user a viewer + keeps those edits; the one that brings edit rights back releases them." [opened-file?] (ptk/reify ::recover-persistence ptk/WatchEvent @@ -953,8 +1006,17 @@ (slow-retry-pending? (:persistence state)) (rx/of (resume-persistence true)) + ;; A viewer's edits wait for edit rights to come back, still + ;; queued and on screen. + (and (not opened-file?) + (= :save-permission-denied (dm/get-in state [:persistence :error :code])) + (not (dm/get-in state [:permissions :can-edit]))) + (rx/of (notify-kept-failure)) + + ;; An opened file was loaded without the released edits; one + ;; still open shows them, so they are reverted. (= :error status) - (rx/of (release-failed-file) (resume-persistence)) + (rx/of (release-failed-file (not opened-file?)) (resume-persistence)) (queue-needs-a-runner? (:persistence state)) (rx/of (resume-persistence)) @@ -1100,6 +1162,16 @@ (rx/map (fn [_] (sync-unload-guard))) (rx/take-until stoper-s)) + ;; Locks the workspace while saves fail and the user can edit + (->> stream + (rx/filter #(or (ptk/type? ::update-status %) + (ptk/type? ::persistence-failed %) + (ptk/type? ::resume-persistence %) + (ptk/type? ::discard-commit %) + (ptk/type? ::dc/change-team-role %))) + (rx/map (fn [_] (plk/sync-failure-lock))) + (rx/take-until stoper-s)) + (->> local-commits-s (rx/debounce 200) (rx/map (fn [_] diff --git a/frontend/src/app/main/data/persistence/commits.cljs b/frontend/src/app/main/data/persistence/commits.cljs index 2d9c49b75b..fea5ee671c 100644 --- a/frontend/src/app/main/data/persistence/commits.cljs +++ b/frontend/src/app/main/data/persistence/commits.cljs @@ -128,6 +128,14 @@ :compensate (into [] (filter #(contains? groups (:undo-group %))) saved) :groups groups})) +(defn release-plan + "How to roll back every pending commit of `file-id`, as if the server had + rejected them all, together with the rest of their undo groups." + [pstate file-id] + (rollback-plan pstate + {:file-id file-id :commits []} + {:commit-ids (map :id (pending-commits pstate file-id))})) + (defn rollback-summary "Counts the commits `plan` reverts and the files they belong to." [{:keys [files compensate]}] diff --git a/frontend/src/app/main/data/persistence/lock.cljs b/frontend/src/app/main/data/persistence/lock.cljs new file mode 100644 index 0000000000..b9ad3cd8ce --- /dev/null +++ b/frontend/src/app/main/data/persistence/lock.cljs @@ -0,0 +1,77 @@ +;; 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 app.main.data.persistence.lock + "Read-only lock of the workspace while saves fail." + (:require + [app.common.data.macros :as dm] + [app.main.data.workspace.edition :as dwe] + [beicon.v2.core :as rx] + [potok.v2.core :as ptk])) + +(defn failing? + "True while saves fail: the queue is halted, or an attempt of the slow + cycle is in flight (status :saving until it lands or fails again)." + [pstate] + (or (= :error (:status pstate)) + (true? (:recovering pstate)))) + +(defn- lock-on-failure + "Puts the workspace in read-only mode and keeps the read-only value it + had, which `unlock-after-failure` brings back." + [] + (ptk/reify ::lock-on-failure + ptk/UpdateEvent + (update [_ state] + (if (dm/get-in state [:workspace-global :persistence-lock]) + state + (update state :workspace-global + (fn [wglobal] + (assoc wglobal + :persistence-lock {:read-only? (true? (:read-only? wglobal))} + :read-only? true))))) + + ptk/WatchEvent + (watch [_ _ _] + (rx/of :interrupt + (dwe/clear-edition-mode))))) + +(defn- unlock-after-failure + "Brings back the read-only value the workspace had before saves failed. + While the render context is lost the workspace stays read-only and the + value goes to the one `context-restored` brings back." + [] + (ptk/reify ::unlock-after-failure + ptk/UpdateEvent + (update [_ state] + (if-let [lock (dm/get-in state [:workspace-global :persistence-lock])] + (let [read-only? (:read-only? lock)] + (if (dm/get-in state [:render-state :lost]) + (-> state + (update :workspace-global dissoc :persistence-lock) + (assoc-in [:render-state :pre-context-lost-read-only?] read-only?)) + (update state :workspace-global + (fn [wglobal] + (-> wglobal + (dissoc :persistence-lock) + (assoc :read-only? read-only?)))))) + state)))) + +(defn sync-failure-lock + "Locks the workspace when saves start failing and unlocks it when they + recover. A user who cannot edit is never locked: the workspace is + already read-only for them, and the failure toast says why." + [] + (ptk/reify ::sync-failure-lock + ptk/WatchEvent + (watch [_ state _] + (let [lock? (and (failing? (:persistence state)) + (true? (dm/get-in state [:permissions :can-edit]))) + locked? (some? (dm/get-in state [:workspace-global :persistence-lock]))] + (cond + (and lock? (not locked?)) (rx/of (lock-on-failure)) + (and locked? (not lock?)) (rx/of (unlock-after-failure)) + :else (rx/empty)))))) diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index 74fe1252be..262c73c822 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -626,7 +626,8 @@ :workspace-tokens :workspace-undo :workspace-versions) - (update :workspace-global dissoc :read-only? :preview-id :default-font) + (update :workspace-global dissoc :read-only? :preview-id :default-font + :persistence-lock) (assoc-in [:workspace-global :options-mode] :design) (update :files d/update-vals #(dissoc % :data)))) diff --git a/frontend/src/app/main/data/workspace/common.cljs b/frontend/src/app/main/data/workspace/common.cljs index d682bbfe0e..e9911688b2 100644 --- a/frontend/src/app/main/data/workspace/common.cljs +++ b/frontend/src/app/main/data/workspace/common.cljs @@ -6,6 +6,7 @@ (ns app.main.data.workspace.common (:require + [app.common.data.macros :as dm] [app.common.logging :as log] [app.main.data.profile :as du] [app.main.data.workspace.layout :as dwl] @@ -70,11 +71,15 @@ ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; (defn set-workspace-read-only + "Sets the read-only mode. While failing saves lock the workspace, the + value is kept for when the lock is lifted." [read-only?] (ptk/reify ::set-workspace-read-only ptk/UpdateEvent (update [_ state] - (update state :workspace-global assoc :read-only? read-only?)) + (if (dm/get-in state [:workspace-global :persistence-lock]) + (assoc-in state [:workspace-global :persistence-lock :read-only?] read-only?) + (update state :workspace-global assoc :read-only? read-only?))) ptk/WatchEvent (watch [_ _ _] diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index 6367ce0500..c096727e46 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -37,6 +37,7 @@ [app.main.data.helpers :as dsh] [app.main.data.modal :as modal] [app.main.data.notifications :as ntf] + [app.main.data.persistence :as dps] [app.main.data.workspace :as-alias dw] [app.main.data.workspace.groups :as dwg] [app.main.data.workspace.notifications :as-alias dwn] @@ -107,6 +108,7 @@ (map extract-change changes))) (declare sync-file) +(declare notify-sync-file) (defn extract-path-if-missing [item] @@ -1197,6 +1199,30 @@ (when-not (= (:frame-id shape) uuid/zero) (dwt/clear-thumbnail file-id (:page-id shape) (:frame-id shape) "frame"))))))))))) +(defn- mark-library-synced + "Tells the backend that `file-id` is synced with `library-id`. Only the + update notice depends on it, so a failure (e.g. edit rights lost + meanwhile) is logged and ignored." + [file-id library-id] + (->> (rp/cmd! :update-file-library-sync-status + {:file-id file-id + :library-id library-id}) + (rx/catch (fn [cause] + (log/warn :hint "unable to mark the library as synced" + :file-id (str file-id) + :library-id (str library-id) + :cause cause) + (rx/empty))))) + +(defn- restore-library-synced-at + "Puts back the sync date of `library-id` a rejected sync moved forward, + so its update notice can show again." + [library-id synced-at] + (ptk/reify ::restore-library-synced-at + ptk/UpdateEvent + (update [_ state] + (d/update-in-when state [:files library-id] assoc :synced-at synced-at)))) + (defn sync-file "Synchronize the given file from the given library. Walk through all shapes in all pages in the file that use some color, typography or @@ -1217,70 +1243,96 @@ (contains? valid-asset-types asset-type))) (dm/assert! (or (nil? asset-id) (uuid? asset-id))) - (ptk/reify ::sync-file - ptk/UpdateEvent - (update [_ state] - (if (and (not= library-id (:current-file-id state)) - (nil? asset-id)) - (d/update-in-when state [:files library-id] assoc :synced-at (ct/now)) - state)) + (let [;; The sync date this sync replaced, if it replaced one + synced-at (volatile! ::unchanged)] + (ptk/reify ::sync-file + ptk/UpdateEvent + (update [_ state] + (if (and (not= library-id (:current-file-id state)) + (nil? asset-id)) + (do (vreset! synced-at (dm/get-in state [:files library-id :synced-at])) + (d/update-in-when state [:files library-id] assoc :synced-at (ct/now))) + state)) - ptk/WatchEvent - (watch [it state _] - (when (and (some? file-id) (some? library-id)) ; Prevent race conditions while navigating out of the file - (log/info :msg "SYNC-FILE" - :file (debug-pretty-file file-id state) - :library (debug-pretty-file library-id state) - :asset-type asset-type - :asset-id asset-id - :undo-group undo-group) - (let [ldata (dsh/lookup-file-data state file-id) - libraries (dsh/lookup-libraries state) - current-file-id (:current-file-id state) + ptk/WatchEvent + (watch [it state stream] + (when (and (some? file-id) (some? library-id)) ; Prevent race conditions while navigating out of the file + (log/info :msg "SYNC-FILE" + :file (debug-pretty-file file-id state) + :library (debug-pretty-file library-id state) + :asset-type asset-type + :asset-id asset-id + :undo-group undo-group) + (let [ldata (dsh/lookup-file-data state file-id) + libraries (dsh/lookup-libraries state) + current-file-id (:current-file-id state) - changes (cll/generate-sync-file-changes - (pcb/empty-changes it) - undo-group - asset-type - file-id - asset-id - library-id - libraries - current-file-id) + changes (cll/generate-sync-file-changes + (pcb/empty-changes it) + undo-group + asset-type + file-id + asset-id + library-id + libraries + current-file-id) - find-frames (fn [change] - (->> (ch/frames-changed ldata change) - (map #(assoc %1 :page-id (:page-id change))))) + find-frames (fn [change] + (->> (ch/frames-changed ldata change) + (map #(assoc %1 :page-id (:page-id change))))) - updated-frames (->> changes - :redo-changes - (mapcat find-frames) - distinct) + updated-frames (->> changes + :redo-changes + (mapcat find-frames) + distinct) - pending-ids (sync-file-pending-ids file-id changes) + pending-ids (sync-file-pending-ids file-id changes) - frontend-sync - (sync-file-frontend-events - file-id changes updated-frames undo-group)] + frontend-sync + (sync-file-frontend-events + file-id changes updated-frames undo-group)] - (log/debug :msg "SYNC-FILE finished" :js/rchanges (log-changes - (:redo-changes changes) - ldata)) - (rx/concat - ;; Keep the sync pending until its layout work starts. - (wrf/with-pending :sync-file pending-ids frontend-sync) + (log/debug :msg "SYNC-FILE finished" :js/rchanges (log-changes + (:redo-changes changes) + ldata)) + (rx/concat + ;; Keep the sync pending until its layout work starts. + (wrf/with-pending :sync-file pending-ids frontend-sync) - (when (not= file-id library-id) - ;; When we have just updated the library file, give some time for the - ;; update to finish, before marking this file as synced. - ;; TODO: look for a more precise way of syncing this. - ;; Maybe by using the stream (second argument passed to watch) - ;; to wait for the corresponding changes-committed and then proceed - ;; with the :update-file-library-sync-status mutation. - (rx/concat (rx/timer 3000) - (rp/cmd! :update-file-library-sync-status - {:file-id file-id - :library-id library-id})))))))))) + (when (not= file-id library-id) + (let [stopper-s (rx/filter (ptk/type? ::dw/finalize-workspace) stream) + restore (if (= ::unchanged @synced-at) + (rx/empty) + (rx/of (restore-library-synced-at library-id @synced-at)))] + (->> (rx/merge + ;; The server rejected the sync: its commits were undone + (->> stream + (rx/filter (ptk/type? ::dps/rolled-back)) + (rx/filter #(contains? (:file-ids (deref %)) file-id)) + (rx/map (constantly ::rolled-back))) + ;; The flush of the sync commits starts within 3 s; a save + ;; that fails for good ends the wait with no value + (->> (rx/timer 3000) + (rx/mapcat (fn [_] (dps/wait-persisted))) + (rx/map (constantly ::saved)) + (rx/if-empty ::failed))) + (rx/take 1) + (rx/take-until stopper-s) + (rx/mapcat (fn [outcome] + (case outcome + ::saved + (mark-library-synced file-id library-id) + + ;; Never saved: the update notice can show again + ::failed + restore + + ::rolled-back + (rx/concat + restore + ;; Checked once the rollback has undone the sync locally + (->> (dps/wait-persisted) + (rx/map (fn [_] (notify-sync-file))))))))))))))))))) ;; FIXME: the data should be set on the backend for clock consistency diff --git a/frontend/src/app/main/errors.cljs b/frontend/src/app/main/errors.cljs index 8eb7068eaf..5cd8981337 100644 --- a/frontend/src/app/main/errors.cljs +++ b/frontend/src/app/main/errors.cljs @@ -23,7 +23,7 @@ [app.util.dom :as dom] [app.util.globals :as g] [app.util.http :as http] - [app.util.i18n :refer [tr]] + [app.util.i18n :as i18n :refer [tr]] [app.util.timers :as ts] [app.util.webapi :as wapi] [beicon.v2.core :as rx] @@ -622,8 +622,10 @@ (def ^:private delegated-persistence-types "Save failure causes routed to their own error handler: retaining the - changes cannot resolve them." - #{:authentication :not-found}) + changes cannot resolve them. A save answered not-found (file gone, edit + rights lost, missing data) is not here: it keeps the workspace open and + notifies like any other failed save." + #{:authentication}) (defn- delegated-persistence-failure? [{:keys [type cause-type code]}] @@ -691,6 +693,15 @@ :report-link? true :hint (tr "errors.save-still-retrying")) + (= :save-permission-denied (:code data)) + (flash :cause cause + :type (persistence-report-type data) + :timeout nil + :tag persistence-failed-tag + :report-link? true + :links [(reload-page-link)] + :hint (tr "errors.save-permission-lost" (i18n/c (:unsaved-commits data 0)))) + :else (flash :cause cause :type (persistence-report-type data) diff --git a/frontend/src/app/main/refs.cljs b/frontend/src/app/main/refs.cljs index bb7fc0392a..61baf8e3af 100644 --- a/frontend/src/app/main/refs.cljs +++ b/frontend/src/app/main/refs.cljs @@ -335,6 +335,9 @@ (def workspace-read-only? (l/derived :read-only? workspace-global)) +(def persistence-lock + (l/derived :persistence-lock workspace-global)) + (def workspace-paddings-selected (l/derived :paddings-selected workspace-global)) diff --git a/frontend/src/app/main/ui/workspace/viewport/top_bar.cljs b/frontend/src/app/main/ui/workspace/viewport/top_bar.cljs index 7f59804825..bc0c0b0e61 100644 --- a/frontend/src/app/main/ui/workspace/viewport/top_bar.cljs +++ b/frontend/src/app/main/ui/workspace/viewport/top_bar.cljs @@ -7,6 +7,7 @@ (ns app.main.ui.workspace.viewport.top-bar (:require-macros [app.main.style :as stl]) (:require + [app.main.data.persistence :as dps] [app.main.data.workspace :as dw] [app.main.data.workspace.common :as dwc] [app.main.refs :as refs] @@ -29,18 +30,36 @@ (st/emit! :interrupt (dw/set-options-mode :design) (dwc/set-workspace-read-only false)))) - render-context-lost? (mf/deref refs/render-context-lost?)] - [:div {:class (stl/css :viewport-actions)} + render-context-lost? (mf/deref refs/render-context-lost?) + save-locked? (some? (mf/deref refs/persistence-lock)) + persistence (mf/deref refs/persistence)] + [:div {:class (stl/css-case :viewport-actions true + :viewport-actions-save-failed save-locked?)} [:div {:class (stl/css :viewport-actions-container)} - [:div {:class (stl/css :viewport-actions-title)} + [:div {:class (stl/css :viewport-actions-title) + :role (when save-locked? "status")} [:> i18n/tr-html* {:tag-name "span" - :content (if render-context-lost? + :content (cond + save-locked? + (if (dps/save-retrying? persistence) + (tr "workspace.save-failure.retrying") + (tr "workspace.save-failure.stopped")) + + render-context-lost? (tr "workspace.top-bar.webgl-context-lost") + + :else (tr "workspace.top-bar.view-only"))}]] - (if render-context-lost? + (cond + save-locked? + nil + + render-context-lost? [:> button* {:variant "primary" :on-click (fn [] (js/location.reload))} (tr "workspace.top-bar.webgl-context-lost.reload")] + + :else [:> button* {:on-click on-close} (tr "workspace.top-bar.read-only.done")])]])) diff --git a/frontend/src/app/main/ui/workspace/viewport/top_bar.scss b/frontend/src/app/main/ui/workspace/viewport/top_bar.scss index 255ae85080..752dcec135 100644 --- a/frontend/src/app/main/ui/workspace/viewport/top_bar.scss +++ b/frontend/src/app/main/ui/workspace/viewport/top_bar.scss @@ -45,6 +45,26 @@ color: var(--color-foreground-secondary); padding-left: deprecated.$s-8; width: max-content; + + strong { + font-weight: deprecated.$fw700; + } +} + +.viewport-actions-save-failed { + width: max-content; + min-width: min(#{deprecated.$s-400}, calc(100% - var(--sp-xl))); + max-width: calc(100% - var(--sp-xl)); + + .viewport-actions-container { + height: auto; + min-height: deprecated.$s-48; + min-width: 0; + } + + .viewport-actions-title { + width: auto; + } } .done-btn { diff --git a/frontend/src/debug.cljs b/frontend/src/debug.cljs index e6b0bc31e3..7a1abb89af 100644 --- a/frontend/src/debug.cljs +++ b/frontend/src/debug.cljs @@ -626,7 +626,7 @@ (def ^:private request-faults "Debug faults that fail the whole save, as an infrastructure error does." - #{"sql" "io" "interrupted" "not-found" "stack-overflow"}) + #{"sql" "io" "eof" "interrupted" "not-found" "stack-overflow"}) (defn- debug-plugin-data-change "A change that removes plugin data `key` of the file, which is never set, @@ -739,9 +739,11 @@ (default 0) on. Needs the `debug-save-faults` backend flag. Faults 'validation', 'runtime', 'ex-info' and 'assertion' reject the - commits from that one on; 'sql', 'io', 'interrupted', 'not-found' and - 'stack-overflow' fail the whole save, as an infrastructure error does. - A retry is a new save and counts against `times`. + commits from that one on; 'sql', 'io', 'eof', 'interrupted', 'not-found' + and 'stack-overflow' fail the whole save, as an infrastructure error + does. Of these, 'sql', 'io' and 'interrupted' pass on their own (503, + retried); the rest do not. A retry is a new save and counts against + `times`. debug.failNextSave({fault: 'sql'}) debug.failNextSave({fault: 'runtime', commit: 2, times: 3, fileId: '…'})" diff --git a/frontend/test/frontend_tests/data/persistence_retry_test.cljs b/frontend/test/frontend_tests/data/persistence_retry_test.cljs index 1eba1bd1b5..d80301c5c1 100644 --- a/frontend/test/frontend_tests/data/persistence_retry_test.cljs +++ b/frontend/test/frontend_tests/data/persistence_retry_test.cljs @@ -685,6 +685,44 @@ (rx/throw (ex-info "offline" {:type :offline})) (rx/of {:revn 1}))))))) +;; Scenario: the network stays down past the backoff, so the slow cycle +;; takes over. The workspace stays read-only through each slow attempt, +;; including while one is in flight, and goes back to editable once one +;; lands. Proves: the lock follows the failure, not the status flips of +;; the slow cycle. +(t/deftest ^:async the-slow-cycle-keeps-the-workspace-locked-until-a-save-lands + (let [calls (atom 0) + cycles (atom []) + attempt-s (rx/subject)] + (await + (with-persistence + (^:async fn [{:keys [file-id store]}] + (await + (mock/with-mocks* + {rx/timer (mock/stub (slow-cycle-timer cycles))} + (ptk/emit! store (local-commit file-id) ::dps/force-persist) + (await (async/wait-for #(and (= 1 (count @cycles)) + (some? (get-in @store [:workspace-global :persistence-lock]))) + "the spent backoff locks the workspace")) + (t/is (true? (get-in @store [:workspace-global :read-only?]))) + (t/is (dps/save-retrying? (:persistence @store))) + + (rx/push! (last @cycles) :tick) + (await (async/wait-for #(= 5 @calls) "the slow attempt is sent")) + (await (async/settle)) + (t/is (= :saving (get-in @store [:persistence :status]))) + (t/is (some? (get-in @store [:workspace-global :persistence-lock])) + "a slow attempt in flight keeps the lock") + + (rx/push! attempt-s {:revn 1}) + (await (async/wait-for #(nil? (get-in @store [:workspace-global :persistence-lock])) + "the landed save lifts the lock")) + (t/is (false? (get-in @store [:workspace-global :read-only?])))))) + (fn [_ _] + (if (< (swap! calls inc) 5) + (rx/throw (ex-info "offline" {:type :offline})) + (->> attempt-s (rx/take 1) (rx/observe-on :async)))))))) + ;; Scenario: a gateway error outlasts the backoff, then the proxy recovers. ;; Proves: each gateway status takes the slow cycle and saves the held edit. (t/deftest ^:async a-gateway-error-arms-the-slow-cycle @@ -823,6 +861,35 @@ (rx/throw (ex-info "offline" {:type :offline})) (rx/of {:revn 1}))))))) +;; Scenario: saves keep failing on a transport error, so the slow cycle +;; holds the edit, and the user is made a viewer; the role event comes again +;; while the user is still a viewer. Proves: the edit stays queued and the +;; leave warning stays on until edit rights return, instead of being dropped +;; while the canvas still shows it. +(t/deftest ^:async a-viewer-keeps-the-edit-held-by-the-slow-cycle + (let [cycles (atom [])] + (await + (with-persistence + (^:async fn [{:keys [file-id requests store]}] + (await + (mock/with-mocks* + {rx/timer (mock/stub (slow-cycle-timer cycles))} + (ptk/emit! store (local-commit file-id) ::dps/force-persist) + (await (async/wait-for #(= 1 (count @cycles)) "the slow cycle is armed")) + (ptk/emit! store + #(assoc-in % [:permissions :can-edit] false) + (ptk/data-event :app.main.data.common/change-team-role)) + (await (async/wait-for #(= :save-permission-denied + (get-in @store [:persistence :error :code])) + "the viewer cannot send the edit")) + (ptk/emit! store (ptk/data-event :app.main.data.common/change-team-role)) + (await (async/settle)) + (t/is (= :error (get-in @store [:persistence :status]))) + (t/is (= 1 (count (get-in @store [:persistence :queue]))) + "the edit stays queued while the user is a viewer") + (t/is (= 4 (count @requests)) "a viewer sends nothing")))) + (fn [_ _] (rx/throw (ex-info "offline" {:type :offline}))))))) + ;; Leaving the workspace tests. ;; Scenario: a save fails on a transport error and the user leaves the diff --git a/frontend/test/frontend_tests/data/persistence_test.cljs b/frontend/test/frontend_tests/data/persistence_test.cljs index dc72ae5a7a..8bf79716b3 100644 --- a/frontend/test/frontend_tests/data/persistence_test.cljs +++ b/frontend/test/frontend_tests/data/persistence_test.cljs @@ -26,6 +26,7 @@ [app.main.data.persistence.commits :as pcm] [app.main.data.render-wasm :as drw] [app.main.data.workspace :as dw] + [app.main.data.workspace.common :as dwc] [app.main.data.workspace.undo :as dwu] [app.main.errors :as errors] [app.main.refs :as refs] @@ -613,10 +614,12 @@ (await (async/wait-for #(and (= :error (get-in @store [:persistence :status])) (= 1 (count (get-in @store [:persistence :queue])))) "the save fails terminally")) - ;; The user acts again: a team-role change is one of the two real + ;; Edit rights come back: a team-role change is one of the two real ;; recovery triggers (the other is opening a file, whose event does ;; real work and is not usable as a bare signal here). - (ptk/emit! store (ptk/data-event :app.main.data.common/change-team-role)) + (ptk/emit! store + #(assoc-in % [:permissions :can-edit] true) + (ptk/data-event :app.main.data.common/change-team-role)) (await (async/wait-for #(and (= :saved (get-in @store [:persistence :status])) (empty? (get-in @store [:persistence :queue]))) "re-entering the file settles the queue")) @@ -626,6 +629,135 @@ "the edit that cannot be saved is not replayed on re-entry") (t/is (= 1 (count @failures)) "the user is still notified exactly once"))))) +;; Scenario: the user renames a shape and is made a viewer before the save, +;; so it fails for lost permission; the role event comes again, then edit +;; rights return. Proves: the edit is kept while the user is a viewer, and +;; released with edit rights back, the file goes back to the server's data +;; and the history loses the edit, so the canvas never shows a change the +;; server does not have. +(t/deftest ^:async regaining-edit-rights-reverts-the-released-edit + (await + (with-persistence + (^:async fn [{:keys [file-id requests failures store]}] + (await + (mock/with-mocks* + {mw/ask! (mock/stub (fn [_] (rx/empty)))} + (let [page-id (uuid/next) + shape-id (uuid/next) + commit (rename-commit file-id page-id shape-id "A" "B")] + (ptk/emit! store + #(-> % + (assoc :current-page-id page-id) + (assoc-in [:files file-id :data] (file-data page-id shape-id "B")) + (assoc :workspace-undo {:items [{:undo-group (:undo-group @commit) + :undo-groups #{(:undo-group @commit)} + :undo-changes (:undo-changes @commit) + :redo-changes (:redo-changes @commit) + :tags #{}}] + :index 0})) + commit + #(assoc-in % [:permissions :can-edit] false) + ::dps/force-persist) + (await (async/wait-for #(= :error (get-in @store [:persistence :status])) + "lost permission fails the save")) + + (ptk/emit! store (ptk/data-event :app.main.data.common/change-team-role)) + (await (async/settle)) + (t/is (= 1 (count (get-in @store [:persistence :queue]))) + "a viewer keeps the edit") + (t/is (= "B" (shape-name store file-id page-id shape-id))) + (t/is (= 2 (count @failures)) + "the count notice shows again over the role change notice") + + (ptk/emit! store + #(assoc-in % [:permissions :can-edit] true) + (ptk/data-event :app.main.data.common/change-team-role)) + (await (async/wait-for #(and (= :saved (get-in @store [:persistence :status])) + (empty? (get-in @store [:persistence :queue]))) + "edit rights back release the edit")) + (t/is (= "A" (shape-name store file-id page-id shape-id)) + "the file shows the server's data") + (t/is (empty? (get-in @store [:workspace-undo :items])) + "the released edit leaves the history") + (t/is (empty? @requests) "the released edit is not sent")))))))) + +;; Answers every save with an unexpected server error, a terminal failure. +(defn- server-error + [_ _] + (rp/handle-response {:status 500 + :body {:type :server-error :code :unhandled} + :uri "https://penpot.example.com/api/main/methods/update-file"})) + +;; Scenario: the user is in the Inspect tab (read-only) when a save fails +;; terminally, then goes back to Design while the dialog is up. The +;; workspace stays read-only until the failure is released, and then takes +;; the mode the user last asked for. Proves: the lock does not lose the +;; read-only mode the workspace had. +(t/deftest ^:async a-failed-save-locks-the-workspace-until-it-recovers + (await + (with-persistence + (^:async fn [{:keys [file-id store]}] + (ptk/emit! store #(assoc-in % [:workspace-global :read-only?] true) + (local-commit file-id) + ::dps/force-persist) + (await (async/wait-for #(some? (get-in @store [:workspace-global :persistence-lock])) + "the failed save locks the workspace")) + (t/is (true? (get-in @store [:workspace-global :read-only?]))) + (t/is (true? (get-in @store [:workspace-global :persistence-lock :read-only?]))) + + (ptk/emit! store (dwc/set-workspace-read-only false)) + (await (async/wait-for #(false? (get-in @store [:workspace-global :persistence-lock :read-only?])) + "the requested mode is kept for later")) + (t/is (true? (get-in @store [:workspace-global :read-only?])) + "the workspace stays read-only while saves fail") + + (ptk/emit! store (ptk/data-event :app.main.data.common/change-team-role)) + (await (async/wait-for #(nil? (get-in @store [:workspace-global :persistence-lock])) + "recovery lifts the lock")) + (t/is (false? (get-in @store [:workspace-global :read-only?])) + "the workspace takes the mode the user asked for")) + server-error))) + +;; Scenario: the user is made a viewer while an edit waits to be saved. The +;; save fails for lost permission, but the workspace is already read-only +;; for a viewer: no lock and no dialog, only the failure toast. Proves: a +;; viewer is never blocked from looking at the file. +(t/deftest ^:async a-viewer-is-not-locked-by-a-failed-save + (await + (with-persistence + (^:async fn [{:keys [file-id failures store]}] + (ptk/emit! store (local-commit file-id) + #(assoc-in % [:permissions :can-edit] false) + ::dps/force-persist) + (await (async/wait-for #(= :error (get-in @store [:persistence :status])) + "lost permission fails the save")) + (await (async/settle)) + (t/is (nil? (get-in @store [:workspace-global :persistence-lock]))) + (t/is (nil? (get-in @store [:workspace-global :read-only?]))) + (t/is (= 1 (count @failures)) "the user is told by the toast"))))) + +;; Scenario: saves fail, then the WebGL context is lost, then saves recover +;; before the context comes back. Proves: lifting the save lock does not +;; unlock a workspace the lost context still holds read-only. +(t/deftest ^:async a-lost-render-context-outlasts-the-save-lock + (await + (with-persistence + (^:async fn [{:keys [file-id store]}] + (ptk/emit! store (local-commit file-id) ::dps/force-persist) + (await (async/wait-for #(some? (get-in @store [:workspace-global :persistence-lock])) + "the failed save locks the workspace")) + (ptk/emit! store (drw/context-lost) + (ptk/data-event :app.main.data.common/change-team-role)) + (await (async/wait-for #(nil? (get-in @store [:workspace-global :persistence-lock])) + "recovery lifts the save lock")) + (t/is (true? (get-in @store [:workspace-global :read-only?])) + "the lost context keeps the workspace read-only") + + (ptk/emit! store (drw/context-restored)) + (await (async/wait-for #(false? (get-in @store [:workspace-global :read-only?])) + "the restored context brings back the first mode"))) + server-error))) + ;; Scenario: two files have pending edits and the first send fails ;; terminally. Opening a file again releases the failed file, the other file ;; still saves, and the editor settles saved instead of staying halted. @@ -1031,7 +1163,9 @@ "the save fails terminally")) (await (async/settle)) (t/is (contains? @tasks :persistence) "the retained edit keeps the guard on") - (ptk/emit! store (ptk/data-event :app.main.data.common/change-team-role)) + (ptk/emit! store + #(assoc-in % [:permissions :can-edit] true) + (ptk/data-event :app.main.data.common/change-team-role)) (await (async/wait-for #(empty? (get-in @store [:persistence :queue])) "recovery releases the edit")) (await (async/wait-for #(not (contains? @tasks :persistence)) diff --git a/frontend/test/frontend_tests/data/workspace_libraries_sync_test.cljs b/frontend/test/frontend_tests/data/workspace_libraries_sync_test.cljs new file mode 100644 index 0000000000..daa5e38955 --- /dev/null +++ b/frontend/test/frontend_tests/data/workspace_libraries_sync_test.cljs @@ -0,0 +1,114 @@ +;; 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.data.workspace-libraries-sync-test + "Tests for marking a library as synced after `dwl/sync-file`." + (:require + [app.common.time :as ct] + [app.common.types.file :as ctf] + [app.common.uuid :as uuid] + [app.main.data.persistence :as dps] + [app.main.data.workspace.libraries :as dwl] + [app.main.repo :as rp] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.async :as async] + [frontend-tests.helpers.mock :as mock] + [potok.v2.core :as ptk])) + +(def ^:private synced-at + (ct/inst 1000)) + +(defn- library-store + "A store with the open file `file-id` and its library `library-id`, last + synced at `synced-at`." + [file-id library-id] + (ptk/store {:state {:current-file-id file-id + :permissions {:can-edit true} + :files {file-id {:id file-id + :data (ctf/make-file-data file-id)} + library-id {:id library-id + :library-of file-id + :synced-at synced-at + :modified-at synced-at + :data (ctf/make-file-data library-id)}}} + :on-error #(t/is false (str %))})) + +(defn- ^:async sync-library + "Syncs the library and lets the save end with `saved`, an observable for + `dps/wait-persisted`; `before-save` runs once the sync started. Waits until + `settled?`, given the commands sent and the library sync date, is true, + then calls `check` with them, the store and the file ids, and awaits + what it returns." + [saved before-save settled? check] + (let [file-id (uuid/next) + library-id (uuid/next) + store (library-store file-id library-id) + commands (atom []) + result #(hash-map :commands @commands + :synced-at (get-in @store [:files library-id :synced-at]) + :store store + :file-id file-id + :library-id library-id)] + (await + (mock/with-mocks* + {rx/timer (mock/stub (fn [& _] (rx/of 0))) + dps/wait-persisted (mock/stub (fn [& _] saved)) + rp/cmd! (mock/stub (fn [cmd _] + (swap! commands conj cmd) + (rx/of nil)))} + (ptk/emit! store (dwl/sync-file file-id library-id)) + (await (async/wait-for #(not= synced-at (:synced-at (result))) + "the sync moves the sync date forward")) + (before-save store file-id) + (await (async/wait-for #(settled? (result)) "the sync settles")) + (await (async/settle)) + (await (check (result))))))) + +;; Scenario: the sync's commits are saved. Proves: the library is marked as +;; synced on the server. +(t/deftest ^:async a-saved-sync-marks-the-library-as-synced + (await + (sync-library (rx/of :saved) + (fn [_ _]) + #(seq (:commands %)) + (fn [{:keys [commands]}] + (t/is (= [:update-file-library-sync-status] commands)))))) + +;; Scenario: the server rejects the save of the sync's commits and the +;; rollback undoes them. Proves: the library is not marked as synced and its +;; sync date goes back, so the update notice can show again. +(t/deftest ^:async a-rejected-sync-leaves-the-library-to-sync-again + (await + (sync-library (rx/subject) + (fn [store file-id] + (ptk/emit! store (ptk/data-event ::dps/rolled-back + {:file-ids #{file-id}}))) + #(= synced-at (:synced-at %)) + (fn [{:keys [commands] :as result}] + (t/is (empty? commands)) + (t/is (= synced-at (:synced-at result))))))) + +;; Scenario: the save of the sync's commits fails for good, so waiting for +;; it ends with no value; later an edit of the same file is rejected. +;; Proves: the library is not marked as synced, its sync date goes back, +;; and the sync stops listening, so the later rollback leaves the date alone. +(t/deftest ^:async a-failed-sync-save-ends-the-sync + (let [saved (rx/subject)] + (await + (sync-library saved + ;; The save fails for good: the wait ends with no value + (fn [_ _] (rx/end! saved)) + #(= synced-at (:synced-at %)) + (^:async fn [{:keys [commands store file-id library-id]}] + (t/is (empty? commands)) + (let [later (ct/inst 5000)] + (ptk/emit! store + #(assoc-in % [:files library-id :synced-at] later) + (ptk/data-event ::dps/rolled-back {:file-ids #{file-id}})) + (await (async/settle)) + (t/is (= later (get-in @store [:files library-id :synced-at])) + "a later rollback does not wake the ended sync"))))))) diff --git a/frontend/test/frontend_tests/main_errors_test.cljs b/frontend/test/frontend_tests/main_errors_test.cljs index b16affd566..4f59e70b95 100644 --- a/frontend/test/frontend_tests/main_errors_test.cljs +++ b/frontend/test/frontend_tests/main_errors_test.cljs @@ -1021,8 +1021,15 @@ (await (async/settle)) (t/is (= [::rt/nav-raw] (mapv ptk/type @events)))))))) -(t/deftest a-deleted-file-during-save-shows-the-exception-page - (t/testing "a file that no longer exists shows its page, not the save notification" +(defn- shown-notifications + "Notifications the scheduled callbacks of a flash put on screen." + [events] + (keep #(when (implements? ptk/UpdateEvent %) + (get-in (ptk/update % {}) [:notification :content])) + events)) + +(t/deftest a-not-found-save-keeps-the-workspace + (t/testing "a save answered not-found notifies instead of leaving the workspace" (let [events (atom []) scheduled (atom [])] (with-redefs [rt/assign-exception (fn [error] error) @@ -1030,9 +1037,24 @@ (swap! events into emitted))) tm/schedule (mock/stub (fn [callback] (swap! scheduled conj callback)))] - (errors/flash-persistence (ex-info "File not found" {:type :not-found})) + (errors/flash-persistence (ex-info "Save failed" {:type :persistence + :cause-type :not-found + :code :object-not-found})) (doseq [callback @scheduled] (callback)) - (t/is (= [:not-found] (mapv :type @events))))))) + (t/is (not-any? #(= :not-found (:type %)) @events) "no exception page") + (t/is (= [(i18n/tr "errors.save-failed")] (shown-notifications @events))))))) + +(t/deftest lost-edit-permission-tells-how-many-changes-were-not-saved + (let [events (atom []) + scheduled (atom [])] + (with-redefs [st/emit! (mock/stub (fn [& emitted] (swap! events into emitted))) + tm/schedule (mock/stub (fn [callback] (swap! scheduled conj callback)))] + (errors/flash-persistence (ex-info "Edit permission was lost" + {:type :persistence + :code :save-permission-denied + :unsaved-commits 3})) + (doseq [callback @scheduled] (callback)) + (t/is (= [(i18n/tr "errors.save-permission-lost" (i18n/c 3))] (shown-notifications @events)))))) (t/deftest a-restored-version-during-save-reloads-the-file (t/testing "a version restored elsewhere reloads the file instead of notifying" diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index 96ca35d60b..a17ce660f4 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -30,6 +30,7 @@ [frontend-tests.data.workspace-comments-test] [frontend-tests.data.workspace-context-menu-test] [frontend-tests.data.workspace-interactions-test] + [frontend-tests.data.workspace-libraries-sync-test] [frontend-tests.data.workspace-mcp-test] [frontend-tests.data.workspace-media-test] [frontend-tests.data.workspace-pages-test] @@ -132,6 +133,7 @@ [frontend-tests.ui.sidebar-scroll-test] [frontend-tests.ui.static-test] [frontend-tests.ui.stroke-menu-test] + [frontend-tests.ui.viewport-top-bar-test] [frontend-tests.ui.workspace-libraries-test] [frontend-tests.util-clipboard-test] [frontend-tests.util-http-test] @@ -183,6 +185,7 @@ 'frontend-tests.data.workspace-comments-test 'frontend-tests.data.workspace-context-menu-test 'frontend-tests.data.workspace-interactions-test + 'frontend-tests.data.workspace-libraries-sync-test 'frontend-tests.data.workspace-mcp-test 'frontend-tests.data.workspace-media-test 'frontend-tests.data.workspace-pages-test @@ -286,6 +289,7 @@ 'frontend-tests.ui.sidebar-scroll-test 'frontend-tests.ui.static-test 'frontend-tests.ui.stroke-menu-test + 'frontend-tests.ui.viewport-top-bar-test 'frontend-tests.ui.workspace-libraries-test 'frontend-tests.util-clipboard-test 'frontend-tests.util-http-test diff --git a/frontend/test/frontend_tests/ui/viewport_top_bar_test.cljs b/frontend/test/frontend_tests/ui/viewport_top_bar_test.cljs new file mode 100644 index 0000000000..b753122243 --- /dev/null +++ b/frontend/test/frontend_tests/ui/viewport_top_bar_test.cljs @@ -0,0 +1,58 @@ +;; 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.ui.viewport-top-bar-test + (:require + ["react-dom/server" :as rds] + [app.main.refs :as refs] + [app.main.ui.workspace.viewport.top-bar :as top-bar] + [app.util.i18n :as i18n] + [cljs.test :as t :include-macros true] + [cuerdas.core :as str] + [frontend-tests.helpers.mock :as mock] + [okulary.core :as l] + [rumext.v2 :as mf])) + +(defn- render-bar + [state] + (let [state (l/atom state)] + (with-redefs [refs/persistence (l/derived :persistence state) + refs/persistence-lock (l/derived :lock state) + refs/render-context-lost? (l/derived :context-lost state) + i18n/tr (mock/stub (fn [key] key))] + (rds/renderToStaticMarkup (mf/element top-bar/view-only-bar* #js {}))))) + +(t/deftest a-terminal-save-failure-replaces-the-view-mode-message-and-exit + (let [markup (render-bar {:lock {:read-only? false} + :persistence {:status :error :error {:type :server-error}}})] + (t/is (str/includes? markup "workspace.save-failure.stopped")) + (t/is (str/includes? markup "role=\"status\"")) + (t/is (not (str/includes? markup "workspace.top-bar.view-only"))) + (t/is (not (str/includes? markup "