thelinmichael / spotify-web-api-node

A Node.js wrapper for Spotify's Web API.
http://thelinmichael.github.io/spotify-web-api-node/
MIT License
3.1k stars 499 forks source link

Throw an error that 50 is the max for getTracks() #240

Open garyking opened 6 years ago

garyking commented 6 years ago

https://github.com/thelinmichael/spotify-web-api-node/blob/2fcd60c30368255dab658b534b4229909ace5d43/src/spotify-web-api.js#L145

For getTracks(), 50 is the max number of IDs to get, according to the Spotify API. Please throw an error indicating such in the method, because otherwise it becomes difficult to diagnose where the unhandled promise rejection is coming from.

I had passed a few hundred songs to the method, and turns out I just had to chunk them in arrays of 50 to make it work.

JMPerez commented 6 years ago

You are definitely right. The wrapper doesn't contain validation logic for the arguments passed, and when we wrote it we preferred to let the API itself do the validation. This was done in case Spotify decided to change the values (eg the amount of tracks you can fetch at once) so the wrapper didn't need to be changed.

I agree it would be good to add some validation but it would also incur in some extra code that might not be worth it. I would like to get feedback from other users of the library to take a decision.

JMPerez commented 6 years ago

Related: https://github.com/thelinmichael/spotify-web-api-node/issues/136