ramsayleung / rspotify

Spotify Web API SDK implemented on Rust
MIT License
632 stars 121 forks source link

json parse error when getting playlist with no image #459

Closed aome510 closed 3 months ago

aome510 commented 6 months ago

Describe the bug Calling get-playlist API with playlist having no image will return a response with images field null. However, according to https://docs.rs/rspotify-model/0.12.0/rspotify_model/playlist/struct.FullPlaylist.html, rspotify specifies the field as Vec<Image>. This leads to the following error when parsing the json response:

json parse error: invalid type: null, expected a sequence at line 1 column 293: invalid type: null, expected a sequence at line 1 column 293

To Reproduce Steps to reproduce the behavior:

  1. Create a new playlist with Spotify, making sure that it has no image
  2. Call get-playlist API using the new playlist's URI
  3. Observe images field has a value of null
ramsayleung commented 6 months ago

I was unable to reproduce this problem:

#[maybe_async::test(feature = "__sync", async(feature = "__async", tokio::test))]
async fn test_playlist_with_no_image() {
    let playlist_id = PlaylistId::from_id("7G9P6fNlHvQ8lkObO5MJ7O").unwrap();
    let playlist = creds_client()
        .await
        .playlist(playlist_id, None, None)
        .await
        .unwrap();
    println!("playlist: {:?}", playlist);
    assert_eq!(playlist.images.len(), 0)
}

The images field from the response is empty rather than has a value of null:

playlist: FullPlaylist { collaborative: false, description: Some(""), external_urls: {"spotify": "https://open.spotify.com/playlist/7G9P6fNlHvQ8lkObO5MJ7O"}, followers: Followers { total: 0 }, href: "https://api.spotify.com/v1/playlists/7G9P6fNlHvQ8lkObO5MJ7O", id: PlaylistId("7G9P6fNlHvQ8lkObO5MJ7O"), images: [], name: "A New Playlist", owner: PublicUser { display_name: Some("Samray Leung"), external_urls: {"spotify": "https://open.spotify.com/user/2257tjys2e2u2ygfke42niy2q"}, followers: None, href: "https://api.spotify.com/v1/users/2257tjys2e2u2ygfke42niy2q", id: UserId("2257tjys2e2u2ygfke42niy2q"), images: [] }, public: Some(false), snapshot_id: "Miw2MmI2NzNkZjBlMjJjZTU0M2UzODg4NjcxZDQzZTBmZTljYWM1YTc1", tracks: Page { href: "https://api.spotify.com/v1/playlists/7G9P6fNlHvQ8lkObO5MJ7O/tracks?offset=0&limit=100", items: [], limit: 100, next: None, offset: 0, previous: None, total: 0 } }
aome510 commented 6 months ago

I was unable to reproduce this problem:

#[maybe_async::test(feature = "__sync", async(feature = "__async", tokio::test))]
async fn test_playlist_with_no_image() {
    let playlist_id = PlaylistId::from_id("7G9P6fNlHvQ8lkObO5MJ7O").unwrap();
    let playlist = creds_client()
        .await
        .playlist(playlist_id, None, None)
        .await
        .unwrap();
    println!("playlist: {:?}", playlist);
    assert_eq!(playlist.images.len(), 0)
}

The images field from the response is empty rather than has a value of null:

