Network policy: throttle on terminal 429 and make the sync loop / send queue defer to §5 after retry exhaustion (CR-4) #134

New issue

Closed

opened 2026-08-27 19:21:08 +02:00 by thecrealm · 0 comments

thecrealm commented

2026-08-27 19:21:08 +02:00

Owner

Review finding 2026-08-27 (A1). NetworkPolicy.kt:113-117 throws RateLimited(serverDelay) after maxAttempts without calling throttle(); SyncLoop.kt:109-113 then sleeps a fixed backoffOnError (5 s) and SendQueue.kt:54 a fixed retryDelay (3 s), ignoring the surfaced retryAfter. A Retry-After longer than the fixed delay is violated on the next request. The CR-4 storm test drives policy.execute directly, and FleetFairnessGateTest/EncryptedPuppetingGateTest assert politenessViolations.isEmpty() without ever enabling strictRateLimit (vacuous).

Acceptance (§5, §10.3.4, §10.5.2, CR-4, NFR-6):

terminal 429 records the throttle before throwing; the sync loop and send-queue requeue honour RateLimited.retryAfter (server delay always beats the fixed/computed delay) and otherwise use the §5 per-class backoff, not private timers;
SessionCrypto.kt:201-215 own attempt counter reviewed against the same rule;
a strict-429 scenario driven through a full KatrixSession (sync + send + media + /keys/query//keys/claim) and through an AppserviceFleet with strictRateLimit enabled, asserting zero politeness violations;
cr4_serverDelayBindsSyncLoop exercises the real SyncLoop.

Review finding 2026-08-27 (A1). `NetworkPolicy.kt:113-117` throws `RateLimited(serverDelay)` after `maxAttempts` **without** calling `throttle()`; `SyncLoop.kt:109-113` then sleeps a fixed `backoffOnError` (5 s) and `SendQueue.kt:54` a fixed `retryDelay` (3 s), ignoring the surfaced `retryAfter`. A `Retry-After` longer than the fixed delay is violated on the next request. The CR-4 storm test drives `policy.execute` directly, and `FleetFairnessGateTest`/`EncryptedPuppetingGateTest` assert `politenessViolations.isEmpty()` without ever enabling `strictRateLimit` (vacuous). Acceptance (§5, §10.3.4, §10.5.2, CR-4, NFR-6): - terminal 429 records the throttle before throwing; the sync loop and send-queue requeue honour `RateLimited.retryAfter` (server delay always beats the fixed/computed delay) and otherwise use the §5 per-class backoff, not private timers; - `SessionCrypto.kt:201-215` own attempt counter reviewed against the same rule; - a strict-429 scenario driven through a full `KatrixSession` (sync + send + media + `/keys/query`/`/keys/claim`) and through an `AppserviceFleet` with `strictRateLimit` enabled, asserting zero politeness violations; - `cr4_serverDelayBindsSyncLoop` exercises the real `SyncLoop`.

thecrealm added the

now

labels

2026-08-27 19:21:53 +02:00

thecrealm referenced this issue from a commit

2026-08-27 20:02:33 +02:00

Network policy: throttle on terminal 429; sync loop and send queue defer to §5 after retry exhaustion (D81, closes #134)

thecrealm closed this issue

2026-08-27 20:02:33 +02:00

thecrealm referenced this issue

2026-08-27 20:02:44 +02:00

Sync loop failure typing: Stopped(typed cause) for unrecoverable protocol errors, malformed-batch retry through §5 with a §7 event (§10.3.4) #141