[PR #427] [MERGED] Gracefully handle lost network connections #922

Closed
opened 2026-02-27 20:00:28 +03:00 by kerem · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/librespot-org/librespot/pull/427
Author: @kaymes
Created: 1/23/2020
Status: Merged
Merged: 1/30/2020
Merged by: @sashahilton00

Base: devHead: connection-lost-crash


📝 Commits (10+)

  • 362106d Fix error handling for closed channel.
  • b6c676a Prevent librespot from panicking if server connection is lost.
  • 3fe3849 Enable Mercury to be shut down and all pending requests being cancelled.
  • 04b52d7 Have player handle Mercury errors while loading tracks.
  • ea1e092 Enable proper shutdown of the channels.
  • 719943a Don't panic if spirc terminates prematurely.
  • 17821b2 Rename variable to be in line with existing code.
  • f26db01 Rate-limit automatic re-connection attempts when spirc shuts down.
  • dadab48 Don't exit if too many spirc failures.
  • 0e22678 Workaround for Rust 1.33 borrow checker.

📊 Changes

7 files changed (+158 additions, -50 deletions)

View changed files

📝 connect/src/spirc.rs (+5 -1)
📝 core/src/channel.rs (+14 -2)
📝 core/src/connection/mod.rs (+20 -2)
📝 core/src/mercury/mod.rs (+31 -15)
📝 core/src/session.rs (+9 -3)
📝 playback/src/player.rs (+23 -5)
📝 src/main.rs (+56 -22)

📄 Description

Currently librespot tends to panic as soon as something goes wrong with the server connection. Lost server connections can have a variety of causes such as network issues or a spotify server becoming unavailable. Librespot should recover from lost server connections gracefully and reconnect and/or wait for a new connection from a client.

Here I made a series of changes that convert panics into proper error handling in case of lost connections.

  • A lost network connection no longer causes a panic. Instead errors and channel closures are propagated through the system.
  • When the session shuts down, the mercury and channel components are shut down as well and all channels of pending requests are dropped. This notifies pending requests and allows them to handle the situation. This is a necessary to avoid deadlocks. (e.g. the player thread might not join otherwise).
  • Spontaneous shutdown of sqirc in main no longer causes a panic. Instead libreelec interprets it as a connection lost situation and attempts to re-connect with the same credentials. I added a limiter such that no further re-connection attempt is made if 5 re-connects happened within the last 10 minutes. In this case it just waits for another client to connect. This is more a precaution in case spirc shutdowns are triggered by protocol changes or similar one day.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/librespot-org/librespot/pull/427 **Author:** [@kaymes](https://github.com/kaymes) **Created:** 1/23/2020 **Status:** ✅ Merged **Merged:** 1/30/2020 **Merged by:** [@sashahilton00](https://github.com/sashahilton00) **Base:** `dev` ← **Head:** `connection-lost-crash` --- ### 📝 Commits (10+) - [`362106d`](https://github.com/librespot-org/librespot/commit/362106df622126f8ee2af3038a7eeb45b424229f) Fix error handling for closed channel. - [`b6c676a`](https://github.com/librespot-org/librespot/commit/b6c676ad600b461d96ac999a87fc805f10985177) Prevent librespot from panicking if server connection is lost. - [`3fe3849`](https://github.com/librespot-org/librespot/commit/3fe384958833e983ac58da25094ffdc4e0b8d27c) Enable Mercury to be shut down and all pending requests being cancelled. - [`04b52d7`](https://github.com/librespot-org/librespot/commit/04b52d7878d3ad52233007aafd19ce5dc86ff056) Have player handle Mercury errors while loading tracks. - [`ea1e092`](https://github.com/librespot-org/librespot/commit/ea1e0925dcf0717641794f3f4b5d999389723329) Enable proper shutdown of the channels. - [`719943a`](https://github.com/librespot-org/librespot/commit/719943aec9f5e7f66563e288c970207cb154d919) Don't panic if spirc terminates prematurely. - [`17821b2`](https://github.com/librespot-org/librespot/commit/17821b26aaa1860fb0e714f7e792c7088e0f1a20) Rename variable to be in line with existing code. - [`f26db01`](https://github.com/librespot-org/librespot/commit/f26db0111063619ca2e3652273a0ab6da785c289) Rate-limit automatic re-connection attempts when spirc shuts down. - [`dadab48`](https://github.com/librespot-org/librespot/commit/dadab486d2ec8a86b30f618d17599c1a563f2163) Don't exit if too many spirc failures. - [`0e22678`](https://github.com/librespot-org/librespot/commit/0e22678a281fca2b746c4f2210e905398c9e9415) Workaround for Rust 1.33 borrow checker. ### 📊 Changes **7 files changed** (+158 additions, -50 deletions) <details> <summary>View changed files</summary> 📝 `connect/src/spirc.rs` (+5 -1) 📝 `core/src/channel.rs` (+14 -2) 📝 `core/src/connection/mod.rs` (+20 -2) 📝 `core/src/mercury/mod.rs` (+31 -15) 📝 `core/src/session.rs` (+9 -3) 📝 `playback/src/player.rs` (+23 -5) 📝 `src/main.rs` (+56 -22) </details> ### 📄 Description Currently librespot tends to panic as soon as something goes wrong with the server connection. Lost server connections can have a variety of causes such as network issues or a spotify server becoming unavailable. Librespot should recover from lost server connections gracefully and reconnect and/or wait for a new connection from a client. Here I made a series of changes that convert panics into proper error handling in case of lost connections. * A lost network connection no longer causes a panic. Instead errors and channel closures are propagated through the system. * When the session shuts down, the mercury and channel components are shut down as well and all channels of pending requests are dropped. This notifies pending requests and allows them to handle the situation. This is a necessary to avoid deadlocks. (e.g. the player thread might not join otherwise). * Spontaneous shutdown of sqirc in main no longer causes a panic. Instead libreelec interprets it as a connection lost situation and attempts to re-connect with the same credentials. I added a limiter such that no further re-connection attempt is made if 5 re-connects happened within the last 10 minutes. In this case it just waits for another client to connect. This is more a precaution in case spirc shutdowns are triggered by protocol changes or similar one day. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
kerem 2026-02-27 20:00:28 +03:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/librespot#922
No description provided.