diigo-cache: a short page before the stated end peeks at the next page

The real fill failed at "diigo returned 44 items for page 167 of 168":
diigo's `total` overstates what load_user_items returns, so the true
last page looked short and the guard took it for a capped page size.

A short page that isn't the last by `total` now fetches the next page
(paced like any diigo call): empty means the data really ends there —
accepted, with a "total-overstated" notice logging total vs actual;
anything else means items would be skipped — refused, the message now
giving both counts. The fake gains an account whose total claims 60
more than it has; a fill on it runs to done.

Generated-by: Claude Opus 5.5
This commit is contained in:
dckc committed 2026-10-07 05:04:04 -05:00
1 parent f4b68f1b59
commit f5d57f73d3
4 files changed
+65 -17

No files matched your search

+28 -13
View File
@@ -303,22 +303,23 @@ abstract class PagedCache extends DurableObject<CacheEnv> {
// RPC stubs (the app and each pipelined step) are disposed once the // RPC stubs (the app and each pipelined step) are disposed once the
// page is in hand: undisposed stubs keep this DO from being evicted // page is in hand: undisposed stubs keep this DO from being evicted
const app = await this.env.WEBAPP.at(init.webappId); const data = await this.#fetchPage(init.webappId, pageNum, count);
const joined = app.pathjoin("load_user_items");
const queried = joined.query({ sort: "updated", count, page_num: pageNum });
let data: { total: number; items: Item[] };
try {
data = JSON.parse(await queried.get()) as { total: number; items: Item[] };
} finally {
for (const stub of [queried, joined, app]) disposeStub(stub);
}
// diigo may cap the page size below what we asked: a short page that // diigo may cap the page size below what we asked: a short page that
// isn't the last would silently skip items, so refuse it loudly // isn't the last would silently skip items. But diigo's `total` can
// also overstate what it returns, making the true last page look
// short. The next page tells them apart: empty means the data ends
// here; anything else means items would be skipped — refuse loudly.
const lastPage = Math.max(1, Math.ceil(data.total / count)); const lastPage = Math.max(1, Math.ceil(data.total / count));
if (data.items.length < count && pageNum < lastPage) { if (data.items.length < count && pageNum < lastPage) {
const error = `diigo returned ${data.items.length} items for page ${pageNum} of ${lastPage} (asked for ${count}): page size not honored?`; await this.#clock.sleep(MS_PER_CALL);
console.error({ event: "cache", kind: this.kind, page: pageNum, outcome: "short-page", fetched: data.items.length, asked: count }); const next = await this.#fetchPage(init.webappId, pageNum + 1, count);
throw new Error(error); if (next.items.length > 0) {
const error = `diigo returned ${data.items.length} items for page ${pageNum} of ${lastPage} (asked for ${count}), then ${next.items.length} for page ${pageNum + 1}: page size not honored?`;
console.error({ event: "cache", kind: this.kind, page: pageNum, outcome: "short-page", fetched: data.items.length, asked: count, nextFetched: next.items.length });
throw new Error(error);
}
const actual = (pageNum - 1) * count + data.items.length;
console.log({ event: "cache", kind: this.kind, page: pageNum, outcome: "total-overstated", total: data.total, actual });
} }
const kept = this.keep(toRows(data.items, console)); const kept = this.keep(toRows(data.items, console));
// fetched vs kept: the public cache's drop of private rows shows here // fetched vs kept: the public cache's drop of private rows shows here
@@ -326,6 +327,20 @@ abstract class PagedCache extends DurableObject<CacheEnv> {
return this.#etch(pageNum, count, data.total, kept); return this.#etch(pageNum, count, data.total, kept);
} }
/** one diigo page, raw, through the webapp instance; the RPC stubs (the
* app and each pipelined step) are disposed once the page is in hand —
* undisposed stubs keep this DO from being evicted */
async #fetchPage(webappId: string, pageNum: number, count: number): Promise<{ total: number; items: Item[] }> {
const app = await this.env.WEBAPP.at(webappId);
const joined = app.pathjoin("load_user_items");
const queried = joined.query({ sort: "updated", count, page_num: pageNum });
try {
return JSON.parse(await queried.get()) as { total: number; items: Item[] };
} finally {
for (const stub of [queried, joined, app]) disposeStub(stub);
}
}
/** etch a page (one transaction): upsert items, mark the page, keep /** etch a page (one transaction): upsert items, mark the page, keep
* the fetch-time order; returns the cache-page shape */ * the fetch-time order; returns the cache-page shape */
#etch(pageNum: number, count: number, total: number, rows: ItemRow[]): CachePage { #etch(pageNum: number, count: number, total: number, rows: ItemRow[]): CachePage {
+16 -1
View File
@@ -78,7 +78,7 @@ describe("page size", () => {
// settle the RPC promise ourselves: expect().rejects on a DO stub's // settle the RPC promise ourselves: expect().rejects on a DO stub's
// promise leaves the rejection unhandled // promise leaves the rejection unhandled
const failure = () => stub.page(1, PAGE_SIZE).then(() => null, (e: unknown) => String(e)); const failure = () => stub.page(1, PAGE_SIZE).then(() => null, (e: unknown) => String(e));
expect(await failure()).toMatch(/returned 64 items for page 1 of 3 \(asked for 100\)/); expect(await failure()).toMatch(/returned 64 items for page 1 of 3 \(asked for 100\), then 64 for page 2/);
expect(spy.mock.calls.map((c) => c[0]).filter((e) => e?.outcome === "short-page")).toHaveLength(1); expect(spy.mock.calls.map((c) => c[0]).filter((e) => e?.outcome === "short-page")).toHaveLength(1);
// nothing was stored: asking again refuses again // nothing was stored: asking again refuses again
expect(await failure()).toMatch(/page size not honored/); expect(await failure()).toMatch(/page size not honored/);
@@ -87,6 +87,21 @@ describe("page size", () => {
} }
}); });
it("a short page whose next page is empty is the real end: diigo's total overstated, noted not refused", async () => {
const spy = vi.spyOn(console, "log");
try {
const stub = testEnv.CACHE_DO.get(testEnv.CACHE_DO.newUniqueId());
await stub.setup({ webappId: "fake-overstated-webapp" });
// 250 items claimed as 310: page 3 holds the last 50, "of 4"
const page = await stub.page(3, PAGE_SIZE);
expect(page.items).toHaveLength(50);
const notes = spy.mock.calls.map((c) => c[0]).filter((e) => e?.outcome === "total-overstated");
expect(notes).toEqual([expect.objectContaining({ page: 3, total: 310, actual: 250 })]);
} finally {
spy.mockRestore();
}
});
it("the last page may be short", async () => { it("the last page may be short", async () => {
const stub = testEnv.CACHE_DO.get(testEnv.CACHE_DO.newUniqueId()); const stub = testEnv.CACHE_DO.get(testEnv.CACHE_DO.newUniqueId());
await stub.setup({ webappId: WEBAPP_ID }); await stub.setup({ webappId: WEBAPP_ID });
+13 -3
View File
@@ -13,6 +13,10 @@ const TOTAL = 250;
/** the plain account, capped below PAGE_SIZE (as diigo caps at 100) */ /** the plain account, capped below PAGE_SIZE (as diigo caps at 100) */
const CAPPED_WEBAPP = "fake-capped-webapp"; const CAPPED_WEBAPP = "fake-capped-webapp";
const CAP = 64; const CAP = 64;
/** the plain account, but its `total` claims 60 more items than it has
* (as diigo's does, observed 2026-10-07) */
const OVERSTATED_WEBAPP = "fake-overstated-webapp";
const OVERSTATEMENT = 60;
/** a second account, for date filters (goal 1d): 60 bookmarks, item n /** a second account, for date filters (goal 1d): 60 bookmarks, item n
* updated n-1 days + 1 hour before the call, in diigo's updated-desc * updated n-1 days + 1 hour before the call, in diigo's updated-desc
@@ -66,7 +70,7 @@ let calls = 0;
class FakeGet extends RpcTarget { class FakeGet extends RpcTarget {
#path; #path;
#params; #params;
/** null (the plain account), "capped", "dated", or "deep" */ /** null (the plain account), "capped", "overstated", "dated", or "deep" */
#dated; #dated;
constructor(path, params, dated) { constructor(path, params, dated) {
super(); super();
@@ -92,13 +96,19 @@ class FakeGet extends RpcTarget {
for (let n = (page - 1) * count + 1; n <= Math.min(page * count, total); n += 1) { for (let n = (page - 1) * count + 1; n <= Math.min(page * count, total); n += 1) {
items.push(dated ? datedItem(n, now, this.#dated === "deep" ? "deep_dee" : "dated_dan") : item(n)); items.push(dated ? datedItem(n, now, this.#dated === "deep" ? "deep_dee" : "dated_dan") : item(n));
} }
return JSON.stringify({ total, items }); return JSON.stringify({ total: this.#dated === "overstated" ? total + OVERSTATEMENT : total, items });
} }
} }
export class WebAppFactory extends WorkerEntrypoint { export class WebAppFactory extends WorkerEntrypoint {
async at(id) { async at(id) {
const kind = { [KNOWN_WEBAPP]: null, [CAPPED_WEBAPP]: "capped", [DATED_WEBAPP]: "dated", [DEEP_WEBAPP]: "deep" }[id]; const kind = {
[KNOWN_WEBAPP]: null,
[CAPPED_WEBAPP]: "capped",
[OVERSTATED_WEBAPP]: "overstated",
[DATED_WEBAPP]: "dated",
[DEEP_WEBAPP]: "deep",
}[id];
if (kind === undefined) throw new Error("fake webapp: unknown id"); if (kind === undefined) throw new Error("fake webapp: unknown id");
return new FakeGet("", {}, kind); return new FakeGet("", {}, kind);
} }
+8
View File
@@ -131,6 +131,14 @@ describe("fill job (goal 1e)", () => {
expect(status?.error).toMatch(/page size not honored/); expect(status?.error).toMatch(/page size not honored/);
}); });
it("fills to the real end when diigo's total overstates its items", async () => {
const stub = await wired("fake-overstated-webapp");
await stub.startFill();
await drain(stub);
expect(await stub.fillStatus()).toMatchObject({ state: "done" });
expect((await stub.page(3, PAGE_SIZE)).items).toHaveLength(50);
});
it("a job saved under another page size fails instead of resuming; a new start replaces it", async () => { it("a job saved under another page size fails instead of resuming; a new start replaces it", async () => {
const stub = await wired(); const stub = await wired();
// the shape a 24-per-page job left in storage (no pageSize field) // the shape a 24-per-page job left in storage (no pageSize field)