mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 01:54:10 +00:00
Take a list refresh asked for during another, instead of dropping it
bulk-select-all's "the pager agrees with the rows after a bulk action" failed now and then under load. Not the job: it archived the 67 conversations in six seconds. A nudge from the job started a list refresh while it was still writing, the job's "done" arrived while that fetch was out, and _refreshList() returned early because one was already in flight. The fetch came back without the archive, and nothing was left to refresh the list — it showed the archived conversations until something unrelated happened. A request that arrives during a refresh is now remembered and taken once the current fetch lands, keeping the most urgent of whatever asked in the meantime. list-refresh-queue.spec.ts holds the first fetch at the network so the second request always arrives during it; it fails without this change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
fea1a25f82
commit
549ddefb20
2 files changed
+82
No files matched your search
@@ -289,6 +289,9 @@ export default class extends Controller {
|
||||
clearTimeout(this._refreshTimer);
|
||||
this._refreshTimer = null;
|
||||
}
|
||||
|
||||
// The same for a refresh queued behind one in flight.
|
||||
this._refreshAgain = null;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -768,7 +771,22 @@ export default class extends Controller {
|
||||
|
||||
// One at a time: a burst of sync events would otherwise have several
|
||||
// responses racing to write the same element.
|
||||
//
|
||||
// But queued, not dropped. The fetch in flight was asked for BEFORE
|
||||
// whatever this request is about, and may well come back without it:
|
||||
// a bulk job's per-chunk nudge starts a refresh, the job finishes while
|
||||
// that fetch is out, and the job's "done" — the one event that is
|
||||
// certain to describe the finished list — arrived here and was thrown
|
||||
// away. The list then went on showing the archived conversations until
|
||||
// something unrelated happened to refresh it. One follow-up is taken
|
||||
// when the current fetch lands, carrying the most urgent of whatever
|
||||
// asked in the meantime.
|
||||
if (this._refreshing === true) {
|
||||
this._refreshAgain = {
|
||||
immediate: immediate || (this._refreshAgain?.immediate ?? false),
|
||||
url: url ?? this._refreshAgain?.url ?? null,
|
||||
};
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -849,6 +867,14 @@ export default class extends Controller {
|
||||
console.warn("[mail-pane] list refresh failed", error);
|
||||
} finally {
|
||||
this._refreshing = false;
|
||||
|
||||
const again = this._refreshAgain ?? null;
|
||||
|
||||
this._refreshAgain = null;
|
||||
|
||||
if (again !== null) {
|
||||
this._refreshList(again);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
import { test, expect } from "./support/test";
|
||||
|
||||
/**
|
||||
* A list refresh asked for while another is in flight is taken afterwards,
|
||||
* not dropped.
|
||||
*
|
||||
* Found as an intermittent failure of bulk-select-all.spec.ts: a whole-view
|
||||
* archive runs as a job, a nudge from the job starts a refresh while the job is
|
||||
* still writing, the job's "done" arrives while that fetch is out — and was
|
||||
* thrown away, so the list kept the archived conversations on screen with
|
||||
* nothing left to refresh it. Whether it happened depended on how the two
|
||||
* timings fell under load. This makes them fall that way every time: the first
|
||||
* fetch is held at the network, and the second request arrives while it is.
|
||||
*/
|
||||
test("a refresh asked for during another is taken when the first lands", async ({ page }) => {
|
||||
await page.goto("/mail/inbox");
|
||||
await expect(page.locator("#message-list li").first()).toBeVisible();
|
||||
|
||||
let fetches = 0;
|
||||
let releaseFirst: () => void = () => {};
|
||||
const firstHeld = new Promise<void>((resolve) => {
|
||||
releaseFirst = resolve;
|
||||
});
|
||||
|
||||
await page.route("**/mail/inbox**", async (route) => {
|
||||
if (undefined === route.request().headers()["x-list-fragment"]) {
|
||||
await route.continue();
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
fetches += 1;
|
||||
|
||||
if (1 === fetches) {
|
||||
await firstHeld;
|
||||
}
|
||||
|
||||
await route.continue();
|
||||
});
|
||||
|
||||
const jobChanged = (state: string) =>
|
||||
page.evaluate((detail) => {
|
||||
document.body.dispatchEvent(new CustomEvent("core--mercure:jobs-changed", { detail, bubbles: true }));
|
||||
}, { state });
|
||||
|
||||
// The first refresh starts and is held in flight…
|
||||
await jobChanged("done");
|
||||
await expect.poll(() => fetches).toBe(1);
|
||||
|
||||
// …the job's last word arrives while it is…
|
||||
await jobChanged("done");
|
||||
|
||||
// …and is taken once the first lands, rather than lost.
|
||||
releaseFirst();
|
||||
await expect.poll(() => fetches, { timeout: 5_000 }).toBe(2);
|
||||
});
|
||||
Reference in new issue
Block a user