-
Notifications
You must be signed in to change notification settings - Fork 65
Show all client errors when tracks fail to load or play #188
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't say I'm fond of the changes to the audio track code. If an exception bubbles up high enough to hit the for loop to move to the next client, the stream restarts from 0, which is especially problematic in situations where the track has played, say, 50-90% through.
common/src/main/java/dev/lavalink/youtube/AllClientsFailedException.java
Outdated
Show resolved
Hide resolved
common/src/main/java/dev/lavalink/youtube/YoutubeAudioSourceManager.java
Outdated
Show resolved
Hide resolved
|
Current formatting |
|
i think it looks clean with the current state having the errors separate per client makes it slightly more readable |
Yeah I think maybe the only other thing to do might be to put some spaces in front of caused by lines for a little more visual separation, but unfortunately it isn't east to make such big logs look good |
Wanted to take a shot at better organizing error messages for failed tracks or at least start the conversation.
This pr, rather than only watching the last exception surfaces all exceptions from all clients in the logs.
Changes in error handling
Also do note that I removed some of the more specific error handling to determine if an error is possible to send to the next client. Rather than fail fast it seems that trying all clients would be preferable. Rather than letting one client failure potentially stop retries from other clients. One of the cases I could see wanting this check is the errors that manifest after track start some time into the track. Rather than restarting the track with a new client, just killing it doesn't sound too bad, if we want to add that check back in.
Error format
Open to messing some more with the formatting if wanted, not sure I am totally sold on it as is and am fine with iterating. It also probably makes sense to move the yt-source version out of the client info to not repeat itself a ton
Example new stack trace: (invalid oauth token)
Edit: Current state as of 74e0aa6