refactor: propagate coverart execution error upstream instead of pani…

[CrendKing]

Related: https://github.com/oknozor/musicbrainz_rs/issues/77

Currently both the FetchCoverartQuery::execute()s just panic if error happens in network request. Conventionally in a Rust library, errors should be propagated upstream so lib user can decide what to do with them.

Note: I tried to run the tests, but some of them fail even without any change. Maybe the tests are not stable?

List:

async: ``` failures: async_tests::area::area_includes::should_get_area_aliases async_tests::area::area_includes::should_get_area_annotation async_tests::area::area_includes::should_get_area_area_relations async_tests::area::area_includes::should_get_area_event_relations async_tests::area::area_includes::should_get_area_release_relations async_tests::area::area_includes::should_get_area_url_relations async_tests::artist::artist_browse::should_browse_artist_by_area async_tests::artist::artist_browse::should_browse_artist_by_recording async_tests::artist::artist_browse::should_browse_artist_by_release async_tests::artist::artist_browse::should_browse_artist_by_work async_tests::artist::artist_includes::should_get_artist_aliases async_tests::artist::artist_includes::should_get_artist_annotation async_tests::artist::artist_includes::should_get_artist_artist_relations async_tests::artist::artist_includes::should_get_artist_artist_releases_with_disc_ids async_tests::artist::artist_includes::should_get_artist_event_relations async_tests::artist::artist_includes::should_get_artist_genres async_tests::artist::artist_includes::should_get_artist_rating async_tests::artist::artist_includes::should_get_artist_recording_relations async_tests::artist::artist_includes::should_get_artist_recordings async_tests::artist::artist_includes::should_get_artist_release_groups async_tests::artist::artist_includes::should_get_artist_release_relations async_tests::artist::artist_includes::should_get_artist_series_relations async_tests::artist::artist_includes::should_get_artist_tags async_tests::artist::artist_includes::should_get_artist_url_relations async_tests::artist::artist_includes::should_get_artist_work_relations async_tests::artist::artist_includes::should_get_artist_works async_tests::cdstub::cdstub_search::should_search_cdstub async_tests::event::event_browse::should_browse_event_by_area async_tests::event::event_browse::should_browse_event_by_artist async_tests::event::event_includes::should_get_event_aliases async_tests::event::event_includes::should_get_event_annotation async_tests::event::event_includes::should_get_event_artist_relations async_tests::event::event_includes::should_get_event_genres async_tests::event::event_includes::should_get_event_place_relations async_tests::event::event_includes::should_get_event_rating async_tests::event::event_includes::should_get_event_tags async_tests::event::event_includes::should_get_event_url_relations async_tests::instrument::instrument_includes::should_get_instrument_aliases async_tests::instrument::instrument_includes::should_get_instrument_annotation async_tests::instrument::instrument_includes::should_get_instrument_genres async_tests::instrument::instrument_includes::should_get_instrument_tags async_tests::instrument::instrument_includes::should_get_instrument_url_relations async_tests::instrument::instrument_search::should_search_instrument async_tests::label::label_browse::should_browse_label_by_area async_tests::label::label_browse::should_browse_label_by_collection async_tests::label::label_browse::should_browse_label_by_release async_tests::label::label_includes::should_get_label_aliases async_tests::label::label_includes::should_get_label_annotation async_tests::label::label_includes::should_get_label_artist_relations async_tests::label::label_includes::should_get_label_genres async_tests::label::label_includes::should_get_label_label_relations async_tests::label::label_includes::should_get_label_rating async_tests::label::label_includes::should_get_label_recording_relations async_tests::label::label_includes::should_get_label_release_relations async_tests::label::label_includes::should_get_label_releases async_tests::label::label_includes::should_get_label_tags async_tests::label::label_includes::should_get_label_url_relations async_tests::label::label_search::should_search_label async_tests::place::place_browse::should_browse_place_by_area async_tests::place::place_includes::should_get_place_aliases async_tests::place::place_includes::should_get_place_annotation async_tests::place::place_includes::should_get_place_event_relations async_tests::place::place_includes::should_get_place_tags async_tests::recording::recording_browse::should_browse_recording_by_artist async_tests::recording::recording_includes::should_get_recording_aliases async_tests::recording::recording_includes::should_get_recording_annotation async_tests::recording::recording_includes::should_get_recording_url_relations async_tests::recording::recording_includes::should_get_recording_work_relations async_tests::recording::recording_search::should_search_recording async_tests::release::release_browse::should_browse_release_by_area async_tests::release::release_browse::should_browse_release_by_artist async_tests::release::release_browse::should_browse_release_by_collection async_tests::release::release_browse::should_browse_release_by_label async_tests::release::release_browse::should_browse_release_by_release_group async_tests::release::release_browse::should_browse_release_by_track async_tests::release::release_includes::should_get_release_annotation async_tests::release::release_includes::should_get_release_genres async_tests::release::release_includes::should_get_release_level_relations async_tests::release::release_includes::should_get_release_media async_tests::release::release_includes::should_get_release_recordings async_tests::release::release_includes::should_get_release_release_groups async_tests::release::release_search::should_search_artist async_tests::release_group::release_group_browse::should_browse_release_group_by_artist async_tests::release_group::release_group_browse::should_browse_release_group_by_collection async_tests::release_group::release_group_browse::should_browse_release_group_by_release async_tests::release_group::release_group_coverart::should_get_release_group_coverart_after_fetch async_tests::release_group::release_group_includes::should_get_release_group_aliases async_tests::release_group::release_group_includes::should_get_release_group_artists async_tests::release_group::release_group_includes::should_get_release_group_genres async_tests::release_group::release_group_includes::should_get_release_group_rating async_tests::release_group::release_group_includes::should_get_release_group_releases async_tests::release_group::release_group_includes::should_get_release_group_series_relations async_tests::release_group::release_group_includes::should_get_release_group_url_relations async_tests::release_group::release_group_search::should_search_artist async_tests::series::series_includes::should_get_series_aliases async_tests::series::series_includes::should_get_series_annotation async_tests::series::series_includes::should_get_series_genres async_tests::series::series_includes::should_get_series_tags async_tests::work::work_browse::should_browse_work_by_collection async_tests::work::work_includes::should_get_work_aliases async_tests::work::work_includes::should_get_work_annotation async_tests::work::work_includes::should_get_work_artist_relations async_tests::work::work_includes::should_get_work_genres async_tests::work::work_includes::should_get_work_label_relations async_tests::work::work_includes::should_get_work_recording_relations async_tests::work::work_includes::should_get_work_url_relations async_tests::work::work_includes::should_get_work_work_relations async_tests::work::work_search::should_search_work test result: FAILED. 59 passed; 108 failed; 0 ignored; 0 measured; 0 filtered out; finished in 74.12s ``` blocking: ``` failures: blocking_tests::artist::artist_includes::should_get_artist_series_relations blocking_tests::place::place_includes::should_get_place_genres blocking_tests::recording::recording_includes::should_get_recording_releases blocking_tests::recording::recording_includes::should_get_recording_tags blocking_tests::release_group::release_group_includes::should_get_release_group_aliases test result: FAILED. 162 passed; 5 failed; 0 ignored; 0 measured; 0 filtered out; finished in 104.77s ```

[AlePon123]

this is problem with deserialization struct. some coveart have int type ID field and some has String type

[AlePon123]

this is problem with deserialization struct. some coveart have int type ID field and some has String type

in my project i dont find better way to do this then i just hardcode it

#[derive(Debug, Serialize, Deserialize, Default)]
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, Default)]
struct CoverArtStringID {
    approved: bool,
    back: bool,
    comment: String,
    edit: u64,
    front: bool,
    id: String,
    image: String,
    thumbnails: Thumbnail,
    types: Vec<ImageType>,
}

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

#[derive(Debug, Serialize, Deserialize, Default)]
struct CoverartJsonStringID {
    images: Vec<CoverArtStringID>,
}
#[derive(Debug, Serialize, Deserialize, Default)]
struct CoverartJsonIntID {
    images: Vec<CoverArtIntID>,
}

async fn try_get_coverart(release: &Release, client: &reqwest::Client) -> Result<String, ()> {
    let url = "http://coverartarchive.org/release/".to_string() + release.id.as_str();
    let request = client.get(&url);
    if request.send().await.unwrap().status() == 200 {
        match reqwest::get(&url)
            .await
            .unwrap()
            .json::<CoverartJsonStringID>()
            .await
        {
            Ok(art) => return Ok(art.images[0].image.clone()),
            Err(_) => {
                let art = reqwest::get(&url)
                    .await
                    .unwrap()
                    .json::<CoverartJsonIntID>()
                    .await
                    .unwrap();
                return Ok(art.images[0].image.clone());
            }
        }
    }
    Err(())
}

[AlePon123]

here's example of coverart with string id: https://ia801300.us.archive.org/18/items/mbid-376c5157-09e2-47f4-80de-ae66b6350590/index.json and here's example of coverart with int id https://ia800105.us.archive.org/7/items/mbid-298338c3-2cf2-49fa-a65a-ff52f02dd366/index.json

[CrendKing]

@AlePon123, I understand your problem, but we are dealing with different issues. This PR tries to avoid panicking when the request to coverart query itself fails. Yours is to have different coverart entity definition.

Could you please open separate issue/PR?

[timvancann]

This is currently still broken, any progress on this PR?

[RustyNova016]

The tests aren't passing as it test on the live MB DB... And of course, it changes. I did fix them in my own fork, but didn't make this PR's change in it. I'll try to merge into mine too