nextcloud / nextcloud/richdocuments

Save As: route changed before reload confirmed can spawn a duplicate editor instance with no error handling (regression from #5719)

Open
#5,981 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
453
Forks
147
Avg merge
14h 54m
Merged PRs (30d)
83

Description

#5719 ("allow 2nd (and 3rd etc) saveas to work via doc reinit", merged in 9.1.1) fixed the original bug where a second Save As in one session silently no-opped, but introduced two related defects in the reload path it added.

1. Route/identity is mutated before the reload is confirmed, spawning a redundant second editor instance

switchToSavedAsFile() in src/view/Office.vue calls FilesAppIntegration.changeFilesRoute(node.fileid) before await this.load(). Changing the route causes the Files app Viewer to mount a second Office component for the new fileid, which independently runs its own mounted()load() cycle. So a single Save As can produce two concurrent host-side loads (and two WOPI token mints) for the same document, and nothing rolls the route back if the second load fails — the browser URL and "current file" state end up pointing at a document that never finished loading.

2. load()'s failure path never arms the loading-timeout, and is never caught

In load(), this.loadingTimeout = setTimeout(...) — the only mechanism that converts a stuck LOADING state into a visible FAILED error — is set after the awaited axios.post(generateUrl('/apps/richdocuments/token'), ...) call. If that POST rejects, the function returns before the timeout is ever armed. load() is called bare (no try/catch) from mounted(), so the rejection becomes an unhandled promise rejection with no user-visible error: the loading spinner stays up indefinitely.

Combined effect

Any transient failure of POST /apps/richdocuments/token during the second, redundant load (finding 1) — e.g. a brief backend hiccup, or eventual consistency in a clustered/load-balanced deployment right after the file-creating PutRelativeFile write — produces a permanently stuck editor with no error shown, on what looks to the user like the very first Save As of the session. We saw this reproduce reliably behind a load balancer with multiple app-server nodes; not reproducible on a single-node install.

Suggested fix

  • Defer changeFilesRoute() (and any route/identity mutation) until the load reaches DOCUMENT_READY, so a failed reload doesn't leave a redundant, doomed second instance running — or suppress the Viewer's re-open for a route change the editor itself just initiated.
  • Wrap the body of load() in try/catch and arm the loading-timeout independently of (or before) the /token POST, setting LOADING_STATE.FAILED plus a visible error on any rejection, the way the existing Action_Load_Resp failure branch already does.

Environment observed: Nextcloud 32.x, richdocuments 9.1.1, multiple app servers behind a load balancer.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/view/Office.vue by tracing switchToSavedAsFile(), the route change, mounted(), and load(), then inspect the existing Action_Load_Resp failure branch. Reproduce a Save As with a failed /apps/richdocuments/token request and verify that only the intended editor instance remains, the route is not left on an unready document, and a visible FAILED error replaces the indefinite spinner.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.