fix: un dépôt est délivré une fois et retiré, comme en amont
This commit is contained in:
Sylvain Duchesne
@@ -0,0 +1,212 @@
|
||||
/**
|
||||
* Consuming an inbox — the ONE act that retires a deposit, and the ONE loop that performs it.
|
||||
*
|
||||
* ── Upstream pops, and the order is not an accident ───────────────────────
|
||||
* Read at all three of its sites, because the order is the whole contract:
|
||||
*
|
||||
* 1. the broker REMOVES the message from the queue — `ClientEvent::InboxPopRequest` →
|
||||
* `inbox_pop_for_user` → `take_first_msg` → `MSGS.take_first_value`, which is
|
||||
* `get_first_key_value` followed by `del` (`engine/repo/src/kcv_storage.rs:557`,
|
||||
* `engine/storage-rocksdb/src/kcv_storage.rs:239-243`). The removal happens in the
|
||||
* STORE, before anything has looked at the content;
|
||||
* 2. only then is it delivered (`ClientMessageContentV0::InboxReceive { from_queue: true }`,
|
||||
* `engine/net/src/actors/client/client_event.rs:68`);
|
||||
* 3. the verifier applies it, and a failure is LOGGED AND SWALLOWED —
|
||||
* `log_err!("Error during process_inbox {e}")` — never re-enqueued. It then asks for
|
||||
* the next one (`engine/verifier/src/verifier.rs:1680-1715`).
|
||||
*
|
||||
* So: **remove, then apply, then log whatever went wrong, then move on.** A deposit is
|
||||
* delivered exactly once whether or not its arm succeeded, and an arm that throws loses it
|
||||
* — upstream that message left the store before the arm ever ran. Removing FIRST is
|
||||
* therefore not a risk this module takes, it is the behaviour it reproduces; and it makes
|
||||
* the reverse mistake impossible, since a removal that fails stops the apply instead of
|
||||
* letting an applied deposit stay in the queue.
|
||||
*
|
||||
* ── Why a deliverability question exists here and not upstream ────────────
|
||||
* Upstream an inbox holds nothing but typed messages, and WHO may pop one is settled by
|
||||
* reader registration rather than by content: `inbox_pop_for_user(user)` pops only from
|
||||
* inboxes that user is a registered reader of, so a message is never popped by anybody but
|
||||
* its own recipient. Here an inbox holds a MIX — this package's deposits and a consumer's
|
||||
* own payloads, which `surface/inbox.ts` hands back untouched — so "may this be popped
|
||||
* now?" has to be asked of the content. That is `inbox-dispatch.claimsDeposit`, and it is
|
||||
* asked BEFORE the removal for the same reason the broker checks registration before
|
||||
* `take_first_msg`: what is not deliverable to this consumer was never popped for it.
|
||||
*
|
||||
* ── One removal, and it is here ───────────────────────────────────────────
|
||||
* Both triggers this package drains from — the owner's own session (`inbox.processInbox`)
|
||||
* and the deferred stand-in for an absent owner (`inbox.processForOwner`, scheduled by
|
||||
* `emulated-verifier/inbox-processor.ts`) — call {@link deliverDeposits} and nothing else.
|
||||
* Neither carries a copy of the removal: the triggers differ in WHO drains and WHEN, never
|
||||
* in what consuming means. `grep -rn "DELETE" packages/polyfill/src/` shows the only
|
||||
* deposit removal in the package is {@link removeDeposit} below.
|
||||
*/
|
||||
|
||||
import { registerUpdate } from "./register-write";
|
||||
import { applyDeposit, claims, claimsDeposit } from "./inbox-dispatch";
|
||||
import type { Applying } from "./inbox-dispatch";
|
||||
import { getStoreRegistryDeps } from "../shared-wallet/bootstrap";
|
||||
import { accessLogPrefix, logStage, shortNuri } from "../shared-wallet/access-log";
|
||||
|
||||
/**
|
||||
* One deposit as it is STORED: the RDF subject `inbox.post` minted for it, and what it
|
||||
* carries. The subject is the removal key — `post` gives every deposit a fresh unique
|
||||
* subject, so naming it retires that deposit and no other, where a `ts` could collide.
|
||||
*
|
||||
* Deliberately NOT the published {@link import("../surface/inbox").Deposit}: a subject is
|
||||
* emulation detail (upstream a queued message has no address of its own, it has a
|
||||
* position), so it stays inside the package and an application never learns it.
|
||||
*/
|
||||
export interface StoredDeposit {
|
||||
/** The RDF subject the deposit lives under, in its inbox document's default graph. */
|
||||
readonly subject: string;
|
||||
/** What the deposit carries — handed to the dispatch as-is. */
|
||||
readonly payload: unknown;
|
||||
}
|
||||
|
||||
/**
|
||||
* Retire ONE deposit from its inbox. **The only removal in this package.**
|
||||
*
|
||||
* `DELETE WHERE { <subject> ?p ?o }` — every triple of that one deposit, whichever
|
||||
* predicates it happens to carry (`from` is optional). Two established forms are combined
|
||||
* here, and neither half is invented:
|
||||
*
|
||||
* - `DELETE WHERE` with a VARIABLE predicate (`<s> ?p ?o`) is the shape verified against
|
||||
* the real broker for removing a whole subject
|
||||
* (`docs/decisions/sparql-delete-for-orm-objects.md`);
|
||||
* - it is ANCHORED with no `GRAPH <…>` wrapper, like every other register write in this
|
||||
* package (`public-store.exposeReadCap`, `branch-registers.publishInboxAddress`). That
|
||||
* is not a shortcut, it is where the data IS: `inbox.post` writes the anchored DEFAULT
|
||||
* graph and `DEPOSITS_QUERY` reads it, so a `GRAPH`-wrapped delete would name a graph
|
||||
* the deposits were never written to. The decision above shows the wrapped form because
|
||||
* its subject is an ORM object in a named graph; the anchor plays that role here.
|
||||
*
|
||||
* A `;`-joined compound update is exercised nowhere in this library, so this stays one
|
||||
* statement.
|
||||
*
|
||||
* Through `registerUpdate`, and that is deliberate: retiring a deposit is the verifier's
|
||||
* own bookkeeping, not an application write — upstream it happens inside the broker's
|
||||
* store, and no application makes it. That door skips the OWNERSHIP guard, which a drain
|
||||
* running for an absent owner could not pass, and keeps `assertMayReach` against the named
|
||||
* ring: whoever consumes must hold the inbox's cap, and `processForOwner` names the
|
||||
* owner's ring exactly as it does for every other step it takes.
|
||||
*
|
||||
* A failure PROPAGATES. Nothing is applied for a deposit that could not be retired — the
|
||||
* caller's loop stops and its drain reports itself where drains report — because applying
|
||||
* one that stayed in the queue is the replay this module exists to end.
|
||||
*/
|
||||
// @provenance removeDeposit kind=aligned level=1 ref=engine/broker/src/server_storage/core/inbox.rs:take_first_msg — upstream the store REMOVES the message as it hands it over (`MSGS.take_first_value` = `get_first_key_value` + `del`); here the queue is RDF in a document, so the same act is a DELETE of the deposit's own subject
|
||||
async function removeDeposit(stored: StoredDeposit, ctx: Applying): Promise<void> {
|
||||
const { sessionId } = await getStoreRegistryDeps().getSession();
|
||||
await registerUpdate(
|
||||
sessionId,
|
||||
`DELETE WHERE { <${stored.subject}> ?p ?o }`,
|
||||
ctx.inbox,
|
||||
"inboxConsume",
|
||||
ctx.ring,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Deliver every deposit in `stored` to the dispatch, consuming each as it goes. Answers
|
||||
* how many produced an EFFECT — which is what a drain traces, and all any caller wants.
|
||||
*
|
||||
* The loop both triggers share, so that "what consuming means" cannot come to differ
|
||||
* between the owner's own drain and the deferred one. Per deposit, in this order:
|
||||
*
|
||||
* 1. **claimed?** — a payload no arm answers for is not popped at all, and the loop
|
||||
* moves on leaving it in the inbox. That covers a consumer's own data (not a message,
|
||||
* so nothing is reported) and a deposit whose arm belongs to the owner's own session
|
||||
* while a stranger's is running (see `inbox-dispatch.claimsDeposit`);
|
||||
* 2. **remove** — the pop, before anything is applied, as upstream;
|
||||
* 3. **apply** — the `match`, one arm per declared type. A refusal inside an arm is
|
||||
* reported on the log stream and the deposit is already gone, which is upstream's
|
||||
* `log_err!` + move on, and the end of the re-report-forever loop this package had.
|
||||
*
|
||||
* Sequential rather than parallel, for the reason every drain in this package is: an arm
|
||||
* writes what it applies (`branch-registers.addLink`, on the owner's User branch), so two
|
||||
* in flight over one queue interleave their reads and writes.
|
||||
*/
|
||||
// @provenance deliverDeposits kind=aligned level=1 ref=engine/verifier/src/verifier.rs:inbox — upstream's pop-apply-log-next loop, over a queue that is a document here rather than a column family
|
||||
export async function deliverDeposits(
|
||||
stored: readonly StoredDeposit[],
|
||||
ctx: Applying,
|
||||
): Promise<number> {
|
||||
let applied = 0;
|
||||
for (const deposit of stored) {
|
||||
// The holder can move between two arms as easily as during the read that found these.
|
||||
// Stopping leaves every deposit not yet reached in the queue for its owner — which is
|
||||
// the whole reason this is asked BEFORE the pop rather than after it.
|
||||
if (!holderStillStands(ctx)) {
|
||||
logStage(
|
||||
"ABANDONED " + shortNuri(ctx.inbox) + " — the identity changed while it was being " +
|
||||
"processed; its remaining deposits stay for their owner",
|
||||
);
|
||||
return applied;
|
||||
}
|
||||
if (!claimsDeposit(deposit.payload, ctx)) continue;
|
||||
// No subject, no pop. `?d` is bound by the deposits query, so this should be
|
||||
// unreachable — but `depositsFrom` defaults it to "" rather than dropping the row, and
|
||||
// an empty subject would issue `DELETE WHERE { <> ?p ?o }`: a malformed statement whose
|
||||
// effect on a real broker nobody has established. Refusing to APPLY as well is the
|
||||
// load-bearing half — applying without retiring is precisely the replay this module
|
||||
// exists to end, and it would repeat at every drain for as long as the row sat there.
|
||||
if (deposit.subject === "") {
|
||||
console.error(
|
||||
accessLogPrefix() + " a deposit in " + shortNuri(ctx.inbox) + " carries no subject, " +
|
||||
"so it cannot be retired — refusing to apply it rather than applying it again at " +
|
||||
"every drain. Nothing was consumed and nothing was written.",
|
||||
);
|
||||
continue;
|
||||
}
|
||||
await removeDeposit(deposit, ctx);
|
||||
// Popping is an AWAIT, and the arm below reads AMBIENT state — so the identity can move
|
||||
// across exactly this gap, between the decision to deliver and the delivery itself.
|
||||
// Re-asked rather than assumed: applying now would file this deposit for whoever holds
|
||||
// the page instead of for its owner, which is the mis-filing the whole `Applying`
|
||||
// context exists to prevent.
|
||||
//
|
||||
// BOTH guards, and that matters: `processInbox` names its owner as a constant
|
||||
// (`ownerHoldsTheSession: () => true`) because it only ever runs in that owner's own
|
||||
// session, so the claim check alone cannot see the move — only `stillTheHolder` can.
|
||||
// Checking one and not the other left this hole open, which is how it was missed.
|
||||
//
|
||||
// Reported ungated and then ABANDONED: the deposit is gone and cannot be got back, and
|
||||
// whatever is behind it in the queue belongs to a session that is no longer here.
|
||||
if (!holderStillStands(ctx) || !claims(deposit.payload, ctx)) {
|
||||
console.error(
|
||||
accessLogPrefix() + " a deposit in " + shortNuri(ctx.inbox) + " was consumed but not " +
|
||||
"applied: the identity changed while it was being taken out of the queue, and " +
|
||||
"applying it now would file it for the wrong holder. It cannot be recovered; what " +
|
||||
"is still queued stays for its owner.",
|
||||
);
|
||||
return applied;
|
||||
}
|
||||
// The arm's own I/O failure is REPORTED HERE, at the source, and only then allowed to
|
||||
// travel. It must stop the drain — everything not yet popped has to survive — but a
|
||||
// throw is at the mercy of whoever catches it, and one caller legitimately swallows:
|
||||
// `connect.connectedUser` drops the error when the identity has moved, because an
|
||||
// ownership refusal there is abandoning and not a failure. That guard cannot tell the
|
||||
// two apart, so an arm that failed WHILE the page switched user destroyed a deposit and
|
||||
// printed nothing at all. Reporting before rethrowing closes that without touching a
|
||||
// policy that is right for its own case: the loss is announced by the only code that
|
||||
// knows a deposit was consumed.
|
||||
try {
|
||||
if (await applyDeposit(deposit.payload, ctx)) applied += 1;
|
||||
} catch (error) {
|
||||
console.error(
|
||||
accessLogPrefix() + " a deposit in " + shortNuri(ctx.inbox) + " was consumed and " +
|
||||
"could not be applied — it cannot be recovered; what is still queued stays for " +
|
||||
"the next attempt:",
|
||||
error,
|
||||
);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
return applied;
|
||||
}
|
||||
|
||||
/** Is the identity this drain was authorised for still the one holding the page? A drain
|
||||
* that NAMED its holder rather than resolving it has nothing to lose and always stands. */
|
||||
function holderStillStands(ctx: Applying): boolean {
|
||||
return ctx.stillTheHolder === undefined || ctx.stillTheHolder();
|
||||
}
|
||||
Reference in New Issue
Block a user