playlist: FullPlaylist { collaborative: false, description: Some(""), external_urls: {"spotify": "https://open.spotify.com/playlist/7G9P6fNlHvQ8lkObO5MJ7O"}, followers: Followers { total: 0 }, href: "https://api.spotify.com/v1/playlists/7G9P6fNlHvQ8lkObO5MJ7O", id: PlaylistId("7G9P6fNlHvQ8lkObO5MJ7O"), images: [], name: "A New Playlist", owner: PublicUser { display_name: Some("Samray Leung"), external_urls: {"spotify": "https://open.spotify.com/user/2257tjys2e2u2ygfke42niy2q"}, followers: None, href: "https://api.spotify.com/v1/users/2257tjys2e2u2ygfke42niy2q", id: UserId("2257tjys2e2u2ygfke42niy2q"), images: [] }, public: Some(false), snapshot_id: "Miw2MmI2NzNkZjBlMjJjZTU0M2UzODg4NjcxZDQzZTBmZTljYWM1YTc1", tracks: Page { href: "https://api.spotify.com/v1/playlists/7G9P6fNlHvQ8lkObO5MJ7O/tracks?offset=0&limit=100", items: [], limit: 100, next: None, offset: 0, previous: None, total: 0 } }

Pretty weird. My playlist is public, you can try mine instead. The id is 2gaCJBCZ1h90ZdIjNV6SWw.

> curl --request GET \
          --url https://api.spotify.com/v1/playlists/2gaCJBCZ1h90ZdIjNV6SWw \
          --header 'Authorization: Bearer $TOKEN' | jq

{
  "collaborative": false,
  "description": "just a test playlist",
  "external_urls": {
    "spotify": "https://open.spotify.com/playlist/2gaCJBCZ1h90ZdIjNV6SWw"
  },
  "followers": {
    "href": null,
    "total": 0
  },
  "href": "https://api.spotify.com/v1/playlists/2gaCJBCZ1h90ZdIjNV6SWw",
  "id": "2gaCJBCZ1h90ZdIjNV6SWw",
  "images": null,
  "name": "new playlist",
  "owner": {
    "display_name": "Thang Pham",
    "external_urls": {
      "spotify": "https://open.spotify.com/user/31bewedqaetpovv3yqnw44jtphsy"
    },
    "href": "https://api.spotify.com/v1/users/31bewedqaetpovv3yqnw44jtphsy",
    "id": "31bewedqaetpovv3yqnw44jtphsy",
    "type": "user",
    "uri": "spotify:user:31bewedqaetpovv3yqnw44jtphsy"
  },
  "primary_color": null,
  "public": true,
  "snapshot_id": "MiwxOGE5NGQ5NmU1MmNiYjdiNjYxOGU4ZDQzNjhlOTQ3YjQ3NTA3ZGMy",
  "tracks": {
    "href": "https://api.spotify.com/v1/playlists/2gaCJBCZ1h90ZdIjNV6SWw/tracks?offset=0&limit=0",
    "items": [],
    "limit": 0,
    "next": null,
    "offset": 0,
    "previous": null,
    "total": 0
  },
  "type": "playlist",
  "uri": "spotify:playlist:2gaCJBCZ1h90ZdIjNV6SWw"
}
ramsayleung commented 6 months ago

Could you provide the code snippet you were using when you encountered this error?

aome510 commented 6 months ago

Not sure what you mean by code snippet. I encountered this error when testing https://github.com/aome510/spotify-player/pull/379. After creating new playlist with create-playlist API, I couldn't enter the playlist page because the client fails to retrieve new playlist's data.

ramsayleung commented 6 months ago

Even though I change the test playlist id to yours, I'm still unable to reproduce this error, the images field is deserialized as an empty vector, rather than throwing an error:

