-
Notifications
You must be signed in to change notification settings - Fork 44
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 source has issues if service is ran for an extended duration #211
Comments
I haven't been able to properly debug it either, but it's also happening on our deploy: thread 'tokio-runtime-worker' panicked at 'failed to create response: Serenity(Http(UnsuccessfulRequest(ErrorResponse { status_code: 404, url: Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("discord.com")), port: None, path: "/api/v9/interactions/1036673658422313000/aW50ZXJhY3Rpb246MTAzNjY3MzY1ODQyMjMxMzAwMDpIZTM2N3A3QWkzazZESmdHOHNBdXZ5eGRTcVlQMTlEZ21QYjFQNlpHbzE1dENvUnZjckt4S3RidmxiYlNsOVFLNmJuUm1pVU5RSU1DTmxEOFBTWVNFYWRTVVEyRjJwSFIxUDJxcVEwWlIyWFVGNmtIWEkyejNOZTFGY1U1VGhHYw/callback", query: None, fragment: None }, error: DiscordJsonError { code: 10062, message: "Unknown interaction", errors: [] } })))', src/handlers/serenity.rs:462:18 |
@aquelemiguel , that looks like a serenity error. From the error it looks like it may be trying to edit an interaction that doesn't exist. I wasn't getting the same error when I initially submitted this issue, but that doesn't mean it's not related... |
@aquelemiguel However, the deployed version is 1.4.2, which is a bit far from what we currently have. Let's release 1.4.3 soon and recheck. |
This issue still occurs on 1.5.0, but perhaps the latest rspotify version has fixed it on main. |
Still happens to me on 1.5.1, the error starts when the bot has been running for about an hour. |
I've maybe managed to find the issue, like i said before it probably has to do with the token expiring after a while (about 1hour for me). Basically i simply added a new line of code to request a new token after this line: Line 87 in b2c5ad7
+ spotify.request_token().await?;
I'm aware that that's no were near an efficient way of fixing the problem because it would request a new token each time you call the play command containing a spotify url but i have really little knowledge of Rust and im not capable of implementing a proper fix for now. I'll keep you updated in the following days to let you know if the issue reoccurs or if it's fixed. |
@blimp4242 Actually, maybe that's actually what we're supposed to do! I'll do some investigating as well! |
@blimp4242 Yep, it seems that this is actually expected as Spotify Access Tokens do have an expiration time and will eventually become invalid. The exact expiration time of the Token is specified in the "expires_in" field of the token response. But typically, the lifetime of a Spotify Token is one hour. So yeah, your quick fix is actually THE fix. Feel free to open the PR yourself, if not I can do it. |
But we can't request a new token upon every new spotify search request. Instead, we need to wrap the get of the spotify object with a getter that provides the lock to it but also attempts some dummy operation to check if it is a valid token and then if it fails with an error indicating it expired, requests the new token and returns. |
Or you can just save when it will expire and request a new one on a new request if it has already expired. |
@joao-conde Right, that's why i said it's a jenky implementation, do you want me to PR anyway? |
Hi @blimp4242, firstly thank you for reporting and for the possible fix! ✌️ Regarding the PR, would you be willing to implement the fix as @joao-conde mentioned? |
Thanks @aquelemiguel, im glad to help. |
📝 Description
The Spotify source has issues looking up queries after the service has been running for a few days. I believe this is related to the auth routine but I haven't had time to properly debug this yet.
This may straight up be a byproduct of me running this on a machine without ECC memory, but it seems a little too consistent for that.
🪜 Reproduction Steps
failed to fetch track
ℹ Environment / Computer Info
main
archlinux
📸 Screenshots
No response
The text was updated successfully, but these errors were encountered: