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

Fixed rate limiting issue #852

Merged
merged 1 commit into from
Aug 23, 2021

Conversation

alejandro-angulo
Copy link
Contributor

This change should resolve the issue with users being rate limited when leaving spotify-tui running without anything playing. This should solve #779 .

Comment on lines +366 to +369
Ok(None) => {
let mut app = self.app.lock().await;
app.instant_since_last_current_playback_poll = Instant::now();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I set up logging in a local build and noticed that we weren't respecting the 5s interval when making requests if spotify returned a 204 (no content). This Ok(None) pattern matches the 204 response case.

[2021-08-02T13:36:30Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:30Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:31Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:31Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:32Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:32Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:33Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:33Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:34Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:34Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:35Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:35Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:36Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:36Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:37Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:37Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:38Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:38Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:39Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:39Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:40Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&
[2021-08-02T13:36:40Z DEBUG reqwest::async_impl::client] response '204 No Content' for https://api.spotify.com/v1/me/player?additional_types=episode,track&

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@alejandro-angulo alejandro-angulo marked this pull request as ready for review August 11, 2021 05:16
Copy link
Owner

@Rigellute Rigellute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @alejandro-angulo. Thanks for looking into this and fixing.

Comment on lines +366 to +369
Ok(None) => {
let mut app = self.app.lock().await;
app.instant_since_last_current_playback_poll = Instant::now();
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@Rigellute
Copy link
Owner

Blast, looks like github actions is using a new version of clippy. Which has brought some new errors. I'll aim to fix, and then you can rebase master on your branch.

@Rigellute
Copy link
Owner

I've fixed clippy and can merge this now. Looks like your changes pass the new clippy standards, so should be fine to merge.

@Rigellute Rigellute merged commit 100feac into Rigellute:master Aug 23, 2021
@Rigellute
Copy link
Owner

@all-contributors please add @alejandro-angulo for code

@allcontributors
Copy link
Contributor

@Rigellute

I've put up a pull request to add @alejandro-angulo! 🎉

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

Successfully merging this pull request may close these issues.

None yet

2 participants