From a0b4222fcbb512e434ae59d642ea8701e6b97ad4 Mon Sep 17 00:00:00 2001 From: Kartik Ohri Date: Sat, 19 Mar 2022 13:36:55 +0530 Subject: [PATCH] Do not remove spotify access token on invalid_grant error We used to remove spotify access tokens from our database whenever we detected token revocation at one point. But one day spotify had a downtime while resulted in false revocation errors, and we ended up deleting most of our users' spotify access tokens. Now we don't remove the token from database, this is more resilient and without downsides. If a user actually revoked their token, then its useless anyway so doesn't matter if we remove it. And if it is a false revocation error, we are saved! :) In any case, we do set an error message for the user in the database so that we can skip in future runs and notify them to reconnect if they want. --- listenbrainz/spotify_updater/spotify_read_listens.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/listenbrainz/spotify_updater/spotify_read_listens.py b/listenbrainz/spotify_updater/spotify_read_listens.py index d66e97a612..e02c5c39b6 100644 --- a/listenbrainz/spotify_updater/spotify_read_listens.py +++ b/listenbrainz/spotify_updater/spotify_read_listens.py @@ -336,8 +336,14 @@ def process_one_user(user: dict, service: SpotifyService) -> int: if not current_app.config['TESTING']: notify_error(user['musicbrainz_id'], error_message) # user has revoked authorization through spotify ui or deleted their spotify account etc. - # in any of these cases, we should delete the user's token from. - service.revoke_user(user['user_id']) + # + # we used to remove spotify access tokens from our database whenever we detected token revocation + # at one point. but one day spotify had a downtime while resulted in false revocation errors, and + # we ended up deleting most of our users' spotify access tokens. now we don't remove the token from + # database. this is actually more resilient and without downsides. if a user actually revoked their + # token, then its useless anyway so doesn't matter if we remove it. and if it is a false revocation + # error, we are saved! :) in any case, we do set an error message for the user in the database + # so that we can skip in future runs and notify them to reconnect if they want. raise ExternalServiceError("User has revoked spotify authorization") except ExternalServiceAPIError as e: