fix: gracefully exit when misconfigured or unavailable audio backend #1390
Conversation
|
I tried to limit the changes at first by leaving backend creation up to the worker thread (the way it currently is in the |
| { | ||
| let worker_channel = self.channel.clone(); | ||
| let cfg = self.cfg.clone(); | ||
| let events = self.events.clone(); | ||
| let volume = self.volume(); | ||
| let credentials = self.credentials.clone(); | ||
| ASYNC_RUNTIME.get().unwrap().spawn(Self::worker( | ||
| worker_channel, | ||
| events, | ||
| rx, | ||
| cfg, | ||
| credentials, | ||
| user_tx, | ||
| volume, | ||
| )); | ||
| } |
There was a problem hiding this comment.
Don't know why this scope was here. I don't think it does anything.
When the user has an error in their audio backend configuration or doesn't have audio backends available, gracefully exit instead of panicking.
ThomasFrans
force-pushed
the
fix-unknown-backend-panic
branch
from
7f4f74a to
7eaf748
Compare
| eprintln!("{error}"); | ||
| error!("{error}"); |
There was a problem hiding this comment.
I thought it could be nice to have the complete picture of what happened in the log, as well as a message to stderr to show the user that this was expected (not a crash) and why it quit.
There was a problem hiding this comment.
Hmm, does log::error log to stdout or stderr?
There was a problem hiding this comment.
I don't think it's set up to log to either. It only logs to the file if I remember correctly, because stdout/stderr wouldn't work during most of the program.
There was a problem hiding this comment.
Oh wow, right, this is still a curses application 🤦
When the user has an error in their audio backend configuration or doesn't have audio backends available, gracefully exit instead of panicking.
Describe your changes
Move audio backend initialization into the main thread for simpler error handling. Propagate errors up to the main function and log/print them there before quitting. This makes sure that everything that needs to be dropped is dropped automatically (e.g. not the case when manual exit from
Application::new()which needs manual drop ofCursiveRunnerto reset terminal).Issue ticket number and link
closes #1383
Checklist before requesting a review
not performance improvements, etc.)