Repository navigation
Fix unresponsive stall when server error occurs - #560
Merged
Merged
Conversation
…t blocking further execution by throwing
larson-carter
approved these changes
Dec 31, 2022
larson-carter
left a comment
larson-carter
left a comment
Collaborator
There was a problem hiding this comment.
Sorry for taking so long, thank you for reaching out over twitter DMs. Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #559, also fixes #539 and fixes #521.
The referenced issue occurs here (Presumably, the unresponsiveness occurs due to the playlist not being properly resized. Regardless, ensuring that the resizing does happen even after an error occurs seems to solve this).
ncmpcpp/src/screens/playlist_editor.cpp
Lines 170 to 180 in 417d717
The error code received in the issue is
MPD_SERVER_ERROR_UNKNOWN_CMD, which fails the if statement, and thus the code throws the error.The problem here is throwing leaves the remaining code outside of the catch block (which handles the case where the playlist might not have been iterated through fully) unreachable.
The code already appears to be trying to handle
MPD_SERVER_ERROR_SYSTEM(which in that particular instance is not that different from callingStatus::handleServerError(e)anyway), so we might as well call that regardless of the error code, while not throwing anything in the process.