replaygain: Fix error handling for parallel runs #4506
Conversation
The parallelism strategy in #3478, in retrospect, used a pretty funky way to deal with exceptions in the asynchronous work---since `apply_async` has an `error_callback` parameter that's meant for exactly this. The problem is that the wrapped function would correctly log the exception *and then return `None`*, confusing any downstream code. Instead of just adding `None`-awareness to the callback, let's just avoid running the callback altogether in the case of an error.
|
This fixes the obvious crash in the error handling. However, there is still a problem with invoking |
👍 A few days ago, I've also realized this when testing #4374, which was also plagued by mysterious CI failures that I couldn't trace back to any of the changes there. However, your fix is much cleaner than the quick workaround I came up with! |
This is a second fix to errors that have been cropping up in CI lately. Put this one into the bucket labeled "I don't know why this ever worked," but it's causing problems only on the Windows CI builds.
The parallelism strategy in #3478, in retrospect, used a pretty funky way to deal with exceptions in the asynchronous work—since
apply_asynchas anerror_callbackparameter that's meant for exactly this. The problem is that the wrapped function would correctly log the exception and then returnNone, confusing any downstream code. Instead of just addingNone-awareness to the callback, let's just avoid running the callback altogether in the case of an error.