beetbox / beets

music library manager and MusicBrainz tagger
http://beets.io/
MIT License
12.84k stars 1.82k forks source link

Spotify: Improve error handling and retry logic #4997

Open arsaboo opened 11 months ago

arsaboo commented 11 months ago

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 commented 10 months ago

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'
JOJ0 commented 10 months ago

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

arsaboo commented 10 months ago

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.

JOJ0 commented 10 months ago

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.