Skip to content

fix(ui): send only one DELETE per message when leaving with pending deletes - #1075

Open
shivampwm2020 wants to merge 1 commit into
gotify:masterfrom
shivampwm2020:fix/duplicate-delete-pending-1068
Open

shivampwm2020 wants to merge 1 commit into
gotify:masterfrom
shivampwm2020:fix/duplicate-delete-pending-1068

Conversation

@shivampwm2020

Copy link
Copy Markdown

Problem

Deleting messages in the WebUI and then leaving/closing the tab while the undo
snackbar is still open fires two DELETE /message/{id} requests for the same
message. The second one 404s:

INF HTTP method=DELETE path=/message/150242 status=200
WRN HTTP errors="Error #01: message does not exist" method=DELETE path=/message/150242 status=404

Reported in #1068 (v3.1.1, Docker).

Root cause

ui/src/reactions.ts:8-9 registers the same handler on both page-leave events:

window.addEventListener('pagehide', stores.messagesStore.executePendingDeletes);
window.addEventListener('beforeunload', stores.messagesStore.executePendingDeletes);

Browsers fire both, and executePendingDeletes calls removeSingle for every
entry in pendingDeletes. The guard in removeSingle was checked before the
awaited request, while the entry was only removed after it resolved:

if (!this.pendingDeletes.has(message.id)) {
    return;
}
await axios.delete(...);          // second handler runs here, still sees the entry
...
this.cancelPendingDelete(message); // entry removed only now

So the second invocation passed the guard while the first request was in flight
and sent another DELETE. The same race exists between a page-leave event and
the snackbar's onExited callback (Messages.tsx:49), which also calls
removeSingle for the same id.

Fix

removeSingle now claims the pending delete up front, making the guard atomic —
the entry (and its undo snackbar) is consumed before the request is awaited, so
any later invocation for the same id is a no-op:

if (!this.cancelPendingDelete(message)) {
    return;
}
await axios.delete(...);

The trailing cancelPendingDelete(message) after the await is removed
accordingly (the snackbar is still closed at the same point).

Testing

Added ui/src/tests/pendingDelete.test.ts covering both races:

  1. two page-leave handlers run concurrently for one pending delete;
  2. a page-leave handler and the snackbar onExited race for the same id.

Both assert exactly one DELETE per id.

$ npx vitest run src/tests/pendingDelete.test.ts   # on master (before the fix)
 × sends exactly one DELETE per id when leaving the page with a pending undo
 × does not re-send a DELETE for a message already being deleted
AssertionError: expected [ 'unsetmessage/150242', …(1) ] to deeply equal [ 'unsetmessage/150242' ]
Test Files  1 failed (1)  |  Tests  2 failed (2)

$ npx vitest run src/tests/pendingDelete.test.ts   # with the fix
Test Files  1 passed (1)  |  Tests  2 passed (2)

tsc --noEmit, eslint "src/**/*.{ts,tsx}" and prettier --list-different are
all clean.

Note on test infra: the existing tests are puppeteer end-to-end tests that
drive the built bundle, so nothing in ui/src was ever imported directly by
vitest. vitest's transform pipeline does not apply the babel TC39 decorator
plugin that vite.config.ts uses, so importing @observable/@action source
from a unit test crashed with SyntaxError: Invalid or unexpected token. The
PR adds the same babel plugin to ui/vitest.config.js and a small DOM shim
(ui/src/tests/jsdom-shim.ts) so plain unit tests can import the stores.

Fixes #1068

…eletes

reactions.ts registers messagesStore.executePendingDeletes on both
`pagehide` and `beforeunload`, and browsers fire both when a page is left.
MessagesStore.removeSingle checked `pendingDeletes.has(id)` before awaiting
the request and only removed the entry after the await resolved, so the
second handler passed the guard and issued a second DELETE for the same id
(which 404s with "message does not exist").

Claim the pending delete up front via cancelPendingDelete() so the guard is
atomic; later invocations now no-op instead of firing again. The undo
snackbar is closed at the same point as before.

Adds a vitest regression test covering both the page-leave double event and
the snackbar onExited race (RED on master, GREEN with this fix).
@shivampwm2020
shivampwm2020 requested a review from a team as a code owner October 14, 2026 01:10
@codecov

codecov Bot commented Oct 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.57%. Comparing base (6a49e9c) to head (e14c68e).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1075      +/-   ##
==========================================
+ Coverage   76.54%   76.57%   +0.02%     
==========================================
  Files          68       68              
  Lines        3714     3714              
==========================================
+ Hits         2843     2844       +1     
+ Misses        658      657       -1     
  Partials      213      213              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

WebUI sends duplicate DELETE /message/{id} when leaving the page with pending deletes

1 participant