mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): preserve idempotency key on ambiguous mutation failures
The resolve/reopen retry paths preserved their idempotency requestId only when the error string contained "409"/"processing". The native layer serializes lost-response failures as "relay unreachable: …" and "admin response stream error", so every ambiguous transport/read failure cleared the UUID and the retry became a brand-new command — defeating idempotency on exactly the failures it exists to contain (two-operator interleave: A reopens, response lost, B resolves, A's retry with a fresh UUID clobbers B's later action). Replace the string match with a typed AdminMutationError carrying the relay's HTTP status. The UI preserves the requestId unless the relay definitively rejected the request pre-commit (a non-409 4xx); it preserves on null status (transport/pre-send), 409, 5xx, and a lost response body, so the relay dedupes the retry. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
@@ -0,0 +1,49 @@
|
||||
//! Typed error for admin mutation commands.
|
||||
|
||||
/// Error from an admin mutation command, carrying whether the relay
|
||||
/// authoritatively answered so the UI can decide idempotency-retry policy
|
||||
/// without string-matching the message.
|
||||
///
|
||||
/// `relayStatus` is `Some(code)` only when the relay returned an HTTP status —
|
||||
/// the request reached the relay and it answered. It is `None` for a
|
||||
/// pre-response transport failure (`send()` error, DNS/connect/timeout) or a
|
||||
/// pre-send failure (auth build, body serialisation): the relay never
|
||||
/// committed anything, so a retry must reuse the same idempotency key.
|
||||
///
|
||||
/// A body-read failure mid-stream keeps the status it was reading (the relay
|
||||
/// answered with headers/status but the body was lost) — the outcome is
|
||||
/// unknown, so the caller preserves idempotency and lets the retry dedupe.
|
||||
///
|
||||
/// Serialises `rename_all = "camelCase"`; the JS bridge surfaces it as the
|
||||
/// rejected `TauriInvokeError.payload`, from which the UI reads `relayStatus`.
|
||||
#[derive(Debug, serde::Serialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct AdminMutationError {
|
||||
/// Human-readable message — byte-identical to the string the command
|
||||
/// produced before typing, so existing message parsing is unaffected.
|
||||
pub message: String,
|
||||
/// The relay's HTTP status when a response was received; `None` for a
|
||||
/// transport/pre-send failure where no relay answer exists.
|
||||
pub relay_status: Option<u16>,
|
||||
}
|
||||
|
||||
impl AdminMutationError {
|
||||
/// The relay answered with an HTTP status.
|
||||
pub(super) fn relay(status: reqwest::StatusCode, message: String) -> Self {
|
||||
Self {
|
||||
message,
|
||||
relay_status: Some(status.as_u16()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Pre-send and transport failures carry no relay status: the relay never saw
|
||||
/// the request (or never answered), so the outcome is unambiguously "no commit".
|
||||
impl From<String> for AdminMutationError {
|
||||
fn from(message: String) -> Self {
|
||||
Self {
|
||||
message,
|
||||
relay_status: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -4,7 +4,7 @@
|
||||
//! used by the Tauri command implementations in `mod.rs`.
|
||||
|
||||
use super::client;
|
||||
use super::{ATTACHMENT_CAP, ERROR_BODY_CAP};
|
||||
use super::{AdminMutationError, ATTACHMENT_CAP, ERROR_BODY_CAP};
|
||||
|
||||
/// Fetch a JSON endpoint with NIP-98 auth, one 401-retry, and a size cap.
|
||||
pub(super) async fn fetch_admin_json(
|
||||
@@ -51,7 +51,7 @@ pub(super) async fn post_admin_json(
|
||||
body: &[u8],
|
||||
cap: u64,
|
||||
state: &tauri::State<'_, crate::app_state::AppState>,
|
||||
) -> Result<Vec<u8>, String> {
|
||||
) -> Result<Vec<u8>, AdminMutationError> {
|
||||
mutation_admin_json(reqwest::Method::POST, url, body, cap, state).await
|
||||
}
|
||||
|
||||
@@ -61,7 +61,7 @@ pub(super) async fn patch_admin_json(
|
||||
body: &[u8],
|
||||
cap: u64,
|
||||
state: &tauri::State<'_, crate::app_state::AppState>,
|
||||
) -> Result<Vec<u8>, String> {
|
||||
) -> Result<Vec<u8>, AdminMutationError> {
|
||||
mutation_admin_json(reqwest::Method::PATCH, url, body, cap, state).await
|
||||
}
|
||||
|
||||
@@ -71,7 +71,7 @@ pub(super) async fn put_admin_json(
|
||||
body: &[u8],
|
||||
cap: u64,
|
||||
state: &tauri::State<'_, crate::app_state::AppState>,
|
||||
) -> Result<Vec<u8>, String> {
|
||||
) -> Result<Vec<u8>, AdminMutationError> {
|
||||
mutation_admin_json(reqwest::Method::PUT, url, body, cap, state).await
|
||||
}
|
||||
|
||||
@@ -115,13 +115,19 @@ pub(super) async fn delete_admin_json(
|
||||
}
|
||||
|
||||
/// Shared implementation for POST/PATCH/PUT with NIP-98 payload sha256 binding.
|
||||
///
|
||||
/// Returns a typed [`AdminMutationError`] so the caller can distinguish a
|
||||
/// relay-authoritative failure (a status was received) from a transport or
|
||||
/// pre-send failure (no relay answer). `?` on the `String`-producing steps
|
||||
/// (auth build, `send()` classification) converts via `From<String>` to a
|
||||
/// no-status error, which is correct: none of those reached a relay verdict.
|
||||
pub(super) async fn mutation_admin_json(
|
||||
method: reqwest::Method,
|
||||
url: &str,
|
||||
body: &[u8],
|
||||
cap: u64,
|
||||
state: &tauri::State<'_, crate::app_state::AppState>,
|
||||
) -> Result<Vec<u8>, String> {
|
||||
) -> Result<Vec<u8>, AdminMutationError> {
|
||||
use crate::relay::build_nip98_auth_header_for_keys;
|
||||
|
||||
let keys = state.signing_keys()?;
|
||||
@@ -153,10 +159,10 @@ pub(super) async fn mutation_admin_json(
|
||||
let resp2 = send_request(auth_header2)
|
||||
.await
|
||||
.map_err(|e| crate::relay::classify_request_error(&e))?;
|
||||
return read_admin_response(resp2, cap, ERROR_BODY_CAP).await;
|
||||
return read_admin_mutation_response(resp2, cap, ERROR_BODY_CAP).await;
|
||||
}
|
||||
|
||||
read_admin_response(resp, cap, ERROR_BODY_CAP).await
|
||||
read_admin_mutation_response(resp, cap, ERROR_BODY_CAP).await
|
||||
}
|
||||
|
||||
/// Stream and validate an attachment response, enforcing Content-Type, size,
|
||||
@@ -270,3 +276,68 @@ pub(super) async fn read_admin_response(
|
||||
|
||||
Ok(bytes)
|
||||
}
|
||||
|
||||
/// Read a mutation response, preserving the relay's HTTP status in the error.
|
||||
///
|
||||
/// Mirrors [`read_admin_response`]'s size discipline and message wording so the
|
||||
/// UI's message parsing is unchanged, but on a non-2xx it returns an
|
||||
/// [`AdminMutationError`] tagged with the received status. A redirect or a
|
||||
/// body-read failure also carries the status: the relay answered (headers/
|
||||
/// status arrived), the body outcome is unknown, so the caller preserves the
|
||||
/// idempotency key and lets the retry dedupe against any commit that landed.
|
||||
async fn read_admin_mutation_response(
|
||||
resp: reqwest::Response,
|
||||
success_cap: u64,
|
||||
error_cap: u64,
|
||||
) -> Result<Vec<u8>, AdminMutationError> {
|
||||
use futures_util::StreamExt;
|
||||
|
||||
let status = resp.status();
|
||||
|
||||
if status.is_redirection() {
|
||||
return Err(AdminMutationError::relay(
|
||||
status,
|
||||
format!("admin API returned a {status} redirect (not followed)"),
|
||||
));
|
||||
}
|
||||
|
||||
let (is_success, cap) = if status.is_success() {
|
||||
(true, success_cap)
|
||||
} else {
|
||||
(false, error_cap)
|
||||
};
|
||||
|
||||
if let Some(cl) = resp.content_length() {
|
||||
if cl > cap {
|
||||
return Err(AdminMutationError::relay(
|
||||
status,
|
||||
format!("admin response too large ({cl} bytes, cap {cap} bytes)"),
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
let mut bytes: Vec<u8> = Vec::new();
|
||||
let mut stream = resp.bytes_stream();
|
||||
while let Some(chunk) = stream.next().await {
|
||||
let chunk = chunk.map_err(|e| {
|
||||
AdminMutationError::relay(status, format!("admin response stream error: {e}"))
|
||||
})?;
|
||||
if bytes.len() as u64 + chunk.len() as u64 > cap {
|
||||
return Err(AdminMutationError::relay(
|
||||
status,
|
||||
format!("admin response too large (cap {cap} bytes)"),
|
||||
));
|
||||
}
|
||||
bytes.extend_from_slice(&chunk);
|
||||
}
|
||||
|
||||
if !is_success {
|
||||
let body = String::from_utf8_lossy(&bytes);
|
||||
return Err(AdminMutationError::relay(
|
||||
status,
|
||||
format!("admin API error: {body}"),
|
||||
));
|
||||
}
|
||||
|
||||
Ok(bytes)
|
||||
}
|
||||
|
||||
@@ -53,6 +53,11 @@ use helpers::{
|
||||
post_admin_json, put_admin_json,
|
||||
};
|
||||
|
||||
// ── Typed mutation error ──────────────────────────────────────────────────
|
||||
|
||||
pub(crate) mod error;
|
||||
pub use error::AdminMutationError;
|
||||
|
||||
// ── Typed probe result ────────────────────────────────────────────────────
|
||||
|
||||
/// Result of an `admin_probe` call. Each variant maps to a distinct UI state.
|
||||
@@ -523,7 +528,7 @@ pub async fn admin_resolve_report(
|
||||
id: String,
|
||||
body: serde_json::Value,
|
||||
state: tauri::State<'_, crate::app_state::AppState>,
|
||||
) -> Result<serde_json::Value, String> {
|
||||
) -> Result<serde_json::Value, AdminMutationError> {
|
||||
let origin = origin::AdminOrigin::parse(&origin)?;
|
||||
let id =
|
||||
uuid::Uuid::parse_str(&id).map_err(|_| "report id must be a valid UUID".to_string())?;
|
||||
@@ -534,7 +539,7 @@ pub async fn admin_resolve_report(
|
||||
let body_bytes =
|
||||
serde_json::to_vec(&body).map_err(|e| format!("failed to serialise request body: {e}"))?;
|
||||
let bytes = post_admin_json(&url, &body_bytes, SUCCESS_JSON_CAP, &state).await?;
|
||||
serde_json::from_slice(&bytes).map_err(|e| format!("invalid JSON from relay: {e}"))
|
||||
serde_json::from_slice(&bytes).map_err(|e| format!("invalid JSON from relay: {e}").into())
|
||||
}
|
||||
|
||||
/// Reopen a resolved report — POST /api/admin/v1/reports/{id}/reopen.
|
||||
@@ -550,7 +555,7 @@ pub async fn admin_reopen_report(
|
||||
id: String,
|
||||
body: serde_json::Value,
|
||||
state: tauri::State<'_, crate::app_state::AppState>,
|
||||
) -> Result<serde_json::Value, String> {
|
||||
) -> Result<serde_json::Value, AdminMutationError> {
|
||||
let origin = origin::AdminOrigin::parse(&origin)?;
|
||||
let id =
|
||||
uuid::Uuid::parse_str(&id).map_err(|_| "report id must be a valid UUID".to_string())?;
|
||||
@@ -561,7 +566,7 @@ pub async fn admin_reopen_report(
|
||||
let body_bytes =
|
||||
serde_json::to_vec(&body).map_err(|e| format!("failed to serialise request body: {e}"))?;
|
||||
let bytes = post_admin_json(&url, &body_bytes, SUCCESS_JSON_CAP, &state).await?;
|
||||
serde_json::from_slice(&bytes).map_err(|e| format!("invalid JSON from relay: {e}"))
|
||||
serde_json::from_slice(&bytes).map_err(|e| format!("invalid JSON from relay: {e}").into())
|
||||
}
|
||||
|
||||
/// Cancel a failed enforcement action — POST /api/admin/v1/reports/{id}/cancel.
|
||||
@@ -577,7 +582,7 @@ pub async fn admin_cancel_report(
|
||||
id: String,
|
||||
body: serde_json::Value,
|
||||
state: tauri::State<'_, crate::app_state::AppState>,
|
||||
) -> Result<serde_json::Value, String> {
|
||||
) -> Result<serde_json::Value, AdminMutationError> {
|
||||
let origin = origin::AdminOrigin::parse(&origin)?;
|
||||
let id =
|
||||
uuid::Uuid::parse_str(&id).map_err(|_| "report id must be a valid UUID".to_string())?;
|
||||
@@ -588,7 +593,7 @@ pub async fn admin_cancel_report(
|
||||
let body_bytes =
|
||||
serde_json::to_vec(&body).map_err(|e| format!("failed to serialise request body: {e}"))?;
|
||||
let bytes = post_admin_json(&url, &body_bytes, SUCCESS_JSON_CAP, &state).await?;
|
||||
serde_json::from_slice(&bytes).map_err(|e| format!("invalid JSON from relay: {e}"))
|
||||
serde_json::from_slice(&bytes).map_err(|e| format!("invalid JSON from relay: {e}").into())
|
||||
}
|
||||
|
||||
/// Update feedback status — PATCH /api/admin/v1/feedback/{id}.
|
||||
@@ -600,7 +605,7 @@ pub async fn admin_patch_feedback(
|
||||
id: String,
|
||||
body: serde_json::Value,
|
||||
state: tauri::State<'_, crate::app_state::AppState>,
|
||||
) -> Result<serde_json::Value, String> {
|
||||
) -> Result<serde_json::Value, AdminMutationError> {
|
||||
let origin = origin::AdminOrigin::parse(&origin)?;
|
||||
let id =
|
||||
uuid::Uuid::parse_str(&id).map_err(|_| "feedback id must be a valid UUID".to_string())?;
|
||||
@@ -611,7 +616,7 @@ pub async fn admin_patch_feedback(
|
||||
let body_bytes =
|
||||
serde_json::to_vec(&body).map_err(|e| format!("failed to serialise request body: {e}"))?;
|
||||
let bytes = patch_admin_json(&url, &body_bytes, SUCCESS_JSON_CAP, &state).await?;
|
||||
serde_json::from_slice(&bytes).map_err(|e| format!("invalid JSON from relay: {e}"))
|
||||
serde_json::from_slice(&bytes).map_err(|e| format!("invalid JSON from relay: {e}").into())
|
||||
}
|
||||
|
||||
/// List operators — GET /api/admin/v1/operators.
|
||||
@@ -642,7 +647,7 @@ pub async fn admin_put_operator(
|
||||
pubkey: String,
|
||||
body: serde_json::Value,
|
||||
state: tauri::State<'_, crate::app_state::AppState>,
|
||||
) -> Result<serde_json::Value, String> {
|
||||
) -> Result<serde_json::Value, AdminMutationError> {
|
||||
let origin = origin::AdminOrigin::parse(&origin)?;
|
||||
let pubkey =
|
||||
routes::HexPubkey::parse(&pubkey).map_err(|e| format!("invalid operator pubkey: {e}"))?;
|
||||
@@ -653,7 +658,7 @@ pub async fn admin_put_operator(
|
||||
let body_bytes =
|
||||
serde_json::to_vec(&body).map_err(|e| format!("failed to serialise request body: {e}"))?;
|
||||
let bytes = put_admin_json(&url, &body_bytes, SUCCESS_JSON_CAP, &state).await?;
|
||||
serde_json::from_slice(&bytes).map_err(|e| format!("invalid JSON from relay: {e}"))
|
||||
serde_json::from_slice(&bytes).map_err(|e| format!("invalid JSON from relay: {e}").into())
|
||||
}
|
||||
|
||||
/// Remove an operator — DELETE /api/admin/v1/operators/{pubkey}.
|
||||
|
||||
@@ -55,6 +55,7 @@ import {
|
||||
formatTimestamp,
|
||||
useAsyncLoad,
|
||||
adminErrorMessage,
|
||||
preserveRequestIdOnError,
|
||||
} from "./AdminConsolePanelHelpers";
|
||||
import { FeedbackTab } from "./AdminConsoleFeedbackTab";
|
||||
import { StaffingTab } from "./AdminConsoleStaffingTab";
|
||||
@@ -313,11 +314,12 @@ function ResolveReportForm({
|
||||
toast.success(`Report resolved: ${actionLabel(selectedAction)}`);
|
||||
onResolved();
|
||||
} catch (e) {
|
||||
// On error, reset requestId so the next submit generates a new one.
|
||||
// But: if the error suggests a 409 (report already processing), the
|
||||
// server has a claim — don't reset, let the parent handle it.
|
||||
const msg = e instanceof Error ? e.message : String(e);
|
||||
if (!msg.includes("409") && !msg.includes("processing")) {
|
||||
// Preserve the requestId whenever the outcome is ambiguous (409,
|
||||
// 5xx, a lost response, or a transport failure with no relay answer)
|
||||
// so a retry reuses the same idempotency key and the relay dedupes.
|
||||
// Reset only on a definitive pre-commit rejection (a non-409 4xx), where
|
||||
// a corrected resubmission is a genuinely new command.
|
||||
if (!preserveRequestIdOnError(e)) {
|
||||
requestIdRef.current = null;
|
||||
}
|
||||
toast.error(adminErrorMessage(e));
|
||||
@@ -455,11 +457,11 @@ function ReopenReportForm({
|
||||
toast.success("Report reopened");
|
||||
onReopened();
|
||||
} catch (e) {
|
||||
const msg = e instanceof Error ? e.message : String(e);
|
||||
// A 409 means the report is not reopenable (e.g. it moved to
|
||||
// `processing`). Preserve the requestId so a genuine retry reuses it;
|
||||
// reset otherwise so the next attempt generates a fresh one.
|
||||
if (!msg.includes("409") && !msg.includes("processing")) {
|
||||
// Preserve the requestId on an ambiguous outcome (409, 5xx, lost
|
||||
// response, or a transport failure with no relay answer) so a retry
|
||||
// reuses the same idempotency key; reset only on a definitive pre-commit
|
||||
// rejection (a non-409 4xx).
|
||||
if (!preserveRequestIdOnError(e)) {
|
||||
requestIdRef.current = null;
|
||||
}
|
||||
toast.error(adminErrorMessage(e));
|
||||
|
||||
@@ -88,6 +88,53 @@ export function adminErrorMessage(e: unknown): string {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the relay's HTTP status from a rejected admin mutation, or `null`.
|
||||
*
|
||||
* Native mutation commands reject with a typed `AdminMutationError`
|
||||
* (`{message, relayStatus}`); the Tauri bridge surfaces it as
|
||||
* `TauriInvokeError` whose `payload` is that object. `relayStatus` is a number
|
||||
* only when the relay actually answered — `null`/absent for a transport or
|
||||
* pre-send failure where no relay verdict exists.
|
||||
*/
|
||||
export function adminMutationRelayStatus(e: unknown): number | null {
|
||||
if (e && typeof e === "object" && "payload" in e) {
|
||||
const payload = (e as { payload: unknown }).payload;
|
||||
if (payload && typeof payload === "object" && "relayStatus" in payload) {
|
||||
const status = (payload as { relayStatus: unknown }).relayStatus;
|
||||
if (typeof status === "number") return status;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a failed mutation must reuse its idempotency `requestId` on retry.
|
||||
*
|
||||
* The id is preserved UNLESS the relay definitively rejected the request before
|
||||
* committing — a 4xx other than 409. Those (bad action, unauthorized, not
|
||||
* found) refuse the input pre-commit, so a corrected resubmission is a
|
||||
* genuinely new command and a fresh id is safe.
|
||||
*
|
||||
* Everything else preserves the id so the relay can dedupe against a commit
|
||||
* that may have landed:
|
||||
* - 409 — an idempotency claim or in-progress action already exists;
|
||||
* - 5xx — the relay may have committed before failing;
|
||||
* - a lost response body (status arrived, outcome unknown);
|
||||
* - a transport or pre-send failure with no relay answer (`relayStatus` null).
|
||||
*
|
||||
* This replaces string-matching `"409"`/`"processing"` on the message — which
|
||||
* missed the native layer's transport errors (`relay unreachable: …`, `admin
|
||||
* response stream error`) and cleared the id on exactly the ambiguous
|
||||
* lost-response failures where reuse is required.
|
||||
*/
|
||||
export function preserveRequestIdOnError(e: unknown): boolean {
|
||||
const status = adminMutationRelayStatus(e);
|
||||
if (status === null) return true;
|
||||
if (status === 409) return true;
|
||||
return status < 400 || status >= 500;
|
||||
}
|
||||
|
||||
// ── Shared UI helpers ─────────────────────────────────────────────────────
|
||||
|
||||
export function LoadingSpinner() {
|
||||
|
||||
@@ -100,6 +100,21 @@ toast.error = (msg) => {
|
||||
return 0;
|
||||
};
|
||||
|
||||
// ── Typed native mutation error ──────────────────────────────────────────────
|
||||
//
|
||||
// Admin mutation commands reject with a serialized Rust `AdminMutationError`
|
||||
// (`{message, relayStatus}`, camelCase). The real tauri bridge rejects with
|
||||
// that plain object and `toTauriError` wraps it into a `TauriInvokeError` whose
|
||||
// `.message` is the message and `.payload` is the whole object — from which the
|
||||
// UI reads `relayStatus` to decide idempotency-retry policy. Rejecting with a
|
||||
// plain object here (NOT an Error) reproduces that wire shape exactly.
|
||||
//
|
||||
// `relayStatus` is a number when the relay authoritatively answered, and `null`
|
||||
// for a transport/pre-send failure where no relay verdict exists.
|
||||
function mutationReject(message, relayStatus) {
|
||||
return Promise.reject({ message, relayStatus });
|
||||
}
|
||||
|
||||
// ── Deferred promise helper ──────────────────────────────────────────────────
|
||||
|
||||
function deferred() {
|
||||
@@ -2303,11 +2318,13 @@ test("reopen-enforced-copy: a report with an actionId warns enforcement is not r
|
||||
|
||||
test("reopen-409-preserves-requestId: a not-reopenable conflict reuses the same requestId on retry", async () => {
|
||||
// A 409 (report is not reopenable — e.g. it moved to processing) is an
|
||||
// idempotency-relevant failure: the same requestId must be reused on retry
|
||||
// so the relay can dedupe. A non-409 failure resets it.
|
||||
// idempotency-relevant failure: the relay has a claim, so the same requestId
|
||||
// must be reused on retry to let the relay dedupe. The native command carries
|
||||
// the relay's HTTP status on the rejected error (`relayStatus: 409`), and the
|
||||
// UI's preserveRequestIdOnError reads it — no string-matching.
|
||||
//
|
||||
// Mutation evidence: drop the 409 branch in the catch → requestId resets and
|
||||
// the two attempts carry different ids, going red.
|
||||
// Mutation evidence: make preserveRequestIdOnError reset on 409 → the two
|
||||
// attempts carry different ids and this goes red.
|
||||
|
||||
const origin = "https://admin.example.com";
|
||||
const pubkey = "c4".repeat(32);
|
||||
@@ -2341,8 +2358,9 @@ test("reopen-409-preserves-requestId: a not-reopenable conflict reuses the same
|
||||
const requestIds = [];
|
||||
setIpcHandler("admin_reopen_report", (args) => {
|
||||
requestIds.push(args?.body?.requestId);
|
||||
return Promise.reject(
|
||||
new Error("409 report is not reopenable (current status: processing)"),
|
||||
return mutationReject(
|
||||
"admin API error: 409 report is not reopenable (current status: processing)",
|
||||
409,
|
||||
);
|
||||
});
|
||||
|
||||
@@ -2387,6 +2405,159 @@ test("reopen-409-preserves-requestId: a not-reopenable conflict reuses the same
|
||||
await unmount();
|
||||
});
|
||||
|
||||
test("reopen-lost-response-preserves-requestId: an ambiguous transport failure reuses the requestId on retry", async () => {
|
||||
// The bug this fixes: the native layer serializes a timeout/disconnect as
|
||||
// `relay unreachable: …` and a lost response body as `admin response stream
|
||||
// error` — neither contains "409"/"processing", so the old string-match
|
||||
// cleared the requestId and the retry became a brand-new command. The
|
||||
// concrete harm is a two-operator interleave: A's reopen COMMITS, the
|
||||
// response is lost; B resolves the now-open report; A's retry with a fresh id
|
||||
// reopens B's later resolution. Reusing the original id makes the retry hit
|
||||
// the relay's idempotent path harmlessly.
|
||||
//
|
||||
// A lost-response failure carries no relay verdict (`relayStatus: null`), so
|
||||
// preserveRequestIdOnError must keep the id. Mutation evidence: change the
|
||||
// null-status branch to reset → the two attempts carry different ids, red.
|
||||
|
||||
const origin = "https://admin.example.com";
|
||||
const pubkey = "c5".repeat(32);
|
||||
|
||||
const resolvedItem = {
|
||||
id: "00000000-0000-0000-0000-0000000000c5",
|
||||
communityId: "comm-1",
|
||||
communityHost: "alpha.example.com",
|
||||
reportEventId: "aa",
|
||||
reporterPubkey: "bb",
|
||||
targetKind: "event",
|
||||
target: "cc",
|
||||
reportType: "spam",
|
||||
status: "resolved",
|
||||
createdAt: "2024-06-01T12:00:00Z",
|
||||
};
|
||||
const resolvedDetail = {
|
||||
...resolvedItem,
|
||||
channelId: null,
|
||||
note: null,
|
||||
resolvedBy: "mod_pubkey",
|
||||
resolvedAt: "2024-06-02T08:00:00Z",
|
||||
actionId: null,
|
||||
message: null,
|
||||
};
|
||||
|
||||
setIpcHandler("admin_list_reports", () => Promise.resolve([resolvedItem]));
|
||||
setIpcHandler("admin_get_report", () => Promise.resolve(resolvedDetail));
|
||||
setIpcHandler("admin_list_feedback", () => Promise.resolve([]));
|
||||
|
||||
const requestIds = [];
|
||||
setIpcHandler("admin_reopen_report", (args) => {
|
||||
requestIds.push(args?.body?.requestId);
|
||||
// Transport failure: no relay answer, so no HTTP status.
|
||||
return mutationReject("relay unreachable: network error", null);
|
||||
});
|
||||
|
||||
const { container, doRender, unmount } = mountPanel({ origin, pubkey });
|
||||
await doRender();
|
||||
await settle(30);
|
||||
await openFirstReportDetail(container);
|
||||
await settle(20);
|
||||
|
||||
const submit = container.querySelector("[data-testid='reopen-submit-btn']");
|
||||
assert.ok(submit, "reopen submit button must be present");
|
||||
|
||||
// First attempt → lost response.
|
||||
await act(async () => {
|
||||
fireEvent.click(submit);
|
||||
await new Promise((r) => setTimeout(r, 20));
|
||||
});
|
||||
// Second attempt → same ambiguous failure; requestId must be identical.
|
||||
await act(async () => {
|
||||
fireEvent.click(submit);
|
||||
await new Promise((r) => setTimeout(r, 20));
|
||||
});
|
||||
|
||||
assert.equal(requestIds.length, 2, "two reopen attempts must have been made");
|
||||
assert.equal(
|
||||
requestIds[0],
|
||||
requestIds[1],
|
||||
`requestId must be preserved across a lost-response retry; got: ${JSON.stringify(requestIds)}`,
|
||||
);
|
||||
|
||||
await unmount();
|
||||
});
|
||||
|
||||
test("reopen-4xx-resets-requestId: a definitive pre-commit rejection uses a fresh requestId", async () => {
|
||||
// A non-409 4xx (e.g. 400 bad request) is a definitive pre-commit rejection:
|
||||
// the relay refused the input and committed nothing, so a corrected
|
||||
// resubmission is a genuinely new command and a fresh requestId is correct.
|
||||
// This is the ONLY case that resets — the counterpart to the ambiguous
|
||||
// failures above.
|
||||
//
|
||||
// Mutation evidence: make preserveRequestIdOnError preserve on a 400 → the
|
||||
// two attempts share an id and this goes red.
|
||||
|
||||
const origin = "https://admin.example.com";
|
||||
const pubkey = "c6".repeat(32);
|
||||
|
||||
const resolvedItem = {
|
||||
id: "00000000-0000-0000-0000-0000000000c6",
|
||||
communityId: "comm-1",
|
||||
communityHost: "alpha.example.com",
|
||||
reportEventId: "aa",
|
||||
reporterPubkey: "bb",
|
||||
targetKind: "event",
|
||||
target: "cc",
|
||||
reportType: "spam",
|
||||
status: "resolved",
|
||||
createdAt: "2024-06-01T12:00:00Z",
|
||||
};
|
||||
const resolvedDetail = {
|
||||
...resolvedItem,
|
||||
channelId: null,
|
||||
note: null,
|
||||
resolvedBy: "mod_pubkey",
|
||||
resolvedAt: "2024-06-02T08:00:00Z",
|
||||
actionId: null,
|
||||
message: null,
|
||||
};
|
||||
|
||||
setIpcHandler("admin_list_reports", () => Promise.resolve([resolvedItem]));
|
||||
setIpcHandler("admin_get_report", () => Promise.resolve(resolvedDetail));
|
||||
setIpcHandler("admin_list_feedback", () => Promise.resolve([]));
|
||||
|
||||
const requestIds = [];
|
||||
setIpcHandler("admin_reopen_report", (args) => {
|
||||
requestIds.push(args?.body?.requestId);
|
||||
return mutationReject("admin API error: bad request", 400);
|
||||
});
|
||||
|
||||
const { container, doRender, unmount } = mountPanel({ origin, pubkey });
|
||||
await doRender();
|
||||
await settle(30);
|
||||
await openFirstReportDetail(container);
|
||||
await settle(20);
|
||||
|
||||
const submit = container.querySelector("[data-testid='reopen-submit-btn']");
|
||||
assert.ok(submit, "reopen submit button must be present");
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(submit);
|
||||
await new Promise((r) => setTimeout(r, 20));
|
||||
});
|
||||
await act(async () => {
|
||||
fireEvent.click(submit);
|
||||
await new Promise((r) => setTimeout(r, 20));
|
||||
});
|
||||
|
||||
assert.equal(requestIds.length, 2, "two reopen attempts must have been made");
|
||||
assert.notEqual(
|
||||
requestIds[0],
|
||||
requestIds[1],
|
||||
`a non-409 4xx must reset the requestId; got: ${JSON.stringify(requestIds)}`,
|
||||
);
|
||||
|
||||
await unmount();
|
||||
});
|
||||
|
||||
test("cancel-on-failed: a failed action offers Cancel, POSTs {actionId} to admin_cancel_report, and reloads to open", async () => {
|
||||
// Cancel-then-resolve is the only recovery from a failed enforcement. The
|
||||
// block offers Cancel on `status: "failed"`, fences it on the action id, and
|
||||
@@ -3136,16 +3307,15 @@ test("resolve-rejection-surfaces-parsed-message: a rejected resolve toasts the r
|
||||
|
||||
const humanMessage =
|
||||
"action kick requires the report to have an associated channel";
|
||||
// The native command rejects with `admin API error: {envelope}` — exactly the
|
||||
// shape adminErrorMessage strips down to the envelope's `message`.
|
||||
// The native command rejects with a typed AdminMutationError: message is
|
||||
// `admin API error: {envelope}` (the shape adminErrorMessage strips to the
|
||||
// envelope's `message`) and relayStatus is the relay's 400.
|
||||
const rawError = `admin API error: {"error":{"code":"invalid_action_for_target","message":"${humanMessage}","requestId":"00000000-0000-0000-0000-0000000000e7"}}`;
|
||||
|
||||
setIpcHandler("admin_list_reports", () => Promise.resolve([openItem]));
|
||||
setIpcHandler("admin_get_report", () => Promise.resolve(openDetail));
|
||||
setIpcHandler("admin_list_feedback", () => Promise.resolve([]));
|
||||
setIpcHandler("admin_resolve_report", () =>
|
||||
Promise.reject(new Error(rawError)),
|
||||
);
|
||||
setIpcHandler("admin_resolve_report", () => mutationReject(rawError, 400));
|
||||
|
||||
const { container, doRender, unmount } = mountPanel({ origin, pubkey });
|
||||
await doRender();
|
||||
|
||||
Reference in New Issue
Block a user