oknozor / musicbrainz_rs

A wrapper around the musicbrainz API
MIT License
38 stars 18 forks source link

Covert art response is panicking #77

Open CrendKing opened 1 year ago

CrendKing commented 1 year ago

Code here just unwraps the reqwest response, despite that function returns Result. It's annoying because it seems downloading cover art frequently results in HTTP 429 (I don't know why but that's separate issue).

Could you please just propagate that result so the calling app can handle it appropriately?

AlePon123 commented 1 year ago

i had same issue and found that is a problem with api specifically with CoverArtImage struct id field. In api it has u64 type, but in json it has string type. so i do this

#[derive(Debug, Serialize, Deserialize)]
struct Thumbnail {
    small: Option<String>,
    large: Option<String>,
    res_1200: Option<String>,
    res_500: Option<String>,
    res_250: Option<String>,
}

#[derive(Debug, Serialize, Deserialize)]
enum ImageType {
    Front,
    Back,
    Booklet,
    Medium,
    Tray,
    Obi,
    Spine,
    Track,
    Liner,
    Sticker,
    Poster,
    Watermark,
    Raw,
    Other,
}

#[derive(Debug, Serialize, Deserialize)]
struct CoverArt {
    approved: bool,
    back: bool,
    comment: String,
    edit: u64,
    front: bool,
    id: String, // Change this
    image: String,
    thumbnails: Thumbnail,
    types: Vec<ImageType>,
}

#[derive(Debug, Serialize, Deserialize)]
struct CoverartJson {
    images: Vec<CoverArt>,
}

fn foo() {
 let art =
            reqwest::get("http://coverartarchive.org/release/".to_string() + release.id.as_str())
                .await
                .unwrap()
                .json::<CoverartJson>()
                .await
                .unwrap();
        println!("{:#?}", art);
}
AlePon123 commented 1 year ago

@oknozor can you please fix it?

AlePon123 commented 1 year ago

Note that are some releases have string id type but some have integer type

oknozor commented 1 year ago

Hey @AlePon123 I am sorry but I am in vacation right now. But you have already figured out what was going wrong. Maybe you can submit a PR ? Otherwise i'll fix when I am back.

CrendKing commented 1 year ago

@oknozor The change I needed is simple: https://github.com/oknozor/musicbrainz_rs/pull/81. Not sure if it's the same AlePon123 needs.