Improved error message for resampler failures#2226
Open
CdrSonan wants to merge 4 commits into
Open
Conversation
Collaborator
|
Resampler used to pop error dialog. It was considered too annoying and suppressed. Adding it back will be considered too annoying again by people and suppressed again. I'd consider it enough to log the error to logs. You can open the log window to debug. |
Author
|
This implementation does not change the number of error dialogues the user sees. It replaces the aggregate exception error dialogue containing the wavtool exception(s) with a new, different aggregate exception dialogue that's hopefully more useful. You can see this when you look at the diff in RenderEngine.cs. The new case for resampler exceptions mirrors the default case these errors would previously fall under. Only the error message is changed. |
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
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.
When using the Classic renderer and the resampler encounters an issue, it quietly fails instead of throwing an exception.
An exception is only raised when the wav tool is run in the next step, and it cannot find the resampler output.
This is problematic because the resulting error message contains no information about the actual root cause of the problem, making it effectively useless. This makes spotting oto.ini errors, which are often the cause of resampler failures, as well as debugging the resampler itself more difficult.
This pull request therefore implements a new type of error message specifically for resampler failures:
ProcessRunner.Run(), the stdout and stderr streams, as well as the exit code of the subprocess are accumulated into a string variable, which is returned when the process exits or is terminated. This provides a mechanism for tracking the output of the resampler executable. Resamplers might write error-related messages to stdout instead of stderr, which is why stderr is always included. The existing debug flag still controls whether stdout messages are forwarded to the logger.ProcessRunner.Run()call inDoResamplerReturnsFile(), it is checked whether an output file has been generated. This seems to be the most reliably way to check for resampler success.ResamplerFailedException) is raised. Its error message incorporates the resampler console output captured byProcessRunner.Run().RenderEngine.RenderTracks(), a new else-if is triggered when at least oneResamplerFailedExceptionis included in the exception list created by the renderer. In that case, the message of the generatedMessageCustomizableExceptionis adjusted to include the information that this is a resampler or oto.ini issue, and that generating a singer error report could be useful.A possible extension of this would be to replace the
MessageCustomizableExceptionwith a newAggregateResamplerFailedExceptionclass. This could be detected in theMainWindow, and would allow displaying a fully custom error popup, including a button to immediately generate a singer error report without navigating through menus. I can implement this as well, so please let me know if you think this is a good idea.