Watch
0
0
Fork
You've already forked hyperhive
0

hive-forge: attach-issue/attach-comment fail on a missing download URL

print_url only printed when browser_download_url was Some, so a 2xx
upload response with no URL printed nothing and still exited 0 — a
caller doing url=$(hive-forge attach-issue …) got an empty string with
no signal anything went wrong.

require_url now errors, naming the attachment's id/name, when the
forge returns no download URL.
This commit is contained in:
atlas 2026-09-24 12:58:05 +02:00 • committed by mara
commit 9986da4c7f

View file

@ -51,7 +51,7 @@ pub fn run_issue(client: &Client, args: IssueArgs) -> Result<()> {
.api() .api()
.issue_create_issue_attachment(owner, name, index(args.number)?, &bytes, query) .issue_create_issue_attachment(owner, name, index(args.number)?, &bytes, query)
.send()?; .send()?;
print_url(&resp); println!("{}", require_url(&resp)?);
Ok(()) Ok(())
} }
@ -78,7 +78,7 @@ pub fn run_comment(client: &Client, args: CommentArgs) -> Result<()> {
.api() .api()
.issue_create_issue_comment_attachment(owner, name, index(args.id)?, &bytes, query) .issue_create_issue_comment_attachment(owner, name, index(args.id)?, &bytes, query)
.send()?; .send()?;
print_url(&resp); println!("{}", require_url(&resp)?);
Ok(()) Ok(())
} }
@ -93,8 +93,68 @@ fn file_name(path: &Path) -> Option<String> {
path.file_name().map(|n| n.to_string_lossy().into_owned()) path.file_name().map(|n| n.to_string_lossy().into_owned())
} }
fn print_url(v: &Attachment) { /// The uploaded attachment's browser download URL, hard-erroring if the
if let Some(url) = &v.browser_download_url { /// forge answered 2xx with none. A caller piping the printed URL (e.g.
println!("{url}"); /// `url=$(hive-forge attach-issue …)`) used to get an empty string and no
/// signal when this was `None` — the upload had happened, but nothing said
/// so, and nothing said it hadn't.
fn require_url(v: &Attachment) -> Result<url::Url> {
v.browser_download_url.clone().ok_or_else(|| {
anyhow::anyhow!(
"hive-forge attach: upload succeeded but the forge returned no download URL \
(attachment {})",
attachment_ident(v)
)
})
}
/// Best-effort identifier for an attachment error message — id if present,
/// else name, else a placeholder so the message never comes out empty.
fn attachment_ident(v: &Attachment) -> String {
match (v.id, v.name.as_deref()) {
(Some(id), Some(n)) => format!("id={id} name={n}"),
(Some(id), None) => format!("id={id}"),
(None, Some(n)) => format!("name={n}"),
(None, None) => "<unknown>".to_owned(),
}
}
#[cfg(test)]
mod tests {
use super::require_url;
use forgejo_api::structs::Attachment;
fn attachment(url: Option<&str>) -> Attachment {
Attachment {
browser_download_url: url.map(|u| u.parse().unwrap()),
created_at: None,
download_count: None,
id: Some(7),
name: Some("out.log".to_owned()),
size: None,
r#type: None,
uuid: None,
}
}
#[test]
fn control_present_url_is_returned() {
let a = attachment(Some("https://forge.example/a/1"));
assert_eq!(
require_url(&a).unwrap().as_str(),
"https://forge.example/a/1"
);
}
#[test]
fn missing_url_errors_naming_the_attachment() {
// A 2xx response with no browser_download_url used to print nothing
// and exit 0 — a caller doing `url=$(hive-forge attach-issue …)`
// got an empty string with no signal anything went wrong.
let a = attachment(None);
let err = require_url(&a).unwrap_err();
let msg = err.to_string();
assert!(msg.contains("id=7"), "missing attachment id: {msg}");
assert!(msg.contains("out.log"), "missing attachment name: {msg}");
} }
} }