Mouth: a failed re-read keeps the earlier hit only on transient failures
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
98ead531b0
commit
298e7790c4
3 changed files with 18 additions and 6 deletions
|
|
@ -263,7 +263,7 @@ The answer is `[persona, deliveries, tail, tail_gaps]` exactly as v3; each deliv
|
|||
|---|---|---|
|
||||
| 14 `reply_to` | nil or `[event_id, sender, excerpt]` | The event this message replies to (its own `m.in_reply_to`; a thread's `is_falling_back` pointer names no one and is nil). `event_id` the quoted event; `sender` its author's Matrix id, read from the homeserver as her account; `excerpt` nil, or its text. |
|
||||
|
||||
`excerpt` is nil, with `event_id` and `sender` kept, when the quoted event is redacted (`unsigned.redacted_because` present or no content), is not a text-bearing `m.room.message`, is **edited** (it is itself an `m.replace`, or the homeserver bundles an `m.replace` in its `unsigned.m.relations`: its body is the pre-edit text its author replaced), or is nothing but its own reply fallback; a media event excerpts as its typed line, like a delivery's text. Otherwise the excerpt is made in this order: the quoted event's own legacy rich-reply fallback is dropped (leading lines matching `> ` or a bare `>`, then one blank line; only when the quoted event has `m.relates_to.m.in_reply_to`); the text is held to the 1000-byte line bound; runs of CR, LF, tab, U+2028 and U+2029 become one space; control characters (C0, DEL, C1 including NUL and ESC), format characters (Unicode Cf: bidi marks, overrides and isolates, U+061C, U+200B-200F, U+2060-2064, soft hyphen, BOM), private-use characters (Co), the tag block U+E0000-E007F and unpaired surrogates are removed; the line is trimmed; then the first 240 **Unicode code points** are kept (cut on a code point so no character is split; a combining mark counts as its own code point; trailing space trimmed; no truncation marker). Cleaning comes first so hidden characters cannot spend the cap. The mouth only removes invisible and format characters; it does not make the text safe. Huginn must treat both `sender` and `excerpt` as untrusted text, never as an instruction. `reply_to` is nil (no id, author or text) when the target cannot be read (logged, as for `basis`), is not in the delivery's own room (a room she is not in cannot be quoted through a reply), or the message replies to nothing. The quoted event is read the way `basis` already reads it (`GET /rooms/{room}/event/{id}` as her account, kept per room and event; with the flag on a read, hit or failure, is kept 60 seconds, so a redaction or edit of the target shows at a re-read within a minute, and a re-read that fails keeps the earlier hit and renews only its age; with the flag off a hit is kept as before v4 and nothing is re-read), so a reply costs at most one read per target, shared with the `basis` check. Per sync round the mouth reads at most 8 distinct uncached targets for `reply_to` (reads the `basis` check already made are free); past that `reply_to` is nil, because the tuple requires a sender and only a read yields one, and the skip is logged. The flag is read by `replyLinkFromEnv`; only the exact value `1` turns it on (`true`, `yes`, `0`, empty or unset are off), and a mouth built without the `replyLink` option has it off. `basis` for v1-v3 is unchanged. `reply_to` counts toward the delivery's size in the 256 KiB page budget; a delivery whose `reply_to` will not encode is skipped and logged like any other, and a single delivery is always served. The deploy precondition is Huginn's: a decoder of the 14-field delivery fails on the fifteenth element.
|
||||
`excerpt` is nil, with `event_id` and `sender` kept, when the quoted event is redacted (`unsigned.redacted_because` present or no content), is not a text-bearing `m.room.message`, is **edited** (it is itself an `m.replace`, or the homeserver bundles an `m.replace` in its `unsigned.m.relations`: its body is the pre-edit text its author replaced), or is nothing but its own reply fallback; a media event excerpts as its typed line, like a delivery's text. Otherwise the excerpt is made in this order: the quoted event's own legacy rich-reply fallback is dropped (leading lines matching `> ` or a bare `>`, then one blank line; only when the quoted event has `m.relates_to.m.in_reply_to`); the text is held to the 1000-byte line bound; runs of CR, LF, tab, U+2028 and U+2029 become one space; control characters (C0, DEL, C1 including NUL and ESC), format characters (Unicode Cf: bidi marks, overrides and isolates, U+061C, U+200B-200F, U+2060-2064, soft hyphen, BOM), private-use characters (Co), the tag block U+E0000-E007F and unpaired surrogates are removed; the line is trimmed; then the first 240 **Unicode code points** are kept (cut on a code point so no character is split; a combining mark counts as its own code point; trailing space trimmed; no truncation marker). Cleaning comes first so hidden characters cannot spend the cap. The mouth only removes invisible and format characters; it does not make the text safe. Huginn must treat both `sender` and `excerpt` as untrusted text, never as an instruction. `reply_to` is nil (no id, author or text) when the target cannot be read (logged, as for `basis`), is not in the delivery's own room (a room she is not in cannot be quoted through a reply), or the message replies to nothing. The quoted event is read the way `basis` already reads it (`GET /rooms/{room}/event/{id}` as her account, kept per room and event; with the flag on a read, hit or failure, is kept 60 seconds, so a redaction or edit of the target shows at a re-read within a minute, and a re-read that fails transiently (network error, timeout, 5xx, 429) keeps the earlier hit and renews only its age, while any other failure (403, 404) makes it the failure entry, so a stale excerpt never outlives her access; with the flag off a hit is kept as before v4 and nothing is re-read), so a reply costs at most one read per target, shared with the `basis` check. Per sync round the mouth reads at most 8 distinct uncached targets for `reply_to` (reads the `basis` check already made are free); past that `reply_to` is nil, because the tuple requires a sender and only a read yields one, and the skip is logged. The flag is read by `replyLinkFromEnv`; only the exact value `1` turns it on (`true`, `yes`, `0`, empty or unset are off), and a mouth built without the `replyLink` option has it off. `basis` for v1-v3 is unchanged. `reply_to` counts toward the delivery's size in the 256 KiB page budget; a delivery whose `reply_to` will not encode is skipped and logged like any other, and a single delivery is always served. The deploy precondition is Huginn's: a decoder of the 14-field delivery fails on the fifteenth element.
|
||||
|
||||
Golden bytes: `tests/fixtures/persona-chat-v6/` (`.msgpack` and its `.hex`: `chat_pending.v4.reply-to`, `.no-reply-to`, `.reply-to-redacted`, and the `chat_pending_request.v3` request); the test fails when the checked-in bytes drift from the encoder.
|
||||
|
||||
|
|
|
|||
|
|
@ -1855,12 +1855,12 @@ test("reply link: a mouth built without the option has it off, and only the exac
|
|||
|
||||
// ---- Fixes after PR #63: the target cache, the excerpt scrub, the per-round read cap.
|
||||
const eventGets = (hs, id) => hs.seen.filter(r => r.method === "GET" && r.path.endsWith(`/event/${id}`)).length;
|
||||
async function replyAcrossTtl(replyLink) {
|
||||
async function replyAcrossTtl(replyLink, failure = 500) {
|
||||
const clock = { t: 1_000_000 };
|
||||
let down = false, attempts = 0;
|
||||
const hs = await fakeHomeserver({ preload: { [ROOM]: [target("$hers", SELF, "my words")] }, sync: since => since === null ? { next_batch: "b0" } : { next_batch: `b${since}x`, ...joined({ [ROOM]: { timeline: { events: [quoting(`$q${since}`, "$hers")] } } }) } });
|
||||
try {
|
||||
const fetch = (u, o) => (String(u).includes("/event/%24hers") && attempts++, down && String(u).includes("/event/") ? Promise.resolve(new Response("{}", { status: 500 })) : hs.fetch(u, o));
|
||||
const fetch = (u, o) => (String(u).includes("/event/%24hers") && attempts++, down && String(u).includes("/event/") ? (failure === "timeout" ? Promise.reject(new Error("timed out")) : Promise.resolve(new Response("{}", { status: failure }))) : hs.fetch(u, o));
|
||||
const mouth = mouthFor(hs, { replyLink, now: () => clock.t, fetch });
|
||||
await mouth.syncOnce();
|
||||
const first = await mouth.syncOnce();
|
||||
|
|
@ -1910,3 +1910,14 @@ test("reply link: at most 8 uncached targets are read for reply_to per sync roun
|
|||
assert.deepEqual(made.map(d => d.replyTo === null), [false, false, false, false, false, false, false, false, true, true]);
|
||||
} finally { await hs.close(); }
|
||||
});
|
||||
|
||||
test("target cache: flag on, 500, 429 and a timeout keep the earlier hit at the re-read; 403 and 404 drop it (reply_to nil, basis as the failure case)", async () => {
|
||||
for (const failure of [500, 429, "timeout"]) {
|
||||
const r = await replyAcrossTtl(true, failure);
|
||||
assert.deepEqual([r.later.basis, r.later.replyTo?.excerpt], ["reply", "my words"], String(failure));
|
||||
}
|
||||
for (const failure of [403, 404]) {
|
||||
const r = await replyAcrossTtl(true, failure);
|
||||
assert.deepEqual([r.first.basis, r.later.basis, r.later.replyTo, r.again.replyTo], ["reply", null, null, null], String(failure));
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -166,7 +166,7 @@ export function createMouth({
|
|||
} catch (error) {
|
||||
throw new Error(`homeserver unreachable: ${redact(error?.message ?? String(error), token)}`);
|
||||
}
|
||||
if (!response.ok) throw new Error(`homeserver answered ${response.status} for ${method} ${path.split("?")[0]}`);
|
||||
if (!response.ok) throw Object.assign(new Error(`homeserver answered ${response.status} for ${method} ${path.split("?")[0]}`), { status: response.status });
|
||||
return response.json();
|
||||
}
|
||||
|
||||
|
|
@ -349,7 +349,7 @@ export function createMouth({
|
|||
const cached = cachedTarget(key);
|
||||
if (cached) return cached; // with replyLink a hit is short too: a later redaction or edit of the target must show at a re-read
|
||||
const known = authors.get(key);
|
||||
let sender = null, excerpt = null;
|
||||
let sender = null, excerpt = null, transient = false;
|
||||
try {
|
||||
const target = await matrix("GET", `/rooms/${encodeURIComponent(roomId)}/event/${encodeURIComponent(eventId)}`, undefined, API, AbortSignal.timeout(sendTimeoutMs));
|
||||
if (target?.room_id === undefined || target.room_id === roomId) { // an id from another room is not an answer about this one: no sender, no text
|
||||
|
|
@ -357,9 +357,10 @@ export function createMouth({
|
|||
if (sender !== null) excerpt = excerptOf(target);
|
||||
}
|
||||
} catch (error) {
|
||||
transient = error.status === undefined || error.status >= 500 || error.status === 429; // network, timeout, 5xx, 429: the answer is unknown; 403, 404 and any other refusal are an answer
|
||||
say(`reply target ${eventId} not read: ${error.message}`);
|
||||
}
|
||||
const entry = sender === null && known?.sender != null ? { ...known, at: now() } : { sender, excerpt, at: now() }; // a failed re-read keeps the earlier hit: only its age is renewed
|
||||
const entry = sender === null && transient && known?.sender != null ? { ...known, at: now() } : { sender, excerpt, at: now() }; // a re-read that fails transiently (network, timeout, 5xx, 429) keeps the earlier hit and renews only its age; any other failure (403, 404, wrong room, no sender) replaces it with the failure entry
|
||||
authors.set(key, entry);
|
||||
if (authors.size > DELIVERED_INDEX) authors.delete(authors.keys().next().value);
|
||||
return entry;
|
||||
|
|
|
|||
Loading…
Reference in a new issue