Fix Spotify metadata provider: send token via Authorization header - #468
Open
nicobgr wants to merge 1 commit into
Open
Fix Spotify metadata provider: send token via Authorization header#468nicobgr wants to merge 1 commit into
nicobgr wants to merge 1 commit into
Conversation
Spotify's Web API no longer accepts access_token as a query parameter on /v1/search - it must be sent as an "Authorization: Bearer" header. Requests made the old way get rejected with an error response that handle_json_result_error doesn't expect, crashing with an unhandled TypeError on every single lookup. - Override get_image_track/artist/album in the Spotify provider to send the token via the Authorization header instead of the URL, and make handle_json_result_error defensive against non-dict / error-shaped responses instead of crashing. - Log the real exception (repr(e)) instead of e.__doc__ in thirdparty/__init__.py's get_image_*_all functions - e.__doc__ is just the exception class's generic docstring (e.g. "Inappropriate argument type." for any TypeError), which made this bug and others like it (krateng#424) effectively undiagnosable from the logs. Fixes krateng#467
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #467
Problem
Spotify's Web API no longer accepts
access_tokenas a query parameter on/v1/search- it must be sent as anAuthorization: Bearer <token>header instead. Requests built the old way get rejected with an error-shaped JSON response thathandle_json_result_errordoesn't expect (notracks/albums/artistskey), which crashes with an unhandledTypeError.Since
thirdparty/__init__.py'sget_image_*_allfunctions loge.__doc__(the exception class's docstring) instead of the actual error, this surfaces in the logs as the generic, undiagnosable:...on every single lookup (verified: 0/163 successful Spotify lookups in a real instance's logs before this fix). It silently falls through to the next provider, so it's easy to miss - the only symptom is worse-than-expected cover art coverage.
I believe this also explains #424 ("Mapping key not found" =
KeyError.__doc__, same class of problem, different exception).Fix
maloja/thirdparty/spotify.py: overrideget_image_track/get_image_artist/get_image_albumto send the token via theAuthorizationheader instead of the URL, dropaccess_token={token}from the URL templates, and makehandle_json_result_errordefensive against non-dict / error-shaped responses instead of crashing on them.maloja/thirdparty/__init__.py: logrepr(e)instead ofe.__doc__in the threeget_image_*_allfunctions, so future failures (for any provider, not just Spotify) are actually diagnosable from the logs instead of always showing the same generic exception-class docstring.Testing
Verified against a real Maloja instance (v3.2.6) with valid Spotify credentials:
TypeErrormessage.i.scdn.coimage URLs returned for artist/album/track lookups that previously failed, including via real scrobbles from a live client (not just synthetic tests), e.g.[thirdparty] Got album image for (['Sia'], '1000 Forms Of Fear') from Spotify.