fix: un abonnement en échec n'empoisonne plus le document, et un dépôt ne change plus de destinataire

This commit is contained in:
Sylvain Duchesne
2026-08-17 11:47:21 +02:00
parent 520c8c59a8
commit 33212a8b00
7 changed files with 493 additions and 48 deletions
@@ -61,7 +61,7 @@ import { myInboxes } from "./branch-registers";
import { drainInboxSerially, drainsSettled } from "./inbox-drain";
import { lookupAccount } from "../shared-wallet/account-registry";
import { processInbox } from "../surface/inbox";
import { subscribeDoc, type Unsubscribe } from "../surface/subscribe";
import { subscribeDoc, subscribeDocReportingSetupFailure, type Unsubscribe } from "../surface/subscribe";
import type { Nuri, PrincipalId } from "../model/types";
/**
@@ -78,6 +78,12 @@ interface Observation {
register: Unsubscribe | null;
/** Unsubscribe from the held-caps change signal — the second "which inboxes" channel. */
holdings: Unsubscribe | null;
/**
* The inboxes whose subscription has already been re-attempted once after failing to be
* OPENED. What bounds the repair below to one extra try per inbox, so a document the broker
* genuinely cannot serve costs two calls rather than a loop.
*/
reattempted: Set<Nuri>;
/** True while a re-enumeration is running, so its own effects do not restart it. */
enumerating: boolean;
/** A trigger that arrived mid-cycle: the running one repeats once rather than lose it. */
@@ -128,21 +134,30 @@ async function applyWhatArrived(obs: Observation, inbox: Nuri): Promise<void> {
if (!current(obs)) return;
await drainInboxSerially(inbox, async () => {
// Re-checked INSIDE the queue: this run may have waited behind another one, and the
// identity can have moved while it waited. `processInbox` resolves the holder at each
// of its steps, so running it for the wrong one is not a near-miss — it reads someone
// else's registers and files into someone else's ring.
// identity can have moved while it waited. Running it for the wrong holder is not a
// near-miss — it reads someone else's registers and files into someone else's ring.
//
// This check is NECESSARY and it is not SUFFICIENT, and until 2026-08-17 this comment
// claimed it was. It cannot be: the identity can move after it passes, while
// `processInbox` is mid-read. The claim was that `processInbox` "resolves the holder at
// each step, so the new holder is refused an inbox that is not theirs somewhere in the
// middle" — but its ownership guard runs ONCE, at entry, and a switch after that reached
// the filing with nobody left to refuse it. What it filed was the previous holder's cap,
// into the new holder's ring, durably. The guard that makes this run safe is therefore
// the one INSIDE `processInbox`, which captures the holder its guard authorised; this one
// only spares the work when the switch is already visible.
if (!current(obs)) return;
try {
await processInbox(inbox);
} catch (error) {
// An identity that moved MID-DRAIN throws here too, and it is the ordinary case rather
// than an exotic one: `processInbox` resolves the holder at each step, so the new
// holder is refused an inbox that is not theirs somewhere in the middle. That is
// ABANDONING — which this package has always called "not a failure" — and reporting it
// would put a broker-looking error in the log every time a page switches user.
// Nothing is lost: an inbox is not consumed by being abandoned, so the deposit is
// still there for its owner's next connection. Same rule, same words, as the drain
// loop in `connect.connectedUser`.
// An identity that moved MID-DRAIN can throw here too — the switch may land before
// `processInbox`'s own ownership guard, which then refuses the new holder an inbox that
// is not theirs. (Landing after it, the run ABANDONS quietly instead and returns; both
// leave the deposit where it is.) Abandoning is what this package has always called
// "not a failure", and reporting it would put a broker-looking error in the log every
// time a page switches user. Nothing is lost: an inbox is not consumed by being
// abandoned, so the deposit is still there for its owner's next connection. Same rule,
// same words, as the drain loop in `connect.connectedUser`.
if (!current(obs)) return;
throw error;
}
@@ -175,9 +190,19 @@ async function watchTheInboxes(obs: Observation): Promise<void> {
// `State`, which drains an inbox connection has usually just drained — idempotent,
// and the alternative (skip the first) would lose a deposit that landed in the gap
// between the two.
//
// The failure channel is not decoration: `subscribeDoc` returns SYNCHRONOUSLY and its
// one real `doc_subscribe` is opened afterwards, so a rejection there reaches nobody.
// Without it, this map held an entry for an inbox that was never watched, the loop
// above skipped it at every later enumeration, and the only observable difference from
// a healthy session was that shares stopped arriving.
obs.inboxes.set(
inbox,
subscribeDoc(inbox, () => track(obs, applyWhatArrived(obs, inbox))),
subscribeDocReportingSetupFailure(
inbox,
() => track(obs, applyWhatArrived(obs, inbox)),
(error) => watchFailed(obs, inbox, error),
),
);
logStage("OBSERVING " + shortNuri(inbox) + " for " + obs.holder);
} catch (error) {
@@ -186,6 +211,39 @@ async function watchTheInboxes(obs: Observation): Promise<void> {
}
}
/**
* The subscription on `inbox` could not be OPENED — report it, forget it, and try once more.
*
* **Forget it**, because the entry this observation holds is the whole record of "already
* watched": leaving a dead one in place is how one rejection at connection turned into a
* session-long silence. Removed, the next enumeration subscribes again as if it had never
* been attempted.
*
* **Try once more**, because the usual cause is a repo the verifier has not synced yet, and
* the whole cost of finding out is one call. Exactly one extra attempt per inbox, tracked in
* {@link Observation.reattempted}: a broker that genuinely cannot serve this document must
* not be asked again and again by a loop that has no reason to stop. If the second attempt
* fails too, the failure has been reported twice and the inbox waits for the next
* enumeration signal — or for the next connection, where an unapplied deposit has always
* waited.
*/
function watchFailed(obs: Observation, inbox: Nuri, error: unknown): void {
if (!current(obs)) return;
const stale = obs.inboxes.get(inbox);
obs.inboxes.delete(inbox);
if (stale) {
try {
stale();
} catch (thrown) {
console.error(accessLogPrefix() + " releasing a failed inbox observation failed:", thrown);
}
}
reportUnobserved("this inbox could not be watched: " + shortNuri(inbox), error);
if (obs.reattempted.has(inbox)) return;
obs.reattempted.add(inbox);
track(obs, enumerate(obs));
}
/**
* Follow the register that says which inboxes exist, so one opened MID-SESSION is picked
* up. See the module header: `openDocumentInbox` appends its record to the User branch of
@@ -279,6 +337,7 @@ export async function startObservingInboxes(): Promise<void> {
inboxes: new Map(),
register: null,
holdings: null,
reattempted: new Set(),
enumerating: false,
enumerateAgain: false,
pending: new Set(),