fix(client): spaced update saving more times than necesssary and causing performance issues

This commit is contained in:
Elian Doran
2026-04-10 12:00:08 +03:00
parent a4b716f8c7
commit dfe6063929
2 changed files with 95 additions and 2 deletions

View File

@@ -0,0 +1,87 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import SpacedUpdate from "./spaced_update";
// Mock logError which is a global in Trilium
vi.stubGlobal("logError", vi.fn());
describe("SpacedUpdate", () => {
beforeEach(() => {
vi.useFakeTimers();
});
afterEach(() => {
vi.useRealTimers();
});
it("should only call updater once per interval even with multiple pending callbacks", async () => {
const updater = vi.fn(async () => {
// Simulate a slow network request - this is where the race condition occurs
await new Promise((resolve) => setTimeout(resolve, 100));
});
const spacedUpdate = new SpacedUpdate(updater, 50);
// Simulate rapid typing - each keystroke calls scheduleUpdate()
// This queues multiple setTimeout callbacks due to recursive scheduleUpdate() calls
for (let i = 0; i < 10; i++) {
spacedUpdate.scheduleUpdate();
// Small delay between keystrokes
await vi.advanceTimersByTimeAsync(5);
}
// Advance time past the update interval to trigger the update
await vi.advanceTimersByTimeAsync(100);
// Let the "network request" complete and any pending callbacks run
await vi.advanceTimersByTimeAsync(200);
// The updater should have been called only ONCE, not multiple times
// With the bug, multiple pending setTimeout callbacks would all pass the time check
// during the async updater call and trigger multiple concurrent requests
expect(updater).toHaveBeenCalledTimes(1);
});
it("should call updater again if changes occur during the update", async () => {
const updater = vi.fn(async () => {
await new Promise((resolve) => setTimeout(resolve, 50));
});
const spacedUpdate = new SpacedUpdate(updater, 30);
// First update
spacedUpdate.scheduleUpdate();
await vi.advanceTimersByTimeAsync(40);
// Schedule another update while the first one is in progress
spacedUpdate.scheduleUpdate();
// Let first update complete
await vi.advanceTimersByTimeAsync(60);
// Advance past the interval again for the second update
await vi.advanceTimersByTimeAsync(100);
// Should have been called twice - once for each distinct change period
expect(updater).toHaveBeenCalledTimes(2);
});
it("should restore changed flag on error so retry can happen", async () => {
const updater = vi.fn()
.mockRejectedValueOnce(new Error("Network error"))
.mockResolvedValue(undefined);
const spacedUpdate = new SpacedUpdate(updater, 50);
spacedUpdate.scheduleUpdate();
// Advance to trigger first update (which will fail)
await vi.advanceTimersByTimeAsync(60);
// The error should have restored the changed flag, so scheduling again should work
spacedUpdate.scheduleUpdate();
await vi.advanceTimersByTimeAsync(60);
expect(updater).toHaveBeenCalledTimes(2);
});
});

View File

@@ -77,16 +77,22 @@ export default class SpacedUpdate {
}
if (Date.now() - this.lastUpdated > this.updateInterval) {
// Update these BEFORE the async call to prevent race conditions.
// Multiple setTimeout callbacks may be pending from recursive scheduleUpdate() calls.
// Without this, they would all pass the time check during the await and trigger multiple requests.
this.lastUpdated = Date.now();
this.changed = false;
this.onStateChanged("saving");
try {
await this.updater();
this.onStateChanged("saved");
this.changed = false;
} catch (e) {
// Restore changed flag on error so a retry can happen
this.changed = true;
this.onStateChanged("error");
logError(getErrorMessage(e));
}
this.lastUpdated = Date.now();
} else {
// update isn't triggered but changes are still pending, so we need to schedule another check
this.scheduleUpdate();