Peter-Schorn / SpotifyAPI

A Swift library for the Spotify web API. Supports all endpoints.
https://peter-schorn.github.io/SpotifyAPI/documentation/spotifywebapi
MIT License
251 stars 32 forks source link

null snapshot ID on `removeAllOccurrencesFromPlaylist` causes a 500 error #36

Closed shaundon closed 2 years ago

shaundon commented 2 years ago

Hi, me again.

I ran into a strange error today and after some investigation I think I found the root cause.

When calling removeAllOccurrencesFromPlaylist, if you don't pass in a snapshotId (which is an optional argument), the body that's sent to api.spotify.com looks like this:

{
  "tracks": [
    {
      "uri": "spotify:track:7G6TDmGJXDoBaV6NGj3Yic"
    },
    {
      "uri": "spotify:track:3lKZf71jeNi14udIFPJ29V"
    },
    {
      "uri": "spotify:track:0b16LTzby1YRVd2nq2Z0fw"
    }
  ],
  "snapshot_id": null
}

This causes a 500 error on Spotify's API. The API should obviously handle this better, I'll open an issue on Spotify's as well as this one.

If I remove the "snapshot_id": null part, the request proceeds correctly. Therefore I'm wondering if it makes sense to not include snapshot_id in the request body if it's nil?

I'd be happy to make a pull request to do this myself, I think it would be relatively straightforward.

Edit: It's also worth noting that there's a very simple workaround for this error – just don't pass in a nil snapshotId, which is what I did.

Peter-Schorn commented 2 years ago

Thanks for discovering this bug!

The reason that snapshot_id was included in the JSON payload with a value of null instead of being omitted entirely (which is the pattern used by the synthesized Codable conformance for optional properties) was because I made a call to KeyedEncodingContainer.encode(_:forKey:) instead of KeyedEncodingContainer.encodeIfPresent(_:forKey:) for URIsContainer.snapshotId. The latter method omits the value from the JSON payload entirely if the property is nil. https://github.com/Peter-Schorn/SpotifyAPI/blob/6ba2711c7ea80d870b93c0923379351640584bd7/Sources/SpotifyWebAPI/Object%20Model/Post%20Request%20Objects/Playlist%20Objects/URIsContainer.swift#L77

I have since fixed this. Expect a new version shortly.

shaundon commented 2 years ago

Amazing, thanks for the detailed explanation and fast response! ❤️