Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sequential Pagination of artist albums never terminates #492

Closed
infality opened this issue Sep 1, 2024 · 3 comments
Closed

Sequential Pagination of artist albums never terminates #492

infality opened this issue Sep 1, 2024 · 3 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@infality
Copy link

infality commented Sep 1, 2024

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

@infality infality added bug Something isn't working help wanted Extra attention is needed labels Sep 1, 2024
@ramsayleung
Copy link
Owner

ramsayleung commented Sep 13, 2024

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
Copy link
Owner

ramsayleung commented Sep 13, 2024

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

@infality
Copy link
Author

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants