ramsayleung / rspotify

Spotify Web API SDK implemented on Rust
MIT License
639 stars 122 forks source link

Sequential Pagination of artist albums never terminates #492

Closed infality closed 1 month ago

infality commented 1 month ago

Describe the bug When fetching an artist's albums using the async pagination the request goes on endlessly. The albums are fetched successfully, but then the while loop does not terminate. I can even see that there is traffic happening on the network interface. I let it run for 1-2 minutes before killing it. Happens with different artists as well. I'm using pretty much the same code as the sequential example from pagination_async.rs.

I don't use Rust async a lot, so this may be as well an error on my part, but I'm pretty sure this used to terminate before.

To Reproduce Taken mostly from the pagination_async.rs example

use futures::stream::TryStreamExt;
use futures_util::pin_mut;
use rspotify::{
    model::{AlbumType, ArtistId, Country, Market},
    prelude::*,
    ClientCredsSpotify, Credentials,
};

#[tokio::main]
async fn main() {
    env_logger::init();
    let creds = Credentials::from_env().unwrap();
    let spotify = ClientCredsSpotify::new(creds);
    spotify.request_token().await.unwrap();

    let stream = spotify.artist_albums(
        ArtistId::from_id("4AA8eXtzqh5ykxtafLaPOi").unwrap(),
        Some(AlbumType::Album),
        Some(Market::Country(Country::UnitedStates)),
    );
    pin_mut!(stream);
    println!("Items (blocking):");
    while let Some(item) = stream.try_next().await.unwrap() {
        println!("* {}", item.name);
    }
}

cargo.toml dependencies

[dependencies]
rspotify = { version = "0.13", features = ["env-file"]}
tokio = { version = "1.39", features = ["full"] }
futures = "0.3"
futures-util = "0.3"
env_logger = "0.11.5"

Expected behavior Program should terminate

Log/Output data

Items (blocking):
* Anomaly
* Not All The Beautiful Things
* Divide & Conquer (Remixes)
* Divide & Conquer
ramsayleung commented 1 month ago

Thanks for your detailed report, I can reproduce this problem with your code. After adding a logging statement at the paginate_with_ctx function, I figure out what's going wrong:

