beets icon indicating copy to clipboard operation
beets copied to clipboard

Spotify: Improve error handling and retry logic

Open arsaboo opened this issue 9 months ago • 4 comments

I am pulling this conversation out of #4996 to discuss ways to improve the retry logic and error handling in the Spotify Plugin.

One of the problems is that Spotify does not return the retry-after interval most of the time. The current logic to handle this is to retry a few times and exit. However, improving the overall retry logic and error handling would be nice.

arsaboo avatar Nov 17 '23 17:11 arsaboo

Another error that I encountered today (no status_code). Dumping here for reference:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/urllib3/connection.py", line 169, in _new_conn
    conn = connection.create_connection(
  File "/usr/lib/python3/dist-packages/urllib3/util/connection.py", line 96, in create_connection
    raise err
  File "/usr/lib/python3/dist-packages/urllib3/util/connection.py", line 86, in create_connection
    sock.connect(sa)
OSError: [Errno 101] Network is unreachable

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 700, in urlopen
    httplib_response = self._make_request(
  File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 383, in _make_request
    self._validate_conn(conn)
  File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 1017, in _validate_conn
    conn.connect()
  File "/usr/lib/python3/dist-packages/urllib3/connection.py", line 353, in connect
    conn = self._new_conn()
  File "/usr/lib/python3/dist-packages/urllib3/connection.py", line 181, in _new_conn
    raise NewConnectionError(
urllib3.exceptions.NewConnectionError: <urllib3.connection.HTTPSConnection object at 0x7f451fb9b520>: Failed to establish a new connection: [Errno 101] Network is unreachable

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/arsaboo/.local/lib/python3.10/site-packages/requests/adapters.py", line 486, in send
    resp = conn.urlopen(
  File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 756, in urlopen
    retries = retries.increment(
  File "/usr/lib/python3/dist-packages/urllib3/util/retry.py", line 574, in increment
    raise MaxRetryError(_pool, url, error or ResponseError(cause))
urllib3.exceptions.MaxRetryError: HTTPSConnectionPool(host='api.spotify.com', port=443): Max retries exceeded with url: /v1/tracks/XYZ (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7f451fb9b520>: Failed to establish a new connection: [Errno 101] Network is unreachable'))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/arsaboo/.local/lib/python3.10/site-packages/beetsplug/spotify.py", line 176, in _handle_response
    response = request_type(
  File "/home/arsaboo/.local/lib/python3.10/site-packages/requests/api.py", line 73, in get
    return request("get", url, params=params, **kwargs)
  File "/home/arsaboo/.local/lib/python3.10/site-packages/requests/api.py", line 59, in request
    return session.request(method=method, url=url, **kwargs)
  File "/home/arsaboo/.local/lib/python3.10/site-packages/requests/sessions.py", line 589, in request
    resp = self.send(prep, **send_kwargs)
  File "/home/arsaboo/.local/lib/python3.10/site-packages/requests/sessions.py", line 703, in send
    r = adapter.send(request, **kwargs)
  File "/home/arsaboo/.local/lib/python3.10/site-packages/requests/adapters.py", line 519, in send
    raise ConnectionError(e, request=request)
requests.exceptions.ConnectionError: HTTPSConnectionPool(host='api.spotify.com', port=443): Max retries exceeded with url: /v1/tracks/XYZ (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7f451fb9b520>: Failed to establish a new connection: [Errno 101] Network is unreachable'))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/arsaboo/.local/bin/beet", line 8, in <module>
    sys.exit(main())
  File "/home/arsaboo/.local/lib/python3.10/site-packages/beets/ui/__init__.py", line 1865, in main
    _raw_main(args)
  File "/home/arsaboo/.local/lib/python3.10/site-packages/beets/ui/__init__.py", line 1852, in _raw_main
    subcommand.func(lib, suboptions, subargs)
  File "/home/arsaboo/.local/lib/python3.10/site-packages/beetsplug/spotify.py", line 485, in func
    self._fetch_info(items, ui.should_write(), opts.force_refetch)
  File "/home/arsaboo/.local/lib/python3.10/site-packages/beetsplug/spotify.py", line 664, in _fetch_info
    popularity, isrc, ean, upc = self.track_info(spotify_track_id)
  File "/home/arsaboo/.local/lib/python3.10/site-packages/beetsplug/spotify.py", line 685, in track_info
    track_data = self._handle_response(
  File "/home/arsaboo/.local/lib/python3.10/site-packages/beetsplug/spotify.py", line 188, in _handle_response
    if e.response.status_code == 401:
AttributeError: 'NoneType' object has no attribute 'status_code'

arsaboo avatar Dec 04 '23 15:12 arsaboo

Better late than never I'll add some ideas here on what comes to mind when thinking about error handling strategies with API's

  • Use a library that does that already.
    • With Discogs, beets relies on thepython3-discogs-client which uses a common strategy: "retry and exponential backoff". It was implemented by @alifhughes in this PR: https://github.com/joalla/discogs_client/pull/34
      • Even though Discogs has a very low rate-limit (https://www.discogs.com/developers#page:home,header:home-rate-limiting, which is even lower than Spotify and Deezer), since we implemented this change in the lib back in 2021 reports of rate-limiting finally stopped for good :-).
    • Relying on external libraries has its downsides (see musicbrainzngs not receiving updates discussions elsewhere FIXME), I'd like to suggest it anyway: spotipy as far as I know already includes a rate-limiting approach. I use it in other personal Spotify projects and up to now never had rate-limiting problems. I don't do mass-imports like we do with beets though -> Would be interesting to find out how spotipy does rate-limiting....
  • Here's an open PR regarding exactly aformentioned strategy but with the fetchart plugin and iTunes. It never got merged, we might want to finally look into it: https://github.com/beetbox/beets/pull/4639, and then could even take it as an example for Spotify.
  • I understand that probably the issues with the "unrelyability" of Spotify's API with returning retry-after might be a big issue for all these ideas. Again I 'm wondering: How does spotipy do it? Do they rely on it or have a fallback strategy of some sort?
  • @sampsyo suggests a similar idea from the requests docs here: https://github.com/beetbox/beets/pull/4943#issuecomment-1753597923, which involves using "requests sessions" which we currently don't do. The code in python3-discogs-client though, does not use "requests sessions", it might make sense to look into it and see if some of it could be used for an implementation directly in our Spotify plugin. We could also try to kindly ask @alifhughes for advice :-)
  • And finally, here's what Spotify officially recommends, which @arsaboo already followed and made use of some of those strategies in the beets plugin: https://developer.spotify.com/documentation/web-api/concepts/rate-limits

JOJ0 avatar Dec 11 '23 10:12 JOJ0

Spotipy has a well-developed retry logic (and a lot of other features) and is very well-maintained. I use it in my other plugin and have been very happy with it. I strongly feel that moving to the Spotipy library is a good idea and will serve us well in the long run (less code, maintenance, etc.). Otherwise, we will be reinventing the wheel here.

arsaboo avatar Dec 11 '23 14:12 arsaboo

Did some digging in the Spotipy docs again, I did not look at the code yet but the initialization options look promising. It sounds like a fancy backoff and retry strategy is in place: https://spotipy.readthedocs.io/en/2.22.1/?highlight=retry#module-spotipy.client

I also feel like that relying on this library would be a good direction.

JOJ0 avatar Dec 19 '23 09:12 JOJ0