fix(offline): report editor-warm failures honestly; cover failure labels; align docs (F1-F3)
F1: warmPageYdoc now returns didSync (true only on the real 'synced' event,
false on the 8s timeout / error, which are now logged). The tree menu gates
the 'available offline' success toast on result.ok AND didSync, and pushes
'editor' into the failed set otherwise — so a page whose Yjs body never
warmed is no longer reported as fully offline-available.
F2: tests for the page/space/currentUser failure labels and the didSync
true/false paths.
F3: correct the mobile-app-plan / mobile-bootstrap present-state sections to
match shipped code (Capacitor, CORS allowlist, Swagger, offline reading,
/l in the SW denylist).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -326,6 +326,99 @@ describe("makePageAvailableOffline", () => {
|
||||
|
||||
errorSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("records 'page' when the central document fetch (getPageById) rejects", async () => {
|
||||
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
// The central page document fetch fails (the most realistic failure).
|
||||
(getPageById as ReturnType<typeof vi.fn>).mockRejectedValue(
|
||||
new Error("network"),
|
||||
);
|
||||
(getPageBreadcrumbs as ReturnType<typeof vi.fn>).mockResolvedValue([]);
|
||||
(getSidebarPages as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
items: [],
|
||||
meta: { nextCursor: null },
|
||||
});
|
||||
(getPageComments as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
items: [],
|
||||
meta: { nextCursor: null },
|
||||
});
|
||||
|
||||
const result = await makePageAvailableOffline({
|
||||
pageId: "uuid-1",
|
||||
spaceId: "space-uuid",
|
||||
});
|
||||
|
||||
// With no page document, the space step is skipped (no slug), so the only
|
||||
// failure label is "page".
|
||||
expect(result.ok).toBe(false);
|
||||
expect(result.failed).toContain("page");
|
||||
expect(result.failed).not.toContain("space");
|
||||
expect(errorSpy).toHaveBeenCalled();
|
||||
|
||||
errorSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("records 'space' when ONLY the space prefetch rejects", async () => {
|
||||
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
(getPageById as ReturnType<typeof vi.fn>).mockResolvedValue(okPage);
|
||||
(getPageBreadcrumbs as ReturnType<typeof vi.fn>).mockResolvedValue([]);
|
||||
(getSidebarPages as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
items: [],
|
||||
meta: { nextCursor: null },
|
||||
});
|
||||
(getPageComments as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
items: [],
|
||||
meta: { nextCursor: null },
|
||||
});
|
||||
// Fail ONLY the space prefetch (queryKey ["space", slug]); the currentUser
|
||||
// and sidebar-children prefetches still resolve.
|
||||
prefetchQuery.mockImplementation(async (opts: any) => {
|
||||
if (opts?.queryKey?.[0] === "space") throw new Error("network");
|
||||
return undefined;
|
||||
});
|
||||
|
||||
const result = await makePageAvailableOffline({
|
||||
pageId: "uuid-1",
|
||||
spaceId: "space-uuid",
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
expect(result.failed).toContain("space");
|
||||
expect(errorSpy).toHaveBeenCalled();
|
||||
|
||||
errorSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("records 'currentUser' when ONLY the currentUser prefetch rejects", async () => {
|
||||
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
(getPageById as ReturnType<typeof vi.fn>).mockResolvedValue(okPage);
|
||||
(getPageBreadcrumbs as ReturnType<typeof vi.fn>).mockResolvedValue([]);
|
||||
(getSidebarPages as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
items: [],
|
||||
meta: { nextCursor: null },
|
||||
});
|
||||
(getPageComments as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
items: [],
|
||||
meta: { nextCursor: null },
|
||||
});
|
||||
// Fail ONLY the currentUser prefetch (queryKey ["currentUser"]); the space
|
||||
// and sidebar-children prefetches still resolve.
|
||||
prefetchQuery.mockImplementation(async (opts: any) => {
|
||||
if (opts?.queryKey?.[0] === "currentUser") throw new Error("network");
|
||||
return undefined;
|
||||
});
|
||||
|
||||
const result = await makePageAvailableOffline({
|
||||
pageId: "uuid-1",
|
||||
spaceId: "space-uuid",
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
expect(result.failed).toContain("currentUser");
|
||||
expect(errorSpy).toHaveBeenCalled();
|
||||
|
||||
errorSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe("warmPageYdoc", () => {
|
||||
@@ -343,7 +436,8 @@ describe("warmPageYdoc", () => {
|
||||
)![1] as () => void;
|
||||
|
||||
handler();
|
||||
await expect(promise).resolves.toBeUndefined();
|
||||
// Returns true because the real "synced" event fired.
|
||||
await expect(promise).resolves.toBe(true);
|
||||
|
||||
// Listener detached and everything cleaned up.
|
||||
expect(h.providerOff).toHaveBeenCalledWith("synced", expect.any(Function));
|
||||
@@ -358,16 +452,24 @@ describe("warmPageYdoc", () => {
|
||||
expect(h.ydocDestroy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("resolves and cleans up after the timeout when synced never fires", async () => {
|
||||
it("resolves false and cleans up after the timeout when synced never fires", async () => {
|
||||
vi.useFakeTimers();
|
||||
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
const promise = warmPageYdoc("p1", "ws://x");
|
||||
|
||||
// Do not fire "synced"; let the 8s safety timeout settle it.
|
||||
await vi.advanceTimersByTimeAsync(8000);
|
||||
await expect(promise).resolves.toBeUndefined();
|
||||
// Returns false (the doc never synced) and logs the timeout with the pageId.
|
||||
await expect(promise).resolves.toBe(false);
|
||||
expect(errorSpy).toHaveBeenCalledWith(
|
||||
"warmPageYdoc: timed out before sync",
|
||||
{ pageId: "p1" },
|
||||
);
|
||||
|
||||
expect(h.providerDestroy).toHaveBeenCalledTimes(1);
|
||||
expect(h.idbDestroy).toHaveBeenCalledTimes(1);
|
||||
expect(h.ydocDestroy).toHaveBeenCalledTimes(1);
|
||||
|
||||
errorSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -247,16 +247,25 @@ export async function makePageAvailableOffline({
|
||||
* pull the server state into IndexedDB, then tears both down once synced (or
|
||||
* after a timeout). Entirely wrapped in try/catch — NEVER throws.
|
||||
*
|
||||
* Returns true ONLY when the provider's real "synced" event fired — i.e. the
|
||||
* server state actually landed in IndexedDB. The timeout and failure paths
|
||||
* return false (and log with the pageId) so the caller does not report a page
|
||||
* as offline-available when its editor body never warmed. For a wiki the editor
|
||||
* body IS the page, so a silent timeout here is a real misreport.
|
||||
*
|
||||
* Only meaningful when online at warm time; offline it is a no-op that resolves.
|
||||
*/
|
||||
export async function warmPageYdoc(
|
||||
pageId: string,
|
||||
collabUrl: string,
|
||||
token?: string,
|
||||
): Promise<void> {
|
||||
): Promise<boolean> {
|
||||
let ydoc: Y.Doc | null = null;
|
||||
let local: IndexeddbPersistence | null = null;
|
||||
let remote: HocuspocusProvider | null = null;
|
||||
// Flipped to true ONLY inside the real "synced" handler; the timeout/failure
|
||||
// paths leave it false. Returned so the caller can record a failed editor warm.
|
||||
let didSync = false;
|
||||
|
||||
try {
|
||||
const documentName = pageYdocName(pageId);
|
||||
@@ -274,27 +283,38 @@ export async function warmPageYdoc(
|
||||
await new Promise<void>((resolve) => {
|
||||
let settled = false;
|
||||
let timeoutId: ReturnType<typeof setTimeout> | undefined;
|
||||
const finish = () => {
|
||||
// `synced` is true only when called from the real "synced" handler; the
|
||||
// timeout path passes false so didSync stays false on a give-up.
|
||||
const finish = (synced: boolean) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
didSync = synced;
|
||||
// Clear the pending timeout and detach the listener so neither leaks
|
||||
// after we resolve.
|
||||
if (timeoutId !== undefined) clearTimeout(timeoutId);
|
||||
try {
|
||||
provider.off("synced", finish);
|
||||
provider.off("synced", onSynced);
|
||||
} catch {
|
||||
// best-effort
|
||||
}
|
||||
if (!synced) {
|
||||
// Gave up before the server synced: the page body never landed in
|
||||
// IndexedDB. Log with the pageId (parity with the other warm steps)
|
||||
// so the caller can report the editor step as failed.
|
||||
console.error("warmPageYdoc: timed out before sync", { pageId });
|
||||
}
|
||||
resolve();
|
||||
};
|
||||
|
||||
const onSynced = () => finish(true);
|
||||
|
||||
// Resolve once the server state has synced into the local doc...
|
||||
provider.on("synced", finish);
|
||||
provider.on("synced", onSynced);
|
||||
// ...or give up after a short timeout so we never hang.
|
||||
timeoutId = setTimeout(finish, 8000);
|
||||
timeoutId = setTimeout(() => finish(false), 8000);
|
||||
});
|
||||
} catch {
|
||||
// best-effort
|
||||
} catch (error) {
|
||||
console.error("warmPageYdoc: warm failed", { pageId, error });
|
||||
} finally {
|
||||
try {
|
||||
remote?.destroy();
|
||||
@@ -312,4 +332,6 @@ export async function warmPageYdoc(
|
||||
// best-effort
|
||||
}
|
||||
}
|
||||
|
||||
return didSync;
|
||||
}
|
||||
|
||||
@@ -90,18 +90,29 @@ export function NodeMenu({ node, canEdit }: NodeMenuProps) {
|
||||
pageId: node.id,
|
||||
spaceId: node.spaceId,
|
||||
});
|
||||
// Best-effort: warm the page's Yjs document into IndexedDB.
|
||||
await warmPageYdoc(node.id, getCollaborationUrl(), collabQuery?.token);
|
||||
// Warm the page's Yjs document into IndexedDB. For a wiki the editor body
|
||||
// IS the page, so this only truly succeeds when the doc actually synced;
|
||||
// a timeout/failure here must NOT be reported as offline-available.
|
||||
const ydocSynced = await warmPageYdoc(
|
||||
node.id,
|
||||
getCollaborationUrl(),
|
||||
collabQuery?.token,
|
||||
);
|
||||
|
||||
if (result.ok) {
|
||||
// Fold a failed editor warm into the failed-step set so it surfaces in the
|
||||
// same error UI as the read-query failures (the editor body never landed
|
||||
// in IndexedDB, so the page would open blank offline).
|
||||
const failed = ydocSynced ? result.failed : [...result.failed, "editor"];
|
||||
|
||||
if (result.ok && ydocSynced) {
|
||||
notifications.show({ message: t("Page is now available offline") });
|
||||
} else {
|
||||
// Partial warm — the page may still be partly usable offline, but some
|
||||
// queries failed to cache, so surface it as an error rather than a
|
||||
// silent success. Name the failed step(s) (AGENTS.md: errors must be
|
||||
// specific, never a bare generic string); `result.failed` carries them.
|
||||
// queries (or the editor body) failed to cache, so surface it as an
|
||||
// error rather than a silent success. Name the failed step(s) (AGENTS.md:
|
||||
// errors must be specific, never a bare generic string).
|
||||
notifications.show({
|
||||
message: `${t("Failed to make page available offline")}: ${result.failed.join(", ")}`,
|
||||
message: `${t("Failed to make page available offline")}: ${failed.join(", ")}`,
|
||||
color: "red",
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user