pub fn paginate_with_ctx<'a, Ctx: 'a + Send, T, Request>(
    ctx: Ctx,
    req: Request,
    page_size: u32,
) -> Paginator<'a, ClientResult<T>>
where
    T: 'a + Unpin + Send + std::fmt::Debug,
    Request: 'a + for<'ctx> Fn(&'ctx Ctx, u32, u32) -> RequestFuture<'ctx, T> + Send,
{
    use async_stream::stream;
    let mut offset = 0;
    Box::pin(stream! {
        loop {
            let request = req(&ctx, page_size, offset);
            let page = request.await?;
            println!("page: {:?}", page); // newly added logging statement
            offset += page.items.len() as u32;
            for item in page.items {
                yield Ok(item);
            }
            if page.next.is_none() {
                break;
            }
        }
    })
}

The standard output:

Items (blocking):
page: Page { href: "https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=0&limit=50&market=US&include_groups=album", items: [SimplifiedAlbum { album_group: Some("album"), album_type: Some("album"), artists: [SimplifiedArtist { external_urls: {"spotify": "https://open.spotify.com/artist/4AA8eXtzqh5ykxtafLaPOi"}, href: Some("https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi"), id: Some(ArtistId("4AA8eXtzqh5ykxtafLaPOi")), name: "What So Not" }], available_markets: [], external_urls: {"spotify": "https://open.spotify.com/album/7J8SSnGauta2Uy7HKhEDMA"}, href: Some("https://api.spotify.com/v1/albums/7J8SSnGauta2Uy7HKhEDMA"), id: Some(AlbumId("7J8SSnGauta2Uy7HKhEDMA")), images: [Image { height: Some(640), url: "https://i.scdn.co/image/ab67616d0000b2736c547112826dc9445dc87880", width: Some(640) }, Image { height: Some(300), url: "https://i.scdn.co/image/ab67616d00001e026c547112826dc9445dc87880", width: Some(300) }, Image { height: Some(64), url: "https://i.scdn.co/image/ab67616d000048516c547112826dc9445dc87880", width: Some(64) }], name: "Anomaly", release_date: Some("2022-09-16"), release_date_precision: Some("day"), restrictions: None }, SimplifiedAlbum { album_group: Some("album"), album_type: Some("album"), artists: [SimplifiedArtist { external_urls: {"spotify": "https://open.spotify.com/artist/4AA8eXtzqh5ykxtafLaPOi"}, href: Some("https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi"), id: Some(ArtistId("4AA8eXtzqh5ykxtafLaPOi")), name: "What So Not" }], available_markets: [], external_urls: {"spotify": "https://open.spotify.com/album/5282LhSm8KbFddnDh9aaPv"}, href: Some("https://api.spotify.com/v1/albums/5282LhSm8KbFddnDh9aaPv"), id: Some(AlbumId("5282LhSm8KbFddnDh9aaPv")), images: [Image { height: Some(640), url: "https://i.scdn.co/image/ab67616d0000b273259fa5105f5daaae35a0ed31", width: Some(640) }, Image { height: Some(300), url: "https://i.scdn.co/image/ab67616d00001e02259fa5105f5daaae35a0ed31", width: Some(300) }, Image { height: Some(64), url: "https://i.scdn.co/image/ab67616d00004851259fa5105f5daaae35a0ed31", width: Some(64) }], name: "Not All The Beautiful Things", release_date: Some("2018-03-09"), release_date_precision: Some("day"), restrictions: None }, SimplifiedAlbum { album_group: Some("album"), album_type: Some("album"), artists: [SimplifiedArtist { external_urls: {"spotify": "https://open.spotify.com/artist/4AA8eXtzqh5ykxtafLaPOi"}, href: Some("https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi"), id: Some(ArtistId("4AA8eXtzqh5ykxtafLaPOi")), name: "What So Not" }], available_markets: [], external_urls: {"spotify": "https://open.spotify.com/album/3XaUNjcSJ6oyhoaFTqRbLb"}, href: Some("https://api.spotify.com/v1/albums/3XaUNjcSJ6oyhoaFTqRbLb"), id: Some(AlbumId("3XaUNjcSJ6oyhoaFTqRbLb")), images: [Image { height: Some(640), url: "https://i.scdn.co/image/ab67616d0000b27332f252931e059d460c188731", width: Some(640) }, Image { height: Some(300), url: "https://i.scdn.co/image/ab67616d00001e0232f252931e059d460c188731", width: Some(300) }, Image { height: Some(64), url: "https://i.scdn.co/image/ab67616d0000485132f252931e059d460c188731", width: Some(64) }], name: "Divide & Conquer (Remixes)", release_date: Some("2017-06-16"), release_date_precision: Some("day"), restrictions: None }, SimplifiedAlbum { album_group: Some("album"), album_type: Some("album"), artists: [SimplifiedArtist { external_urls: {"spotify": "https://open.spotify.com/artist/4AA8eXtzqh5ykxtafLaPOi"}, href: Some("https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi"), id: Some(ArtistId("4AA8eXtzqh5ykxtafLaPOi")), name: "What So Not" }], available_markets: [], external_urls: {"spotify": "https://open.spotify.com/album/2EvKkEYA4lEptctltrFEpz"}, href: Some("https://api.spotify.com/v1/albums/2EvKkEYA4lEptctltrFEpz"), id: Some(AlbumId("2EvKkEYA4lEptctltrFEpz")), images: [Image { height: Some(640), url: "https://i.scdn.co/image/ab67616d0000b2731c74e68fa3bcdcb9028d7792", width: Some(640) }, Image { height: Some(300), url: "https://i.scdn.co/image/ab67616d00001e021c74e68fa3bcdcb9028d7792", width: Some(300) }, Image { height: Some(64), url: "https://i.scdn.co/image/ab67616d000048511c74e68fa3bcdcb9028d7792", width: Some(64) }], name: "Divide & Conquer", release_date: Some("2016-09-09"), release_date_precision: Some("day"), restrictions: None }], limit: 50, next: Some("https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=50&limit=50&market=US&include_groups=album"), offset: 0, previous: None, total: 97 }
* Anomaly
* Not All The Beautiful Things
* Divide & Conquer (Remixes)
* Divide & Conquer
page: Page { href: "https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=4&limit=50&market=US&include_groups=album", items: [], limit: 50, next: Some("https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=54&limit=50&market=US&include_groups=album"), offset: 4, previous: Some("https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=0&limit=50&market=US&include_groups=album"), total: 97 }
page: Page { href: "https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=4&limit=50&market=US&include_groups=album", items: [], limit: 50, next: Some("https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=54&limit=50&market=US&include_groups=album"), offset: 4, previous: Some("https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=0&limit=50&market=US&include_groups=album"), total: 97 }
page: Page { href: "https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=4&limit=50&market=US&include_groups=album", items: [], limit: 50, next: Some("https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=54&limit=50&market=US&include_groups=album"), offset: 4, previous: Some("https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=0&limit=50&market=US&include_groups=album"), total: 97 }
page: Page { href: "https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=4&limit=50&market=US&include_groups=album", items: [], limit: 50, next: Some("https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=54&limit=50&market=US&include_groups=album"), offset: 4, previous: Some("https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=0&limit=50&market=US&include_groups=album"), total: 97 }
page: Page { href: "https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=4&limit=50&market=US&include_groups=album", items: [], limit: 50, next: Some("https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=54&limit=50&market=US&include_groups=album"), offset: 4, previous: Some("https://api.spotify.com/v1/artists/4AA8eXtzqh5ykxtafLaPOi/albums?offset=0&limit=50&market=US&include_groups=album"), total: 97 }
....
infinite page information.

If you observe carefully, you will notice the Spotify server keep returning next field(which is not None), but the items is empty, it totally doesn't make sense that the response doesn't contain item in the current page, but you are telling me that you have more items in the next page.

Now Spotify is also telling me this artist has 97 albums, it's a lie(or you could call it as a bug). I've tested this album in the web console provided by Spotify, they only have 4 items.

Obviously, there are two bugs on the Spotify server side, I will create a patch to bypass this problem by checking the items field, it's a bit frustrating for library developer to keep taking care of the problems caused by Spotify.

ramsayleung commented 1 month ago

You could checkout the master branch with latest commit and rerun your code, this problem should be addressed :)

infality commented 1 month ago

Thank you, it works with the master branch.

I also tested a few API calls with curl and noticed that in this case the next field is only null if offset is greater than 46. With other artists the threshold is somewhere else. I also saw a total album count of 682 on another artist.

This is completely broken and has been for several weeks now... At least I know now that the problem was not on my end :)