diff --git a/sidecar/src/main.rs b/sidecar/src/main.rs index bfaaaa4..7c7ce7c 100644 --- a/sidecar/src/main.rs +++ b/sidecar/src/main.rs @@ -72,7 +72,26 @@ use tracing_subscriber::EnvFilter; /// index only the columns they already had, so the new fields ride inside the stored JSON and the /// new kind lands in `events` like any other. That is the dumb-forwarder property doing its job: /// the sidecar defines no schema for a frame's contents and so needs no change when they grow. -pub const PROTOCOL_VERSION: u32 = 5; +/// +/// v6 (Protocol 6): the first bump that is about a GUARANTEE rather than about data, and the first +/// the sidecar mostly gets for free. Two things: +/// +/// * **`idempotencyKey` on inbound commands.** A command that carries one is executed by the shard +/// at most once; a repeat is answered with the original reply rather than re-run. That is what +/// makes a world-writing verb retryable at all — until now a lost acknowledgement was +/// indistinguishable from a command that never applied, so the website had to declare every +/// write un-retryable and accept losing one rather than risk doubling it. The sidecar's part is +/// to CARRY the key (it rides in the command body, which every write endpoint already passes +/// through verbatim) and to understand the one new answer the shard can now give: `bridge.busy`, +/// meaning a command under that key is still in flight. See `web::respond`. +/// * **`champ.boss.killed` is a new kind**: a champion's defeat, with the damage table only the +/// shard ever sees. It was previously inferable from `champ.update` going `bossUp` true then +/// false alongside a nearby `mob.killed`, which is fragile and says nothing about who did the +/// work. It lands in `events` and on the feed like any other kind, with no code here at all — +/// the dumb-forwarder property again. +/// +/// **No store migration.** Nothing gains a column; the new kind is persisted whole like every other. +pub const PROTOCOL_VERSION: u32 = 6; // Not `#[tokio::main]`: on Windows the SCM dispatcher takes over this thread and starts the runtime // itself, on its own thread, once the service actually begins. The runtime is built by whichever diff --git a/sidecar/src/web.rs b/sidecar/src/web.rs index f878148..9909f1a 100644 --- a/sidecar/src/web.rs +++ b/sidecar/src/web.rs @@ -246,13 +246,31 @@ fn constant_time_eq(a: &[u8], b: &[u8]) -> bool { // ---- shared reply handling ---- +/// Protocol 6. `bridge.busy` says a command carrying this `idempotencyKey` is already in flight on +/// the shard: nothing was run, and the caller should come back. +/// +/// It maps to **425 Too Early**, which is what that status is for — a server unwilling to risk +/// processing a request that might be a replay. The obvious alternative, 409, is already the +/// protocol-version gate's answer, and those two want opposite dispositions from a client: a version +/// mismatch is a deployment fault nobody should retry, and a busy shard is a retry that should +/// succeed on its own. Sharing a status would have made the difference readable only by inspecting +/// the body, which is exactly how a retry loop ends up hiding a mismatched deployment. +/// +/// It is checked BEFORE the `.error` suffix test in each responder below, and it is deliberately not +/// spelled `bridge.busy.error`: nothing is wrong. The work is happening. +const BUSY_KIND: &str = "bridge.busy"; +const BUSY_STATUS: StatusCode = StatusCode::TOO_EARLY; + /// Turns an RPC result into an HTTP response. A `bridge.error` reply from the shard becomes a 4xx; -/// a real reply is returned as-is; transport failures map to 503/504. +/// a `bridge.busy` reply becomes a 425; a real reply is returned as-is; transport failures map to +/// 503/504. fn respond(result: Result) -> (StatusCode, Json) { match result { Ok(value) => { let kind = value.get("kind").and_then(|k| k.as_str()).unwrap_or(""); - if kind == "bridge.error" || kind.ends_with(".error") { + if kind == BUSY_KIND { + (BUSY_STATUS, Json(value)) + } else if kind == "bridge.error" || kind.ends_with(".error") { let reason = value .get("reason") .and_then(|r| r.as_str()) @@ -286,7 +304,9 @@ fn respond_admin(result: Result) -> (StatusCode, Json) { match result { Ok(value) => { let kind = value.get("kind").and_then(|k| k.as_str()).unwrap_or(""); - if kind == "admin.error" { + if kind == BUSY_KIND { + (BUSY_STATUS, Json(value)) + } else if kind == "admin.error" { let reason = value .get("reason") .and_then(|r| r.as_str()) @@ -324,7 +344,9 @@ fn respond_account(result: Result) -> (StatusCode, Json) match result { Ok(value) => { let kind = value.get("kind").and_then(|k| k.as_str()).unwrap_or(""); - if kind == "account.error" { + if kind == BUSY_KIND { + (BUSY_STATUS, Json(value)) + } else if kind == "account.error" { let reason = value .get("reason") .and_then(|r| r.as_str()) @@ -454,6 +476,16 @@ async fn link_delete( /// Forwards a staff moderation command to the shard, correlated on a fresh reqId. Injects `kind` /// and `reqId`, requiring the caller-supplied `actor` up front (the shard enforces it too). The /// body's remaining fields (account/serial/durationSec/reason/text/hue) pass straight through. +/// +/// **Protocol 6: `idempotencyKey` is one of those remaining fields**, and passing it through is the +/// whole of the sidecar's part in the guarantee. It is worth stating rather than leaving to the +/// word "remaining", because a later refactor that narrowed this to a known field list would quietly +/// turn every retried world write back into a possible duplicate, and nothing here would fail. +/// +/// The key belongs to the CALLER's unit of work — the website's event step — so the sidecar neither +/// generates one nor validates it. Note also that `reqId` is regenerated on every call: a retry +/// carries the same idempotency key under a NEW correlation id, which is exactly why the shard +/// re-stamps a replayed reply rather than echoing the id the first attempt used. async fn admin_call(st: &AppState, kind: &str, body: Value) -> (StatusCode, Json) { let mut obj = match body { Value::Object(m) => m, @@ -978,3 +1010,67 @@ async fn ws_client(mut socket: WebSocket, state: AppState) { info!("ws client disconnected"); } + +#[cfg(test)] +mod tests { + use super::*; + + fn reply(kind: &str) -> Result { + Ok(json!({"t": 1, "kind": kind, "reqId": "r-9"})) + } + + /// Protocol 6. Every responder must recognise `bridge.busy`, because every write plane can be + /// retried: the staff plane, the account plane and the plain command plane all reach handlers + /// that a keyed retry can arrive at. A responder that missed it would return 200 with a body + /// saying nothing happened, which is the worst of the three possible answers. + #[test] + fn busy_maps_to_425_on_every_plane() { + assert_eq!(respond(reply("bridge.busy")).0, StatusCode::TOO_EARLY); + assert_eq!(respond_admin(reply("bridge.busy")).0, StatusCode::TOO_EARLY); + assert_eq!( + respond_account(reply("bridge.busy")).0, + StatusCode::TOO_EARLY + ); + } + + /// 425 must not collide with the protocol-version gate's 409: a mismatch is a deployment fault + /// nobody should retry, a busy shard is a retry that will succeed. Same-status would make the + /// two readable only by inspecting the body. + #[test] + fn busy_is_not_the_version_gates_status() { + assert_ne!(BUSY_STATUS, StatusCode::CONFLICT); + } + + /// A replayed reply is an ordinary success. The shard marks it `replayed: true` for the log, and + /// the caller must be able to treat it exactly as it would have treated the answer it lost. + #[test] + fn a_replayed_reply_is_still_a_200() { + let value = json!({"t": 1, "kind": "admin.ok", "reqId": "r-9", "replayed": true}); + let (status, body) = respond_admin(Ok(value)); + assert_eq!(status, StatusCode::OK); + assert_eq!(body.0.get("replayed").and_then(|v| v.as_bool()), Some(true)); + } + + /// The error mapping the busy arm is threaded in front of must be untouched by it. + #[test] + fn errors_still_map_as_before() { + assert_eq!( + respond(Ok( + json!({"kind": "bridge.error", "reason": "unknown account"}) + )) + .0, + StatusCode::NOT_FOUND + ); + assert_eq!( + respond_admin(Ok(json!({"kind": "admin.error", "reason": "protected"}))).0, + StatusCode::FORBIDDEN + ); + assert_eq!( + respond_account(Ok( + json!({"kind": "account.error", "reason": "already exists"}) + )) + .0, + StatusCode::CONFLICT + ); + } +}