hive-forge: dependency remove accepts the forge's 201 Created
The forge always answers 201 Created for a successful dependency removal, but the generated forgejo-api client only treats 200 as success for that endpoint, so every successful removal surfaced as ForgejoError::UnexpectedStatusCode(201). apply_and_verify caught this, read the dependency list back, saw the edge was gone, and printed a "converged despite a reported error" warning — appropriate for a genuinely unpredictable anomaly, but this mismatch is deterministic and not a real problem. Treat that specific status as success outright on the remove path instead of routing it through the read-back/warning path. The add path's different, genuinely intermittent 500-despite-success quirk is untouched, and a real remove failure still goes through the read-back and still surfaces if it does not converge. Closes #4531
This commit is contained in:
parent
97cde357e5
commit
a704a844d1
1 changed files with 26 additions and 9 deletions
|
|
@ -13,6 +13,7 @@ use std::collections::HashSet;
|
|||
|
||||
use anyhow::{Result, bail};
|
||||
use clap::{Args as ClapArgs, Subcommand};
|
||||
use forgejo_api::ForgejoError;
|
||||
use forgejo_api::structs::IssueMeta;
|
||||
use serde_json::Value;
|
||||
|
||||
|
|
@ -70,14 +71,18 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
|
|||
/// verify by reading the dependency list back rather than trusting the
|
||||
/// HTTP status this verb family returns.
|
||||
///
|
||||
/// Measured: the status code lies in both directions on this endpoint
|
||||
/// pair. `create` can 500 on a call that actually wrote the
|
||||
/// edge (an intermittent server-side double-processing, not a client
|
||||
/// retry — ruled out separately). `remove` can answer `201 Created` on a
|
||||
/// call that actually deleted the edge, which the typed client's
|
||||
/// endpoint spec maps to an error since 201 isn't the expected status for
|
||||
/// a deletion. So a `send()` error here only means "maybe" — the
|
||||
/// authoritative signal is whether the edge is actually there afterward.
|
||||
/// Measured: `create` can 500 on a call that actually wrote the edge (an
|
||||
/// intermittent server-side double-processing, not a client retry —
|
||||
/// ruled out separately). A `send()` error there only means "maybe" —
|
||||
/// the authoritative signal is whether the edge is actually there
|
||||
/// afterward.
|
||||
///
|
||||
/// `remove`'s known quirk is handled below, before it ever reaches this
|
||||
/// path: the forge always answers `201 Created` for a successful
|
||||
/// deletion, which the generated client's endpoint spec doesn't expect
|
||||
/// (it only treats 200 as success for that verb), so that specific
|
||||
/// status is treated as success outright rather than routed through the
|
||||
/// read-back below.
|
||||
///
|
||||
/// Calls that errored are only re-classified as real failures if the
|
||||
/// read-back does NOT show the expected converged state (edge present
|
||||
|
|
@ -107,9 +112,21 @@ fn apply_and_verify(
|
|||
.send()
|
||||
};
|
||||
if let Err(e) = res {
|
||||
// `remove` always answers 201 for a successful deletion (not
|
||||
// the 200 the generated client expects for that verb) — a
|
||||
// deterministic, always-on mismatch, not a "something else
|
||||
// went wrong after the write" anomaly, so it's not worth a
|
||||
// read-back or a warning: just accept it.
|
||||
let is_remove_201 = !adding
|
||||
&& matches!(
|
||||
&e,
|
||||
ForgejoError::UnexpectedStatusCode(code) if *code == reqwest::StatusCode::CREATED
|
||||
);
|
||||
if !is_remove_201 {
|
||||
call_errors.push((*dep, e));
|
||||
}
|
||||
}
|
||||
}
|
||||
if call_errors.is_empty() {
|
||||
return Ok(());
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue