fix: une panne n'est plus mise en cache comme une absence de capacité

This commit is contained in:
Sylvain Duchesne
2026-09-15 12:12:47 +02:00
parent 817f9074c4
commit e5d73a670b
2 changed files with 181 additions and 40 deletions
@@ -102,10 +102,28 @@ import type { Nuri, ReadCap } from "../model/types";
* The round-trip is what is worth saving, not the filing. So the memo holds the value and * The round-trip is what is worth saving, not the filing. So the memo holds the value and
* the caller files it for whoever is connected, every time. * the caller files it for whoever is connected, every time.
* *
* A scope never changes here (a document is created in a store and stays there), so a * ── What may be memoised is an ANSWER — never a failure ───────────────────
* cached `null` cannot go stale for a document that existed when it was taken. It CAN for * Two values are filed here, and each is safe to keep for its OWN reason:
* one created afterwards in the same page — {@link resetPublicStoreFetches} is the way * - a **cap** — the document served it, and nothing withdraws it within a session;
* out, and a session or wallet reset calls it. * - **`null`** — the document's Header branch was READ and carries no exposed cap: an
* ESTABLISHED absence. Safe because a scope never changes here (a document is created
* in a store and stays there), so that answer cannot go stale for a document that
* existed when it was taken.
* A fetch that THREW is neither of those. The broker was not asked, or could not answer,
* and "no answer" is not "no cap" — so it is dropped from the map the moment it rejects
* and the next caller asks again. {@link downloadReadCap} is what keeps the two apart.
*
* That separation repairs a defect that stood until 2026-09-15: the download caught its
* own errors and returned the same `null` as a real absence, so ONE transient outage
* recorded "no capability" for that document for the rest of the session. Deposits being
* consumed, a deposit on that document was then retired before its arm could read
* anything — and every re-deposit sent to repair it met the same memoised `null` and was
* destroyed too. The justification above was written when `null` meant absence and
* ONLY absence; it was left standing when the `catch` gave the value a second meaning it
* never covered. It is now stated per value, so it cannot silently widen again.
*
* A cached absence CAN still go stale for a document created afterwards in the same page —
* {@link resetPublicStoreFetches} is the way out, and a session or wallet reset calls it.
*/ */
const attempted = new Map<Nuri, Promise<ReadCap | null>>(); const attempted = new Map<Nuri, Promise<ReadCap | null>>();
@@ -163,8 +181,10 @@ export async function exposeReadCap(doc: Nuri, cap: ReadCap): Promise<void> {
* Nothing is asked when the cap is already held: a document you can read needs no * Nothing is asked when the cap is already held: a document you can read needs no
* fetching, and skipping it keeps the ordinary path free of physical reads. * fetching, and skipping it keeps the ordinary path free of physical reads.
* *
* Never throws — a document that is not in a public store simply answers nothing, which * Never throws. Two different negatives reach the caller as the same `false`, and the
* is not an error but the normal case. * caller is right not to tell them apart — it obtained no cap either way. They are kept
* apart where it matters, in the memo: an absence is remembered, a failure is not (see
* the note on {@link attempted}).
*/ */
// @provenance fetchReadCap kind=declared-not-wired level=1 ref=engine/net/src/types.rs:PublicRepoLinkV0 — the type DECLARES the cap is downloaded from the outer overlay if the brokers allow; it is constructed nowhere, both `PinRepo` constructors hard-code `expose_outer: false`, and `ExtTopicSyncReq` is `unimplemented!()` // @provenance fetchReadCap kind=declared-not-wired level=1 ref=engine/net/src/types.rs:PublicRepoLinkV0 — the type DECLARES the cap is downloaded from the outer overlay if the brokers allow; it is constructed nowhere, both `PinRepo` constructors hard-code `expose_outer: false`, and `ExtTopicSyncReq` is `unimplemented!()`
export async function fetchReadCap(docLike: Nuri): Promise<boolean> { export async function fetchReadCap(docLike: Nuri): Promise<boolean> {
@@ -179,8 +199,29 @@ export async function fetchReadCap(docLike: Nuri): Promise<boolean> {
if (pending === undefined) { if (pending === undefined) {
pending = downloadReadCap(doc); pending = downloadReadCap(doc);
attempted.set(doc, pending); attempted.set(doc, pending);
// A failure is not an answer, so it must not be filed as one: drop the memo the
// moment the download rejects, and the next caller reaches the broker again.
// Attached HERE — once per download, before anybody awaits — so the eviction and the
// report each happen exactly once however many callers share this one in-flight
// fetch, and happen at all even if every one of them has already given up. The
// identity check is what keeps it from evicting a LATER attempt: by the time a
// rejection lands, the map may already hold a fresh promise for this document.
const started = pending;
void started.catch((error: unknown) => {
if (attempted.get(doc) === started) attempted.delete(doc);
console.error(accessLogPrefix() + " fetchReadCap failed:", error);
});
}
let cap: ReadCap | null;
try {
cap = await pending;
} catch {
// Reported and forgotten by the handler attached where the download started, just
// above — not swallowed here. The answer to the caller is the same as for an absence
// (no cap was obtained), but the memo now holds nothing for this document, so the
// next call asks the network instead of replaying this failure.
return false;
} }
const cap = await pending;
if (cap === null) return false; if (cap === null) return false;
// Filed for whoever is connected NOW, on every call — the memo spares the round-trip, // Filed for whoever is connected NOW, on every call — the memo spares the round-trip,
// not the filing. See the note on {@link attempted}. // not the filing. See the note on {@link attempted}.
@@ -189,33 +230,38 @@ export async function fetchReadCap(docLike: Nuri): Promise<boolean> {
return true; return true;
} }
/** The fetch itself, through the machinery's door. Returns the cap, or `null` when the /**
* document is not in a public store — which is the normal case, not an error. */ * The fetch itself, through the machinery's door. **Two outcomes, deliberately not one** —
* telling them apart is this function's whole shape:
*
* - it **resolves** — the Header branch was READ. With the cap when the document exposes
* one; with `null` when it does not, which is the normal case and not an error. Either
* way the network answered, so the value is an answer and {@link attempted} may keep it.
* - it **rejects** — the question could not be put at all: the repo would not open, the
* broker did not answer, the read threw. That is "no answer", not "no cap", and
* {@link fetchReadCap} keeps it out of the memo for exactly that reason.
*
* So nothing is caught here. It used to be, and the two collapsed into the same `null`.
*/
async function downloadReadCap(doc: Nuri): Promise<ReadCap | null> { async function downloadReadCap(doc: Nuri): Promise<ReadCap | null> {
const s = await session(); const s = await session();
try { // The repo has to be in the session before an anchored read resolves it — the
// The repo has to be in the session before an anchored read resolves it — the // cold-start heal, through the PHYSICAL door: this is the emulated broker serving
// cold-start heal, through the PHYSICAL door: this is the emulated broker serving // an outer overlay, and it does not ask who is asking (see `open-repo.ts`).
// an outer overlay, and it does not ask who is asking (see `open-repo.ts`). await ensurePhysicalRepoOpen(doc);
await ensurePhysicalRepoOpen(doc); const res = await physicalQuery(
const res = await physicalQuery( s.sessionId,
s.sessionId, `SELECT ?c WHERE { <${HEADER_BRANCH_SUBJECT}> <${P.exposedReadCap}> ?c }`,
`SELECT ?c WHERE { <${HEADER_BRANCH_SUBJECT}> <${P.exposedReadCap}> ?c }`, undefined,
undefined, doc,
doc, "fetchReadCap",
"fetchReadCap", );
); for (const row of readBindings(res)) {
for (const row of readBindings(res)) { const cap = bindingValue(row, "c");
const cap = bindingValue(row, "c"); // `targetOf` guards the one confusion that would matter: a cap exposed on
// `targetOf` guards the one confusion that would matter: a cap exposed on // document A must not file a cap for document B. A document only ever speaks
// document A must not file a cap for document B. A document only ever speaks // for itself.
// for itself. if (cap && hasReadCap(cap) && targetOf(cap) === doc) return cap;
if (cap && hasReadCap(cap) && targetOf(cap) === doc) return cap;
}
} catch (error) {
// Not in a public store, not synced, or no such document — all of them mean the
// same thing to the caller: no cap was obtained.
console.error(accessLogPrefix() + " fetchReadCap failed:", error);
} }
return null; return null;
} }
+103 -8
View File
@@ -33,19 +33,27 @@ function inject() {
if (m) quads.push({ g: anchor, s: m[1]!, p: m[2]!, o: m[3]! }); if (m) quads.push({ g: anchor, s: m[1]!, p: m[2]!, o: m[3]! });
return undefined; return undefined;
}); });
const sparql_query = mock(async (...a: unknown[]) => ({ // Documents the emulated broker cannot be asked about right now — an outage, a repo
results: { // this session has not synced, a rejected read. Scoped to ONE anchor on purpose: the
bindings: quads // connection work `setCurrentUser` fires reads on its own account, and an outage that
.filter((q) => q.g === (a[3] as string) && q.p === `${SHIM}:exposedReadCap`) // took those down would be testing something else.
.map((q) => ({ c: { value: q.o } })), const unreachable = new Set<string>();
}, const sparql_query = mock(async (...a: unknown[]) => {
})); if (unreachable.has(a[3] as string)) throw new Error("emulated broker: cannot serve that overlay");
return {
results: {
bindings: quads
.filter((q) => q.g === (a[3] as string) && q.p === `${SHIM}:exposedReadCap`)
.map((q) => ({ c: { value: q.o } })),
},
};
});
configure({ ng: { doc_create: mock(async () => "did:ng:o:x"), sparql_update, sparql_query } as any, useShape: (() => {}) as any }); configure({ ng: { doc_create: mock(async () => "did:ng:o:x"), sparql_update, sparql_query } as any, useShape: (() => {}) as any });
configureStoreRegistry({ getSession: async () => SESSION }); configureStoreRegistry({ getSession: async () => SESSION });
resetCaps(); resetCaps();
resetPublicStoreFetches(); resetPublicStoreFetches();
setCurrentUser(null); setCurrentUser(null);
return { sparql_query, quads }; return { sparql_query, quads, unreachable };
} }
afterEach(() => { afterEach(() => {
@@ -186,3 +194,90 @@ test("resetting the caps forgets the memo — a stale yes would hand back what i
expect(await fetchReadCap(PUB)).toBe(true); expect(await fetchReadCap(PUB)).toBe(true);
expect(sparql_query.mock.calls.length).toBeGreaterThan(before); // asked again expect(sparql_query.mock.calls.length).toBeGreaterThan(before); // asked again
}); });
// ── A failure is not an answer ───────────────────────────────────────────────
// REGRESSION (2026-09-15). `downloadReadCap` caught its own errors and returned the same
// `null` as a genuine absence, which the memo then held for the whole session: ONE
// transient outage recorded "no capability" for that document until the session ended.
// Deposits being consumed, the deposit was retired before its arm read anything — and
// every re-deposit sent to repair it met the same memoised `null` and was destroyed too.
/** How many times the network was asked ABOUT `nuri` — anchored reads only, so the
* connection work happening in the background on other documents does not count. */
function asksAbout(sparql_query: { mock: { calls: unknown[][] } }, nuri: string): number {
return sparql_query.mock.calls.filter((c) => c[3] === nuri).length;
}
test("a fetch that FAILED is asked again, and the cap arrives once the broker recovers", async () => {
const { sparql_query, unreachable } = inject();
setCurrentUser("alice");
await aliceExposesHerNote();
armEmulation();
setCurrentUser("bob");
unreachable.add(PUB); // the broker cannot answer for this document
expect(await fetchReadCap(PUB)).toBe(false);
expect(asksAbout(sparql_query, PUB)).toBe(1);
expect(getCaps().capFor(PUB)).toBeUndefined();
unreachable.delete(PUB); // …and recovers
expect(await fetchReadCap(PUB)).toBe(true);
expect(asksAbout(sparql_query, PUB)).toBe(2); // asked AGAIN — the outage was not an answer
expect(getCaps().capFor(PUB)).toBe(mintCap(PUB));
});
// The two negatives in one test, so neither half can be "fixed" by sacrificing the other:
// an absence was READ off the Header branch and is kept; a failure was never read at all.
test("an absence is remembered, a failure is not — the two negatives are not the same", async () => {
const { sparql_query, unreachable } = inject();
armEmulation();
setCurrentUser("bob");
const absent = "did:ng:o:exposes-nothing" as Nuri;
expect(await fetchReadCap(absent)).toBe(false);
expect(await fetchReadCap(absent)).toBe(false);
expect(asksAbout(sparql_query, absent)).toBe(1); // read once, and the answer stands
const unreached = "did:ng:o:cannot-ask" as Nuri;
unreachable.add(unreached);
expect(await fetchReadCap(unreached)).toBe(false);
expect(await fetchReadCap(unreached)).toBe(false);
expect(asksAbout(sparql_query, unreached)).toBe(2); // never answered, so asked every time
});
// The memo's OTHER job. Losing it would turn one cold read by N holders into N round-trips.
test("concurrent askers share ONE failing round-trip, and exactly one re-ask follows", async () => {
const { sparql_query, unreachable } = inject();
armEmulation();
setCurrentUser("bob");
unreachable.add(PUB);
const answers = await Promise.all([fetchReadCap(PUB), fetchReadCap(PUB), fetchReadCap(PUB)]);
expect(answers).toEqual([false, false, false]);
expect(asksAbout(sparql_query, PUB)).toBe(1); // one in-flight fetch served all three
expect(await fetchReadCap(PUB)).toBe(false);
expect(asksAbout(sparql_query, PUB)).toBe(2); // and the failure left nothing behind
});
// `fetchReadCap` files for whoever is connected NOW, on EVERY call — the memo spares the
// round-trip, not the filing. A re-attempt after a failure must obey that too, or the
// recovered cap would land in the ring of whoever happened to hit the outage.
test("a cap obtained after a failure is filed for whoever is connected THEN", async () => {
const { unreachable } = inject();
setCurrentUser("alice");
await aliceExposesHerNote();
armEmulation();
setCurrentUser("bob");
unreachable.add(PUB);
expect(await fetchReadCap(PUB)).toBe(false);
unreachable.delete(PUB);
setCurrentUser("carol");
expect(await fetchReadCap(PUB)).toBe(true);
expect(getCaps().capFor(PUB)).toBe(mintCap(PUB)); // carol asked, carol holds it
setCurrentUser("bob");
expect(getCaps().capFor(PUB)).toBeUndefined(); // bob only ever met the outage
});