Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spotify: Improve error handling and retry logic #4997

Open
arsaboo opened this issue Nov 17, 2023 · 4 comments
Open

Spotify: Improve error handling and retry logic #4997

arsaboo opened this issue Nov 17, 2023 · 4 comments
Labels

Comments

@arsaboo
Copy link
Contributor

arsaboo commented Nov 17, 2023

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
Copy link
Contributor Author

arsaboo commented Dec 4, 2023

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
Copy link
Member

JOJ0 commented Dec 11, 2023

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: Back off function for when rate limit is hit joalla/discogs_client#34
    • 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: [fetchart] Add retry with exponential backoff #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: Gracefully handle Spotify API errors #4943 (comment), 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

@arsaboo
Copy link
Contributor Author

arsaboo commented Dec 11, 2023

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
Copy link
Member

JOJ0 commented Dec 19, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants