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

fix: switch to librespot 0.5.0 #570

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

w-lfchen
Copy link

@w-lfchen w-lfchen commented Oct 8, 2024

disclaimer: i barely know what i'm doing since this is my first time working on this project.

there are some internal api changes which definitely need to be thoroughly reviewed as this is my first time contributing here.

i've tried to follow hrkfdn/ncspot#1244 where applicable.

closes #520
closes #579
closes #580

current state

basic functionality seems to work

this example works without issues after a minor modification:
for some reason, initializing the rutls crpyto provider is required.
adding rustls::crypto::aws_lc_rs::default_provider().install_default().unwrap(); fixes that

todos

it builds again, i am unsure whether i migrated everything correctly.
@w-lfchen
Copy link
Author

w-lfchen commented Oct 10, 2024

@ManiArasteh why not?

why would you not want to accept a pull request aiming to fix the issue you reported?

either provide a valid reason why or admit that you don't want to productively contribute to this project.
ignoring this question means the latter, i am tired of your behavior here and in #520.

@w-lfchen
Copy link
Author

why didn't you just write that instead then?

@w-lfchen
Copy link
Author

why is the commit hash in the lockfile not sufficient?

if we want to update the dependency, we manually have to do so.
after updating dependencies, one should always at least test compilation

@w-lfchen
Copy link
Author

see #520 (comment) for my response, i don't feel like typing out the same thing twice.

@ManiArasteh your behavior is killing my motivation to continue working on this pr, please stop.
i have already spent a lot of time working on this, time i could've used for literally anything else and i expect nothing but human decency and a somewhat productive environment in return, so please work towards clearing this extremely low bar, thank you.

@w-lfchen
Copy link
Author

w-lfchen commented Oct 13, 2024

that's appreciated but not an excuse for your behavior.

@stanekondrej
Copy link

Hey @w-lfchen, I would love to contribute to this PR. If you want/need any help, could you please provide me with some pointers about what needs to be done? I've read #520 and got to this PR from your comment. I hope to be able to be of help.

@w-lfchen
Copy link
Author

@stanekondrej thank you so much!
currently, we seem to be able to log in and generate a token, but the function Spirc::new fails when called here.

this is a problem on our side, the example application in librespot's repo that uses a Spirc works, see the "current state" section in the initial report for more information and a link; i'll update that section if i find something.

@aome510
Copy link
Owner

aome510 commented Oct 14, 2024

Thank you guys for the PR and appreciate the effort for looking into this 🙇. I feel this kind of large-scale upgrade will be better for me to do cause it requires a certain level of familiarity with the codebase. I couldn't find time working on the project nowadays but this is definitely my top priority when I get back!

@stanekondrej
Copy link

@stanekondrej thank you so much!
currently, we seem to be able to log in and generate a token, but the function Spirc::new fails when called here.

this is a problem on our side, the example application in librespot's repo that uses a Spirc works, see the "current state" section in the initial report for more information and a link; i'll update that section if i find something.

I see, I've taken a quick look at the code you linked me to last night, but as it was late for me, I didn't really have time to investigate further. I'll take a closer look at it in the evening as I'm at school at the moment.

As for the comment by the author, if I understand it correctly, you mean that it would be better if you made the changes to the codebase since you have a better understanding of it. I completely get that, but I'd love to help with getting the respective changes implemented.

Thanks in advance

@aome510
Copy link
Owner

aome510 commented Oct 15, 2024

As for the comment by the author, if I understand it correctly, you mean that it would be better if you made the changes to the codebase since you have a better understanding of it. I completely get that, but I'd love to help with getting the respective changes implemented.

Yeah, your understanding is correct. Feel free to take a look and implement the changes. Contributions are always welcomed and highly appreciated!!

@w-lfchen w-lfchen changed the title fix: switch to librespot 0.5.0 dev branch fix: switch to librespot 0.5.0 Oct 16, 2024
Copy link
Author

@w-lfchen w-lfchen left a comment

Choose a reason for hiding this comment

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

i've updated the librespot dependencies to 0.5 since it's been released now in case someone is using this draft to implement the new login flow

@juliamertz
Copy link
Contributor

Hey @w-lfchen, i've managed to get your branch working by passing Session::new(SessionConfig::default(), None) to Spirc::new instead of the shared client session. Everything seems to be working well for me, though I'm not sure what the implications of this change are.

I have tried finding the root cause of this problem, but no luck so far

@w-lfchen
Copy link
Author

@juliamertz thank you!
i'm not sure either, but the application seems to work now, i've pushed a commit implementing your suggestion.
i'll do some cleanup and more tests and then ask for testers in the issue to see how stable this looks ^^

@w-lfchen
Copy link
Author

w-lfchen commented Oct 16, 2024

removed a premature commit i pushed.
it lead to scope issues when reauthenticating which i didn't fully test.

@tstenner
Copy link

I had to move the crypto provider initialization to the main() function, otherwise spotify_player search and spotify_player authenticate would abort.
Other than I didn't notice any regressions.

juliamertz and others added 2 commits October 17, 2024 16:23
* fix: skip Box -> Arc conversion

* fix: initialize crypto provider in main
@apprehensions
Copy link
Contributor

Restarting the client with this PR in use fails.

2024-10-21T16:18:26.905281Z  INFO client_request{request=RestartIntegratedClient}: spotify_player::streaming: Initializing a new integrated player with device_id=4e1c84e5-5c8d-43d5-886b-f35668191ea1
2024-10-21T16:18:26.905443Z  INFO client_request{request=RestartIntegratedClient}: spotify_player::streaming: Starting an integrated Spotify player using librespot's spirc protocol
2024-10-21T16:18:28.111510Z  INFO client_request{request=RestartIntegratedClient}: spotify_player::client: No playback found, trying to connect to an available device...
2024-10-21T16:18:28.111590Z  INFO client_request{request=RestartIntegratedClient}: spotify_player::client: Successfully handled the client request, took: 2152ms
2024-10-21T16:18:29.332896Z  WARN spotify_player::client: No device found. Please make sure you already setup Spotify Connect support as described in https://github.com/aome510/spotify-player#spotify-connect.
2024-10-21T16:18:29.332961Z  INFO spotify_player::client: Trying to connect to device (id=4e1c84e5-5c8d-43d5-886b-f35668191ea1)
2024-10-21T16:18:29.736301Z  WARN spotify_player::client: Connection failed (device_id=4e1c84e5-5c8d-43d5-886b-f35668191ea1): http error: status code 500 Internal Server Error
2024-10-21T16:18:30.952265Z  WARN spotify_player::client: No device found. Please make sure you already setup Spotify Connect support as described in https://github.com/aome510/spotify-player#spotify-connect.
2024-10-21T16:18:30.952332Z  INFO spotify_player::client: Trying to connect to device (id=4e1c84e5-5c8d-43d5-886b-f35668191ea1)
2024-10-21T16:18:31.443451Z  WARN spotify_player::client: Connection failed (device_id=4e1c84e5-5c8d-43d5-886b-f35668191ea1): http error: status code 404 Not Found
2024-10-21T16:18:32.660579Z  WARN spotify_player::client: No device found. Please make sure you already setup Spotify Connect support as described in https://github.com/aome510/spotify-player#spotify-connect.
2024-10-21T16:18:32.660653Z  INFO spotify_player::client: Trying to connect to device (id=4e1c84e5-5c8d-43d5-886b-f35668191ea1)
2024-10-21T16:18:33.152731Z  WARN spotify_player::client: Connection failed (device_id=4e1c84e5-5c8d-43d5-886b-f35668191ea1): http error: status code 404 Not Found

@Kek5chen
Copy link

Kek5chen commented Oct 22, 2024

@ManiArasteh its honestly exhausting trying to follow the issue and the PR with your nonsense around. I would like to ask you to please be quiet and stop saying unnecessary things that nobody has any use for. Thanks for opening the issue but if you have no more value to provide, please just let people do their work and be nothing more than grateful. Thanks everyone else for your contributions to OSS.

Blocked.

@stanekondrej
Copy link

@ManiArasteh Your comment isn't necessarily off-topic, but it provides zero value to the discussion and development. You're just stating the obvious.

@w-lfchen
Copy link
Author

w-lfchen commented Oct 22, 2024

Is that off-topic?

yes, nobody asked for your comment.

Look, I have a Contributor badge

98d8123 is your only contribution. it is also made obsolete by this pr. i'll leave any judgement of these stated facts up to the person reading this.

Your brain is always saying dislike everything from Mani Arasteh.

if this is the conclusion you came to after seeing everyone's reactions to your behavior here where nobody said they hate you as a person, everyone just disliking your behavior, then you should really do some introspection.

@ManiArasteh this is my last warning to you.
i don't want to block people on a platform for working together, the idea alone seems ridiculous to me.
if i block you, you'll no longer be able to contribute to this pr according to these docs which would be sad since you said you were testing this pr (which never seemed to yield any results).
however, your behavior is disruptive, draining of both time and energy, and not productive in any way. i feel like i've been lenient enough by asking you to change your attitude/behavior multiple times, but you never listened.

if i see one more unproductive comment from you, such as a defensive reply to this comment, i will block you.

i hope you have a wonderful day and feel free to report back when you have anything of value to say. if not, stay quiet.
i don't want to have to issue this warning but i am too exhausted to put up with your behavior any longer.

@w-lfchen
Copy link
Author

w-lfchen commented Oct 22, 2024

i blocked them, hopefully we can now work on this in peace.
i apologize for not doing so earlier.

since they blocked me as well, i am no longer able to comment on #520 so please keep that in mind in case that becomes relevant.

@juliamertz
Copy link
Contributor

Restarting the client with this PR in use fails.

What OS are you using, and have you tried clearing your cache?

@apprehensions
Copy link
Contributor

apprehensions commented Oct 22, 2024

I feel like it is a security concern for the security token to be spat out in the logs.

2024-10-22T15:58:29.632356Z  INFO client_request{request=GetContext(Playlist(PlaylistId("2bQwC7mMG3f5UEjYuV5aoI")))}: spotify_player::client: Successfully handled the client request, took: 825ms
2024-10-22T17:50:55.083795Z  INFO client_request{request=Player(ResumePause)}: spotify_player::token: Getting new authentication token...
2024-10-22T17:50:55.234606Z  INFO client_request{request=Player(ResumePause)}: spotify_player::token: Got new token: Token { access_token: "no", expires_in: TimeDelta { secs: 3600, nanos: 0 }, expires_at: Some(noZ), refresh_token: None, scopes: {} }
2024-10-22T17:50:56.065487Z ERROR client_request{request=Player(ResumePause)}: spotify_player::client::handlers: Failed to handle client request: http error: status code 404 Not Found
2024-10-22T17:50:56.955014Z ERROR client_request{request=Player(Volume(75))}: spotify_player::client::handlers: Failed to handle client request: http error: status code 404 Not Found
2024-10-22T17:50:58.646625Z ERROR client_request{request=Player(StartPlayback(Context(Playlist(PlaylistId("2bQwC7mMG3f5UEjYuV5aoI")), Some(Uri("spotify:track:61IaHEbPKZj3I4APmkZSJd"))), None))}: spotify_player::client::handlers: Failed to handle client request: http error: status code 404 Not Found
2024-10-22T17:50:59.821922Z ERROR client_request{request=Player(StartPlayback(Context(Playlist(PlaylistId("2bQwC7mMG3f5UEjYuV5aoI")), Some(Uri("spotify:track:0Op3S08ZQiY7wpm4k9CVcZ"))), None))}: spotify_player::client::handlers: Failed to handle client request: http error: status code 404 Not Found
2024-10-22T17:51:18.645203Z ERROR client_request{request=Player(StartPlayback(Context(Playlist(PlaylistId("2bQwC7mMG3f5UEjYuV5aoI")), Some(Uri("spotify:track:0Op3S08ZQiY7wpm4k9CVcZ"))), None))}: spotify_player::client::handlers: Failed to handle client request: http error: status code 404 Not Found
2024-10-22T17:51:19.596061Z ERROR client_request{request=Player(StartPlayback(Context(Playlist(PlaylistId("2bQwC7mMG3f5UEjYuV5aoI")), Some(Uri("spotify:track:1QkzZUYcm9IzSsEY1HieDS"))), None))}: spotify_player::client::handlers: Failed to handle client request: http error: status code 404 Not Found
2024-10-22T17:51:20.612873Z ERROR client_request{request=Player(StartPlayback(Context(Playlist(PlaylistId("2bQwC7mMG3f5UEjYuV5aoI")), Some(Uri("spotify:track:4zwQYI1yLuUTR6AspMcKBM"))), None))}: spotify_player::client::handlers: Failed to handle client request: http error: status code 404 Not Found

What OS are you using, and have you tried clearing your cache?

Alpine Linux edge, and no.

I've cleared cache. Simply restarting the client by pressing R causes the issue.