#[maybe_async::test(feature = "__sync", async(feature = "__async", tokio::test))]
async fn test_playlist_with_no_image() {
    let playlist_id = PlaylistId::from_id("2gaCJBCZ1h90ZdIjNV6SWw").unwrap();
    let playlist = creds_client()
        .await
        .playlist(playlist_id, None, None)
        .await
        .unwrap();
    println!("playlist: {:?}", playlist);
    assert_eq!(playlist.images.len(), 0)
}
...
playlist: FullPlaylist { collaborative: false, description: Some("just a test playlist"), external_urls: {"spotify": "https://open.spotify.com/playlist/2gaCJBCZ1h90ZdIjNV6SWw"}, followers: Followers { total: 0 }, href: "https://api.spotify.com/v1/playlists/2gaCJBCZ1h90ZdIjNV6SWw", id: PlaylistId("2gaCJBCZ1h90ZdIjNV6SWw"), images: [], name: "new playlist", owner: PublicUser { display_name: Some("Thang Pham"), external_urls: {"spotify": "https://open.spotify.com/user/31bewedqaetpovv3yqnw44jtphsy"}, followers: None, href: "https://api.spotify.com/v1/users/31bewedqaetpovv3yqnw44jtphsy", id: UserId("31bewedqaetpovv3yqnw44jtphsy"), images: [] }, public: Some(false), snapshot_id: "MiwxOGE5NGQ5NmU1MmNiYjdiNjYxOGU4ZDQzNjhlOTQ3YjQ3NTA3ZGMy", tracks: Page { href: "https://api.spotify.com/v1/playlists/2gaCJBCZ1h90ZdIjNV6SWw/tracks?offset=0&limit=100", items: [], limit: 100, next: None, offset: 0, previous: None, total: 0 } }

test test_playlist_with_no_image ... ok
...
aome510 commented 6 months ago

hmmm, interesting. Maybe the result is user-specific 🤔 .That said, it's not a breaking change to add #serde(default) for images field of FullPlaylist and SimplifiedPlaylist right?

ramsayleung commented 6 months ago

#[serde(default)] is not considered a breaking change, then, what's the point of this?

aome510 commented 6 months ago

It will help to not throw an error when parsing playlist with null images field

BeauCranston commented 5 months ago

Hello, I just encountered an error similar to this and my problem was that I had a playlist that was empty however I am on version 0.10 since I am using spotify-tui so this may be irrelevant but thought I would leave this here just in case.

As soon as I added a song to my empty playlist everything worked fine.

trumank commented 5 months ago

It seems Spotify is returning different responses (the image field specifically) depending on the account/bearer token used.

Using rspotify's test account bearer:

{
  "collaborative": false,
  "description": "just a test playlist",
  "external_urls": {
    "spotify": "https://open.spotify.com/playlist/2gaCJBCZ1h90ZdIjNV6SWw"
  },
  "followers": {
    "href": null,
    "total": 0
  },
  "href": "https://api.spotify.com/v1/playlists/2gaCJBCZ1h90ZdIjNV6SWw",
  "id": "2gaCJBCZ1h90ZdIjNV6SWw",
  "images": [],
  "name": "new playlist",
  "owner": {
    "display_name": "Thang Pham",
    "external_urls": {
      "spotify": "https://open.spotify.com/user/31bewedqaetpovv3yqnw44jtphsy"
    },
    "href": "https://api.spotify.com/v1/users/31bewedqaetpovv3yqnw44jtphsy",
    "id": "31bewedqaetpovv3yqnw44jtphsy",
    "type": "user",
    "uri": "spotify:user:31bewedqaetpovv3yqnw44jtphsy"
  },
  "primary_color": null,
  "public": false,
  "snapshot_id": "MiwxOGE5NGQ5NmU1MmNiYjdiNjYxOGU4ZDQzNjhlOTQ3YjQ3NTA3ZGMy",
  "tracks": {
    "href": "https://api.spotify.com/v1/playlists/2gaCJBCZ1h90ZdIjNV6SWw/tracks?offset=0&limit=100",
    "items": [],
    "limit": 100,
    "next": null,
    "offset": 0,
    "previous": null,
    "total": 0
  },
  "type": "playlist",
  "uri": "spotify:playlist:2gaCJBCZ1h90ZdIjNV6SWw"
}

Using the bearer token for my personal account obtained from ncspot's log:

