| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 695
Handle reconnection for Sessions #609
Replies: 16 comments · 26 replies
-
|
So here's my brain dump about the whole reconnection thing. Currently one The alternative is to move to a model where a impl Session {
fn new(config: SessionConfig, cache: Option<Cache>, handle: Handle) -> Session { ... }
fn connect(&self, credentials: Credentials) -> impl Future<Credentials, ConnectionError> { ... }
}The Trying to use an unconnected session (to get metadata, audio key/data, etc...) should fail gracefully. All the APIs already use Future/Stream, so these just need to resolve into errors. Same happens if the connection is lost while a request is in flight. Player and Spirc need to behave correctly when those requests fail. For Player it just means returning to the NotPlaying state and signal the caller that playback has completed with an error. For Spirc, when Player signals a connection problem it should probably stop playback. Reading from or sending to the mercury channel may start failing as well. Now to get back up, I suggest the Session signals in some way to This can mean various things depending on the component. The components need to expose an API that looks like: impl Component {
fn connection_established(&self, username: String) { ... }
fn dispatch(&self, cmd: u8, mut data: Bytes) { ... }
fn connection_closed(&self) { ... }
}There are some risks of race conditions that need to be carefully though about. I think I have some initial implementation of this somewhere, I'll try and find it again. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
@plietar did you find that initial implementation anywhere? |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Regardless, I think we should be aiming to make this as simple to integrate as possible, as handling reconnection logic from whatever project uses librespot sounds like plenty of headache for anyone who tries to use librespot, whilst also being pretty low level functionality that I personally would expect to be handled in any library I used. Anyway, these are just my two cents since I'm not sufficently versed in rust to be able to rewrite how sessions are handled. Would be good to hear others thoughts as to how this should be handled. |
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 2
-
|
I am trying to wrap my head around this issue. AFAICS, the disconnect error starts at Since
So this works for the binary case. For the library case @plietar suggests to let main reconnect using last credentials and let Spirc, Player, etc. pick up the new session and reset its state accordingly. I have two problems trying to implement this
If you look at examples/play.rs you can do a match on |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Is there any way to mitigate this problem? |
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 2
-
This doesn't address the cause but aids with the symptoms: Script to be run as a cronjob every minute which checks the status and restarts the service if an error is spotted. |
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 7
-
|
Is the cronjob patch the best solution for this issue currently? Sometimes I'm 25 songs in, sometimes I'm 4 when I lose connection. This is running on a server connected directly to my modem/router via ethernet. I can't imagine it's actually losing internet connection this consistently. |
Beta Was this translation helpful? Give feedback.
All reactions
-
😕 2
-
Sounds like this is closely related to, and would probably help solve #266 |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Let's solve this as part of the new api. Mercury will not be central anymore for the session. There will be the http api and the websocket. I implemented the websocket already (#753), and considered reconnection from the start. There is the The websocket works almost independent from the rest of the session. Only we need to request a token from mercury to establish a connection. I solved this as follows: One must pass an "async" closure to The closure holds a weak reference to the session. What happens if Mercury is down as well or the session object is gone? The closure shouldn't return anything but have it's own little reconnection logic and try it periodically again. The This wasn't so hard, since the websocket receives only things and isn't used to send requests. There's no one else who has to care if the session is down. Nevertheless, I think a similar approach could work for Mercury. First of all, we should split the Mercury connection into another struct which is parallel to Then, we create a background task to handle reconnection like in If a request is done, it should be possible to pass an optional timeout, and it should fail with a Audio channels just won't yield anything, and maybe it's possible to make it look exactly as if it's lagging because of slow internet connection. In this case, no big changes would be required for the player. For every other case it has to be investigated how to handle a Your feedback? |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
This sounds sensible. W.R.T how audio playback could be handled in the event of a reconnection being in progress, I believe the 'yield nothing' approach is what is used by the official clients - the controls and UI all indicate that playback is occurring, except the progress bar does not move and no sound comes out. Once the connection is reestablished the audio resumes playing and the progress bar continues. I believe we already implement a microcosm of this when a track is loaded - the player is set to the loading state, then once loaded the progress bar is set to the current playback position and everything continues normally. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Something else worth considering is whether we actually need mercury at all. It's mainly the connect functionality where this question remains, but from what I can see the apps have almost fully switched to http/grpc |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Are you sure regarding grpc? I didn't see any signs of it in librespot-java. Which address? How does authentication work? |
Beta Was this translation helpful? Give feedback.
All reactions
-
There aren't that many grpc endpoints, but if one looks at the http traffic, a number of them respond with a content type of grpc, and the proto definitions have grpc annotations |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
So I assume there's still the usual authentication necessary, and then a token is used for the http api? |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
@Johannesd3 having started work on this now I realize the same holds not only for Mercury but also What do you think would be the best approach to copy this behavior to Mercury and |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
I wasn't able to read every message the last week, and I don't quite understand what you mean by "the same".
Yes, you're right. I already had it in mind when I wrote it.
I'm not sure, after all they differ very much. Reconnecting was easy in
I'd assume that The old "mercury" session is another story. What happens to subscriptions, what happens to pending requests on reconnection? On the other hand, iirc librespot-java doesn't even use subscriptions anymore. Do the official Spotify clients still need mercury subscriptions, @devgianlu @sashahilton00? It would make things a lot easier if we can remove the subscription functionality. If you still think it's possible to generalize approaches, I would try to avoid a macro based solution. I'm always afraid of wrapping large code parts into a macro. Traits make better use of type-safety and are easier to understand for IDEs. |
Beta Was this translation helpful? Give feedback.
All reactions
-
Most of the subscriptions can be done with the dealer, the last time I was able to capture Hermes, there ware subscriptions only for UI stuff like presence. |
Beta Was this translation helpful? Give feedback.
All reactions
-
I did speak/write too soon, you're right it's not the same at all in that sense. Yes,
If so then the same would apply for |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
I want to add here a use case which makes these silent disconnects very frequent: laptops with docks. Undocking my laptop will continue playing until the current song ends, then go silent. It seems like it dies at a point at which it would have downloaded something, maybe? Where the buffer ends, basically. The client should be able to survive network cards going missing (while another is still there, like a wifi), imo. If it does that, we'd be golden on all possible fronts. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Since this feature request is still open, I'm adding another use case. I use power over ethernet for my raspberry pi. Sometimes the connection drops for a second and it crashes raspotify. As the track data is cached for a few seconds, it could keep playing whatever is cached and attempt a reconnect in the background using an exponential backoff. Here is the error log I'm seeing when the connection drops: |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Looks like I have a very similar issue: Spotifyd/spotifyd#1211 |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
The connection is being dropped on Spotify's side. It doesn't really have anything to do with the quality of your connection. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
It seems to me that this error mostly happens on raspberry pi and it never occurred to me on desktop. How could that be if it is dropped on their side? |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
It's a pretty vague error, but that's what a 104 means. It could be for who knows what reason but Spotify is the "peer" and they reset the connection. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Until more gets done I wrote a better monitoring script to restart librespot, if anyone wants it. I need it due to using Starlink, which loses connection for a moment at least once every hour. It will actively follow librespot's journal and restart it if core::session throws an error (ignoring other common errors that don't break it, like 502's from ::spirc) /root/bin/monitor-librespot.sh: #!/bin/bash
last_restart=$(</proc/uptime)
last_restart=${last_restart%%.*}
echo "Starting librespot monitoring at uptime: $last_restart"
journalctl -b -f -o cat _SYSTEMD_UNIT=librespot.service --since now | stdbuf -o0 awk '$2 == "ERROR" && $3 == "librespot_core::session]" { print }' | while read -r line; do
now=$(</proc/uptime)
now=${now%%.*}
echo "$line"
if [ "$now" -gt $((last_restart + 5)) ]; then
last_restart="$now"
echo "Error found! Restarting librespot service at uptime: $now"
systemctl restart librespot.service
fi
done/etc/systemd/system/monitor-librespot.service: Once created, run |
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
|
looks like it is indeed is! seems last time i compiled was a few days before that went in, and that commit appears to be working well! |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
If you feel like being adventurous I could use some testing of this PR? |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
after some longer testing, this script is still useful as it appears the 4d402e6 commit does not always restart spirc, see log: |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
You don't need to restart librespot on a 104. You would just reconnect manually. It would be nice if that were automatic but you're script isn't doing anything useful either in that case. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
I wish this were the case but it seems when I receive those errors, the librespot instance disappears from the windows Spotify desktop app's streaming target list and restarting librespot is the only thing that makes it show up again. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Is there any work being done to add some session recovery to 4d402e6? |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
I’m not working on it but can answer some of your questions. The buffer is currently a on-disk write-through so practically unlimited in capacity. The issue is probably more that when the Spirc thread detects a disconnection, it drops to the main thread which in turn drops the player. Theoretically it should be possible to restart on the same position for sure, though I am not sure how much work it would be to hack |
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 1
-
|
Hi, |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
I'll throw 50 quid in the hat, no qualms, for a thing I've used everyday for free for years this is the only irritation! |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Would gladly contribute to a bounty to improve this! |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
I'm testing go-librespot now. My experiences are much better. Very stable connection. |
Beta Was this translation helpful? Give feedback.
All reactions
-
❤️ 2
-
|
I see that go-librespot is just retrying a bunch of times, rotating around the access points. That isn't what librespot does but we might as well go down this route and paper over whatever problem this is. Maybe it is just Spotify's shoddy servers. Happy with that approach @roderickvd ? |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Certainly. Hope you can find some straightforward way to do that. Don't want to be discouraging but just to share some memories: because librespot previously was not architected to do that, I think I remember it would require a bit of refactoring to do it. Sounds simple but may take more lines than you'd expect, because AP authentication and finally seeing if connectivity is OK happen in rather different places. |
Beta Was this translation helpful? Give feedback.
All reactions
-
|
Reconnection logic has been greatly improved, closing for now but feel free to reopen if you still have specific use cases. |
Beta Was this translation helpful? Give feedback.
All reactions
This discussion was converted from issue #134 on February 22, 2021 23:39.
Uh oh!
There was an error while loading. Please reload this page.
-
This issue serves as a placeholder for discussion around the rewrite of the session handling logic, as it is currently one of the less stable parts of librespot. Issue #103 is related to this.
Beta Was this translation helpful? Give feedback.
All reactions