fix(wikis): stop swallowing fatal errors and marking failed scans complete
`getWikiPage`'s attachments fetch had a bare `catch` that swallowed 401/403/429 one function away from discovery code that correctly rethrows them. During a 50-page discovery scan a rate limit was eaten once per page and every page cached with `attachments: []`, then reported to the model as fact. The discovery probe loop incremented `consecutiveMisses` only on 404, so a run where every probe failed with a 500 or a network error never tripped the cutoff, never rethrew, and cached nothing — yet still ran `discoveredCacheKeys.add(cacheKey)` unconditionally, marking discovery complete. Every later wiki search then short-circuited on an empty corpus and returned `[]` for the life of the process. Discovery is now marked complete only when at least one page was cached or the loop ended via the 404 cutoff. An all-errors scan satisfies neither, so it retries; a genuinely empty wiki trips the cutoff and is still marked complete rather than re-probed on every search. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
216b460c56
commit
a8d3c23096
+26
-3
@@ -11,6 +11,7 @@ import { extractIdFromHref, extractRawText } from "../client/hal-parser.ts";
|
||||
import type { HalCollection, HalLink, HalResource } from "../client/types.ts";
|
||||
import { resolveClient, resolveProjectId } from "./helper.ts";
|
||||
import { rankRecords, type FieldSpec, type MatchMode } from "../search/rank.ts";
|
||||
import { isFatalSearchError } from "../search/pipeline.ts";
|
||||
|
||||
export interface WikiPageAttachment {
|
||||
id: number;
|
||||
@@ -216,8 +217,15 @@ export async function getWikiPage(
|
||||
if (attachmentsResponse._embedded?.elements) {
|
||||
attachments = attachmentsResponse._embedded.elements.map(normalizeWikiAttachment);
|
||||
}
|
||||
} catch {
|
||||
// If attachments collection cannot be retrieved, default to empty list
|
||||
} catch (error: unknown) {
|
||||
// A 401/403/429 here is not "this page has no attachments" — it means we
|
||||
// could not read. Swallowing it caches (and reports) an empty attachment
|
||||
// list as fact, and during a discovery scan eats the same rate limit once
|
||||
// per probed page.
|
||||
if (isFatalSearchError(error)) {
|
||||
throw error;
|
||||
}
|
||||
// Otherwise (404, 500, network) degrade to an empty list.
|
||||
}
|
||||
|
||||
const detail = normalizeWikiPageDetail(pageResource, attachments);
|
||||
@@ -324,6 +332,9 @@ export async function searchWikiPages(
|
||||
const MAX_PROBE_ID = 50;
|
||||
const CONSECUTIVE_404_CUTOFF = 5;
|
||||
let consecutiveMisses = 0;
|
||||
// True only when the scan stopped because the 404 cutoff said "the wiki
|
||||
// ends here", as opposed to erroring its way through every probe.
|
||||
let reachedMissCutoff = false;
|
||||
|
||||
for (let id = 1; id <= MAX_PROBE_ID; id++) {
|
||||
try {
|
||||
@@ -343,6 +354,7 @@ export async function searchWikiPages(
|
||||
consecutiveMisses++;
|
||||
}
|
||||
if (consecutiveMisses >= CONSECUTIVE_404_CUTOFF) {
|
||||
reachedMissCutoff = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
@@ -368,7 +380,18 @@ export async function searchWikiPages(
|
||||
}
|
||||
}
|
||||
|
||||
discoveredCacheKeys.add(cacheKey);
|
||||
// Mark discovery complete only when it genuinely completed. If every
|
||||
// probe failed with a 500 or a network error the cutoff never trips and
|
||||
// nothing is cached — marking that scan "done" would short-circuit every
|
||||
// later search for the life of the process and return [] forever.
|
||||
// Either condition on its own is proof of a real scan: a cached page
|
||||
// means we read the wiki, and the 404 cutoff means we read to its end
|
||||
// (a genuinely empty wiki is 404s all the way down, and must still be
|
||||
// marked complete or every search re-probes it).
|
||||
const discoveryCompleted = cacheMap.size > 0 || reachedMissCutoff;
|
||||
if (discoveryCompleted) {
|
||||
discoveredCacheKeys.add(cacheKey);
|
||||
}
|
||||
}
|
||||
|
||||
// 3. Resolve target project ID if provided
|
||||
|
||||
@@ -729,3 +729,118 @@ describe("Wiki Fuzzy Search", () => {
|
||||
expect(results).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("Wiki Error Propagation", () => {
|
||||
beforeEach(() => {
|
||||
clearWikiCache();
|
||||
});
|
||||
|
||||
function pageWithFailingAttachments(error: unknown) {
|
||||
return {
|
||||
baseUrl: "http://localhost:8080",
|
||||
getCacheKey: () => "attachments-test",
|
||||
get: async (path: string) => {
|
||||
if (path.endsWith("/attachments")) {
|
||||
throw error;
|
||||
}
|
||||
if (path === "/api/v3/wiki_pages/1") {
|
||||
return {
|
||||
_type: "WikiPage",
|
||||
id: 1,
|
||||
title: "Project Wiki",
|
||||
_links: { project: { href: "/api/v3/projects/1", title: "Demo Project" } },
|
||||
};
|
||||
}
|
||||
throw new OpenProjectError("not found", { statusCode: 404 });
|
||||
},
|
||||
} as unknown as OpenProjectClient;
|
||||
}
|
||||
|
||||
test("getWikiPage propagates a rate limit from the attachments fetch", async () => {
|
||||
await expect(
|
||||
getWikiPage(
|
||||
1,
|
||||
pageWithFailingAttachments(new OpenProjectError("rate limited", { statusCode: 429 }))
|
||||
)
|
||||
).rejects.toThrow("rate limited");
|
||||
});
|
||||
|
||||
test("getWikiPage propagates an authentication failure from the attachments fetch", async () => {
|
||||
await expect(
|
||||
getWikiPage(1, pageWithFailingAttachments(new OpenProjectAuthenticationError()))
|
||||
).rejects.toThrow(OpenProjectAuthenticationError);
|
||||
});
|
||||
|
||||
test("getWikiPage still degrades to an empty attachment list on a non-fatal failure", async () => {
|
||||
const page = await getWikiPage(
|
||||
1,
|
||||
pageWithFailingAttachments(new OpenProjectError("boom", { statusCode: 500 }))
|
||||
);
|
||||
expect(page.id).toBe(1);
|
||||
expect(page.attachments).toEqual([]);
|
||||
});
|
||||
|
||||
test("a discovery scan that errors through every probe is not marked complete", async () => {
|
||||
// Phase 1: every wiki page probe fails with a 500. Nothing is cached, and
|
||||
// the 404 cutoff never trips, so discovery did not actually complete.
|
||||
let failEverything = true;
|
||||
const client = {
|
||||
baseUrl: "http://localhost:8080",
|
||||
getCacheKey: () => "flaky-tenant",
|
||||
get: async (path: string) => {
|
||||
if (path === "/api/v3/wiki_page_links") {
|
||||
return { _type: "Collection", total: 0, _embedded: { elements: [] } };
|
||||
}
|
||||
if (failEverything) {
|
||||
throw new OpenProjectError("upstream exploded", { statusCode: 500 });
|
||||
}
|
||||
if (path === "/api/v3/wiki_pages/1") {
|
||||
return {
|
||||
_type: "WikiPage",
|
||||
id: 1,
|
||||
title: "Deployment Runbook",
|
||||
text: { raw: "How to deploy the service" },
|
||||
_links: { project: { href: "/api/v3/projects/1", title: "Demo Project" } },
|
||||
};
|
||||
}
|
||||
if (path === "/api/v3/wiki_pages/1/attachments") {
|
||||
return { _type: "Collection", total: 0, _embedded: { elements: [] } };
|
||||
}
|
||||
throw new OpenProjectError("not found", { statusCode: 404 });
|
||||
},
|
||||
} as unknown as OpenProjectClient;
|
||||
|
||||
const first = await searchWikiPages({ query: "deployment" }, client);
|
||||
expect(first).toEqual([]);
|
||||
|
||||
// Phase 2: the instance recovers. Discovery must run again rather than
|
||||
// short-circuiting on a cache that was wrongly marked complete and
|
||||
// returning [] for the life of the process.
|
||||
failEverything = false;
|
||||
const second = await searchWikiPages({ query: "deployment" }, client);
|
||||
expect(second.map((page) => page.id)).toEqual([1]);
|
||||
});
|
||||
|
||||
test("a genuinely empty wiki is marked complete and is not re-probed", async () => {
|
||||
let probes = 0;
|
||||
const client = {
|
||||
baseUrl: "http://localhost:8080",
|
||||
getCacheKey: () => "empty-tenant",
|
||||
get: async (path: string) => {
|
||||
if (path === "/api/v3/wiki_page_links") {
|
||||
return { _type: "Collection", total: 0, _embedded: { elements: [] } };
|
||||
}
|
||||
probes++;
|
||||
throw new OpenProjectError("not found", { statusCode: 404 });
|
||||
},
|
||||
} as unknown as OpenProjectClient;
|
||||
|
||||
expect(await searchWikiPages({ query: "anything" }, client)).toEqual([]);
|
||||
const afterFirst = probes;
|
||||
// The 404 cutoff is a real completion signal: five consecutive misses.
|
||||
expect(afterFirst).toBe(5);
|
||||
|
||||
expect(await searchWikiPages({ query: "anything" }, client)).toEqual([]);
|
||||
expect(probes).toBe(afterFirst);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user