{
  "collaborative": false,
  "description": "just a test playlist",
  "external_urls": {
    "spotify": "https://open.spotify.com/playlist/2gaCJBCZ1h90ZdIjNV6SWw"
  },
  "followers": {
    "href": null,
    "total": 0
  },
  "href": "https://api.spotify.com/v1/playlists/2gaCJBCZ1h90ZdIjNV6SWw",
  "id": "2gaCJBCZ1h90ZdIjNV6SWw",
  "images": null,
  "name": "new playlist",
  "owner": {
    "display_name": "Thang Pham",
    "external_urls": {
      "spotify": "https://open.spotify.com/user/31bewedqaetpovv3yqnw44jtphsy"
    },
    "href": "https://api.spotify.com/v1/users/31bewedqaetpovv3yqnw44jtphsy",
    "id": "31bewedqaetpovv3yqnw44jtphsy",
    "type": "user",
    "uri": "spotify:user:31bewedqaetpovv3yqnw44jtphsy"
  },
  "primary_color": null,
  "public": true,
  "snapshot_id": "MiwxOGE5NGQ5NmU1MmNiYjdiNjYxOGU4ZDQzNjhlOTQ3YjQ3NTA3ZGMy",
  "tracks": {
    "href": "https://api.spotify.com/v1/playlists/2gaCJBCZ1h90ZdIjNV6SWw/tracks?offset=0&limit=0",
    "items": [],
    "limit": 0,
    "next": null,
    "offset": 0,
    "previous": null,
    "total": 0
  },
  "type": "playlist",
  "uri": "spotify:playlist:2gaCJBCZ1h90ZdIjNV6SWw"
}
trumank commented 5 months ago

And this patch allows playlists to load for me in ncspot:

diff --git a/rspotify-model/src/playlist.rs b/rspotify-model/src/playlist.rs
index 07bb7b2..776f1e2 100644
--- a/rspotify-model/src/playlist.rs
+++ b/rspotify-model/src/playlist.rs
@@ -5,10 +5,18 @@ use serde::{Deserialize, Serialize};

 use std::collections::HashMap;

 use crate::{Followers, Image, Page, PlayableItem, PlaylistId, PublicUser};

+fn deserialize_null_default<'de, D, T>(deserializer: D) -> Result<T, D::Error>
+where
+    T: Default + serde::Deserialize<'de>,
+    D: serde::Deserializer<'de>,
+{
+    Ok(Option::deserialize(deserializer)?.unwrap_or_default())
+}
+
 /// Playlist result object
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Default)]
 pub struct PlaylistResult {
     pub snapshot_id: String,
 }
@@ -25,10 +33,11 @@ pub struct PlaylistTracksRef {
 pub struct SimplifiedPlaylist {
     pub collaborative: bool,
     pub external_urls: HashMap<String, String>,
     pub href: String,
     pub id: PlaylistId<'static>,
+    #[serde(deserialize_with = "deserialize_null_default")]
     pub images: Vec<Image>,
     pub name: String,
     pub owner: PublicUser,
     pub public: Option<bool>,
     pub snapshot_id: String,
aome510 commented 5 months ago

And this patch allows playlists to load for me in ncspot:

diff --git a/rspotify-model/src/playlist.rs b/rspotify-model/src/playlist.rs
index 07bb7b2..776f1e2 100644
--- a/rspotify-model/src/playlist.rs
+++ b/rspotify-model/src/playlist.rs
@@ -5,10 +5,18 @@ use serde::{Deserialize, Serialize};

 use std::collections::HashMap;

 use crate::{Followers, Image, Page, PlayableItem, PlaylistId, PublicUser};

+fn deserialize_null_default<'de, D, T>(deserializer: D) -> Result<T, D::Error>
+where
+    T: Default + serde::Deserialize<'de>,
+    D: serde::Deserializer<'de>,
+{
+    Ok(Option::deserialize(deserializer)?.unwrap_or_default())
+}
+
 /// Playlist result object
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Default)]
 pub struct PlaylistResult {
     pub snapshot_id: String,
 }