2024-10-22T17:53:43.094085Z  INFO client_request{request=RestartIntegratedClient}: spotify_player::streaming: Application's connect configurations: ConnectConfig { name: "耳栓", device_type: Computer, is_group: false, initial_volume: Some(52428), has_volume_ctrl: true }
2024-10-22T17:53:43.094150Z  INFO client_request{request=RestartIntegratedClient}: spotify_player::streaming: Initializing a new integrated player with device_id=95374bca-3496-41bc-9485-343342592c55
2024-10-22T17:53:43.094336Z  INFO client_request{request=RestartIntegratedClient}: spotify_player::streaming: Starting an integrated Spotify player using librespot's spirc protocol
2024-10-22T17:53:43.605323Z  INFO client_request{request=GetContext(Playlist(PlaylistId("2bQwC7mMG3f5UEjYuV5aoI")))}: spotify_player::client: Get playlist context: spotify:playlist:2bQwC7mMG3f5UEjYuV5aoI
2024-10-22T17:53:43.933315Z  INFO client_request{request=GetContext(Playlist(PlaylistId("2bQwC7mMG3f5UEjYuV5aoI")))}: spotify_player::client: Successfully handled the client request, took: 327ms
2024-10-22T17:53:44.269469Z  INFO client_request{request=RestartIntegratedClient}: spotify_player::client: No playback found, trying to connect to an available device...
2024-10-22T17:53:44.269550Z  INFO client_request{request=RestartIntegratedClient}: spotify_player::client: Successfully handled the client request, took: 2151ms
2024-10-22T17:53:45.483826Z  WARN spotify_player::client: No device found. Please make sure you already setup Spotify Connect support as described in https://github.com/aome510/spotify-player#spotify-connect.
2024-10-22T17:53:45.483906Z  INFO spotify_player::client: Trying to connect to device (id=95374bca-3496-41bc-9485-343342592c55)
2024-10-22T17:53:45.839659Z ERROR client_request{request=Player(StartPlayback(Context(Playlist(PlaylistId("2bQwC7mMG3f5UEjYuV5aoI")), Some(Uri("spotify:track:0IxhlitmfOIesNDJYaNRrZ"))), None))}: spotify_player::client::handlers: Failed to handle client request: http error: status code 404 Not Found
2024-10-22T17:53:46.097105Z  WARN spotify_player::client: Connection failed (device_id=95374bca-3496-41bc-9485-343342592c55): http error: status code 500 Internal Server Error
2024-10-22T17:53:47.316336Z  WARN spotify_player::client: No device found. Please make sure you already setup Spotify Connect support as described in https://github.com/aome510/spotify-player#spotify-connect.
2024-10-22T17:53:47.316428Z  INFO spotify_player::client: Trying to connect to device (id=95374bca-3496-41bc-9485-343342592c55)
2024-10-22T17:53:47.803403Z  WARN spotify_player::client: Connection failed (device_id=95374bca-3496-41bc-9485-343342592c55): http error: status code 404 Not Found
2024-10-22T17:53:48.060431Z ERROR client_request{request=Player(StartPlayback(Context(Playlist(PlaylistId("2bQwC7mMG3f5UEjYuV5aoI")), Some(Uri("spotify:track:0IxhlitmfOIesNDJYaNRrZ"))), None))}: spotify_player::client::handlers: Failed to handle client request: http error: status code 404 Not Found
2024-10-22T17:53:48.805075Z ERROR client_request{request=Player(StartPlayback(Context(Playlist(PlaylistId("2bQwC7mMG3f5UEjYuV5aoI")), Some(Uri("spotify:track:0IxhlitmfOIesNDJYaNRrZ"))), None))}: spotify_player::client::handlers: Failed to handle client request: http error: status code 404 Not Found
2024-10-22T17:53:49.020411Z  WARN spotify_player::client: No device found. Please make sure you already setup Spotify Connect support as described in https://github.com/aome510/spotify-player#spotify-connect.
2024-10-22T17:53:49.020473Z  INFO spotify_player::client: Trying to connect to device (id=95374bca-3496-41bc-9485-343342592c55)
2024-10-22T17:53:49.528061Z  WARN spotify_player::client: Connection failed (device_id=95374bca-3496-41bc-9485-343342592c55): http error: status code 404 Not Found
2024-10-22T17:53:50.741014Z  WARN spotify_player::client: No device found. Please make sure you already setup Spotify Connect support as described in https://github.com/aome510/spotify-player#spotify-connect.
2024-10-22T17:53:50.741138Z  INFO spotify_player::client: Trying to connect to device (id=95374bca-3496-41bc-9485-343342592c55)

@aome510
Copy link
Owner

aome510 commented Oct 22, 2024

I might have sometime this weekend and will start looking into the remaining items. Please keep me posted on the progress or if anyone is working on those already.

Hopefully, we can push a new release by this week 🤞🤞.

@juliamertz
Copy link
Contributor

I've cleared cache. Simply restarting the client by pressing R causes the issue.

Ah okay it seems like i misunderstood, I'm unable to reproduce this on my system though 😕.

@apprehensions
Copy link
Contributor

Our review of the account named in your report has concluded. We have determined that one or more violations of GitHub’s Terms of Service have occurred and have taken appropriate action in response.

Good riddance.

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.

[bug] Authentication Issues Spotify session randomly invalidated
7 participants