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