@@ -25,10 +33,11 @@ pub struct PlaylistTracksRef {
 pub struct SimplifiedPlaylist {
     pub collaborative: bool,
     pub external_urls: HashMap<String, String>,
     pub href: String,
     pub id: PlaylistId<'static>,
+    #[serde(deserialize_with = "deserialize_null_default")]
     pub images: Vec<Image>,
     pub name: String,
     pub owner: PublicUser,
     pub public: Option<bool>,
     pub snapshot_id: String,

#[serde(default)] should be enough

trumank commented 5 months ago

#[serde(default)] will fail if the key is present with a null value:

use serde::Deserialize;

fn deserialize_null_default<'de, D, T>(deserializer: D) -> Result<T, D::Error>
where
    T: Default + serde::Deserialize<'de>,
    D: serde::Deserializer<'de>,
{
    Ok(Option::deserialize(deserializer)?.unwrap_or_default())
}

#[derive(Debug, Deserialize)]
struct ObjDefault {
    #[serde(default)]
    field: Vec<String>,
}

#[derive(Debug, Deserialize)]
struct ObjCustom {
    #[serde(deserialize_with = "deserialize_null_default")]
    field: Vec<String>,
}

const JSON_NULL: &str = r#"{"field": null}"#;
const JSON_ARRAY: &str = r#"{"field": ["a", "b"]}"#;
const JSON_MISSING: &str = r#"{}"#;

#[test]
fn test_default_null() {
    serde_json::from_str::<ObjDefault>(JSON_NULL).unwrap();
}
#[test]
fn test_default_array() {
    serde_json::from_str::<ObjDefault>(JSON_ARRAY).unwrap();
}
#[test]
fn test_default_missing() {
    serde_json::from_str::<ObjDefault>(JSON_MISSING).unwrap();
}
#[test]
fn test_custom_null() {
    serde_json::from_str::<ObjCustom>(JSON_NULL).unwrap();
}
#[test]
fn test_custom_array() {
    serde_json::from_str::<ObjCustom>(JSON_ARRAY).unwrap();
}
#[test]
fn test_custom_missing() {
    serde_json::from_str::<ObjCustom>(JSON_MISSING).unwrap();
}
$ cargo test
running 6 tests
test test_custom_array ... ok
test test_custom_missing ... FAILED
test test_default_missing ... ok
test test_custom_null ... ok
test test_default_array ... ok
test test_default_null ... FAILED

failures:

---- test_custom_missing stdout ----
thread 'test_custom_missing' panicked at serde-test/src/main.rs:51:53:
called `Result::unwrap()` on an `Err` value: Error("missing field `field`", line: 1, column: 2)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- test_default_null stdout ----
thread 'test_default_null' panicked at serde-test/src/main.rs:31:51:
called `Result::unwrap()` on an `Err` value: Error("invalid type: null, expected a sequence", line: 1, column: 14)

failures:
    test_custom_missing
    test_default_null

test result: FAILED. 4 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
aome510 commented 5 months ago

I see, thanks for testing it.

Icelk commented 5 months ago

In the meantime, one may use the API to check which playlist is the culprit. Go the the spotify API documentation link below and test the query with limit = 50 offset = 0 on your own account (you need to be logged in): https://developer.spotify.com/documentation/web-api/reference/get-a-list-of-current-users-playlists Then Ctrl+F for "images": null

00alia00 commented 5 months ago

@Icelk Thanks, that was a life saver! An empty playlist (and therefore no image) was killing it for me! Adding an image to the playlist fixed it.

trumank commented 3 months ago

I believe this should be closed now that #480 is merged?

ramsayleung commented 3 months ago

Closing this issue as the problem has been resolved in #480 .

flrgh commented 3 months ago

Thanks @ramsayleung! Would you be able to cut a patch release for this fix? I have been sourcing this dep from master, but it would be nice to switch back to a release.

ramsayleung commented 3 months ago

The patch release v.0.13.2 has been release #485 :-)