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

Improve error handling #677

Open
dvdsk opened this issue Jan 11, 2025 · 4 comments
Open

Improve error handling #677

dvdsk opened this issue Jan 11, 2025 · 4 comments

Comments

@dvdsk
Copy link
Collaborator

dvdsk commented Jan 11, 2025

needs: cpal to land so we can rate limit the printing of cpal errors. Will also allow us to implement PartialEq, Eq & Hash on our error types.

@PetrGlad
Copy link
Collaborator

PetrGlad commented Feb 8, 2025

I do not think that just throttling errors is enough. We need to improve error handling in general.
In particular error callback we pass during stream initialization has misleading error message (OutputSream::init_stream(..)). Errors that it receives are playback errors likely unrelated to initialization.
For example when the output device disappears/disconnected the errors seem to be unrecoverable so ideally we should try to re-open the stream or at least let users to decide what to do with it. Meanwhile if this happens users have no chance to handle this besides relaunching app manually. On most modern Linux distributions such error is hard to notice since normally a default device from some sound server (like PipeWire) is used. Sound server usually handles the playback fail-over transparently. But you still can trigger such error by stopping the server. E.g. on systems with systemd and PipeWire launch a rodio app and then systemctl --user stop pipewire. Just relaunching the server right away does not help, rodio app will keep spamming.

I tried to prototype automatic reconnection logic but currently it would look rather cumbersome, primarily because then we need an initialization function that re-creates the sources on-demand.

@PetrGlad
Copy link
Collaborator

PetrGlad commented Feb 8, 2025

Another related issue is that when I tried to avoid using standard output for messages I found that alsalib still writes messages when an error occurs, so even if Rust part does not write anything, something still may end up in standard output.

@PetrGlad
Copy link
Collaborator

PetrGlad commented Feb 8, 2025

Another related thing to look at is real-time errors. For example we may not have any error messages or diagnostics if rodio's sources are too slow at producing samples.
I tried to add a second delay to periodic_access callback and nothing really happens. alsalib may show some errors sometimes but even that does not happen every time. The cpal callback does provide real time timestamp that is supposed to play right now but at the moment it is ignored. So potentially we can detect delays in the library.

@dvdsk
Copy link
Collaborator Author

dvdsk commented Feb 8, 2025

you bring up some good issues. Re-engineering error handling in rodio should be done at some point. For now I am going to focus on landing all the in progress and already planned work. Ill rename this to reflect your observations.

@dvdsk dvdsk changed the title Stop cpal errors from filling up stdout Improve error handling Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants