Fix missing / unavailable tracks in search results #131
Conversation
mopidy_spotify/search.py
Outdated
|
|
||
| try: | ||
| params['market'] = session.user_country | ||
| except ValueError as exc: |
There was a problem hiding this comment.
I don't really know, but remember, this is libspotify....
There was a problem hiding this comment.
I guess if we were really worried about libspotify returning rubbish back to us we should handle it in pyspotify. I can remove this (and it's test).
There was a problem hiding this comment.
Hm, in libspotify it is an int and pyspotify doesn't raise any errors no matter what the value is, so I don't see how anything could happen. If the int is 0, you will get null-bytes, but that shouldn't trigger a ValueError.
Edit: Posted this before I saw your last answer. In response to that: Well, if pyspotify raised a ValueError on invalid values from libspotify this would make sense, but as far as I can see, it doesn't do that.
There was a problem hiding this comment.
If that returned int is greater than 65535, then utils.to_country will raise ValueError when it tries to convert a number greater than 255 to chr. That routine could do with another & 0xff in there.
kingosticks
force-pushed
the
fix/search-tracks-available
branch
3 times, most recently
from
84f63ea to
e7247a6
Compare
|
Squashed and I've also removed the exception handling from this PR. It'd only be needed if libspotify started misbehaving (which we have zero reports of), in which case it should be handled in pyspotify rather than here. |
|
Is this necessary now when all requests are authenticated? Couldn't Spotify know from your credentials which market you're in? |
|
You'd hope so, but not quite. From https://developer.spotify.com/web-api/search-item/:
If you don't specify anything, you used to default to US but I'm not sure if that's still true. |
So yeah, we do need the scope even if we never directly look it up. |
|
And @kingosticks needs to update this PR to apply to develop again :-) |
|
Well that didn't work |
kingosticks
force-pushed
the
fix/search-tracks-available
branch
from
eca665e to
9743231
Compare
kingosticks
force-pushed
the
fix/search-tracks-available
branch
from
9743231 to
8eb583e
Compare
|
That's better! |
This fixes #97 by setting the market parameter when doing searches using the Web API.
We don't currently set the parameter and it defaults to 'US' (spotify/web-api#194). Depending on the logged in user's country, this may result in missing or unavailable tracks being returned. For example "The black box revelation" as reported in #106.
I appreciate that considering #130, this might not see the light of day, but at least this should remind us to set it as part of those future changes.