Skip to content

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

Merged
jmattheis merged 1 commit into
gotify:masterfrom
shivampwm2020:fix/duplicate-delete-pending-1068
Oct 18, 2026
Merged

jmattheis merged 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

@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.69%. Comparing base (6a49e9c) to head (2dacf74).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1075      +/-   ##
==========================================
+ Coverage   76.54%   76.69%   +0.14%     
==========================================
  Files          68       68              
  Lines        3714     3724      +10     
==========================================
+ Hits         2843     2856      +13     
+ Misses        658      656       -2     
+ Partials      213      212       -1     

☔ 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.

@jmattheis
jmattheis force-pushed the fix/duplicate-delete-pending-1068 branch from e14c68e to 2dacf74 Compare October 18, 2026 09:03
@jmattheis
jmattheis added this pull request to the merge queue Oct 18, 2026
Merged via the queue into gotify:master with commit f4de619 Oct 18, 2026
3 checks passed
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

2 participants