Skip to content
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

replaygain: Fix error handling for parallel runs #4506

Merged
merged 3 commits into from
Oct 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 16 additions & 22 deletions beetsplug/replaygain.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,14 @@ class FatalGstreamerPluginReplayGainError(FatalReplayGainError):
loading the required plugins."""


def call(args, **kwargs):
def call(args, log, **kwargs):
"""Execute the command and return its output or raise a
ReplayGainError on failure.
"""
try:
return command_output(args, **kwargs)
except subprocess.CalledProcessError as e:
log.debug(e.output.decode('utf8', 'ignore'))
raise ReplayGainError(
"{} exited with status {}".format(args[0], e.returncode)
)
Expand Down Expand Up @@ -261,7 +262,7 @@ def __init__(self, config, log):

# check that ffmpeg is installed
try:
ffmpeg_version_out = call([self._ffmpeg_path, "-version"])
ffmpeg_version_out = call([self._ffmpeg_path, "-version"], log)
except OSError:
raise FatalReplayGainError(
f"could not find ffmpeg at {self._ffmpeg_path}"
Expand Down Expand Up @@ -394,7 +395,7 @@ def _analyse_item(self, item, target_level, peak_method,
self._log.debug(
'executing {0}', ' '.join(map(displayable_path, cmd))
)
output = call(cmd).stderr.splitlines()
output = call(cmd, self._log).stderr.splitlines()

# parse output

Expand Down Expand Up @@ -525,7 +526,7 @@ def __init__(self, config, log):
# Check whether the program is in $PATH.
for cmd in ('mp3gain', 'aacgain'):
try:
call([cmd, '-v'])
call([cmd, '-v'], self._log)
self.command = cmd
except OSError:
pass
Expand Down Expand Up @@ -605,7 +606,7 @@ def compute_gain(self, items, target_level, is_album):

self._log.debug('analyzing {0} files', len(items))
self._log.debug("executing {0}", " ".join(map(displayable_path, cmd)))
output = call(cmd).stdout
output = call(cmd, self._log).stdout
self._log.debug('analysis finished')
return self.parse_tool_output(output,
len(items) + (1 if is_album else 0))
Expand Down Expand Up @@ -1085,7 +1086,7 @@ def run(self):
try:
exc = self._queue.get_nowait()
self._callback()
raise exc[1].with_traceback(exc[2])
raise exc
except queue.Empty:
# No exceptions yet, loop back to check
# whether `_stopevent` is set
Expand Down Expand Up @@ -1338,23 +1339,16 @@ def open_pool(self, threads):

def _apply(self, func, args, kwds, callback):
if self._has_pool():
def catch_exc(func, exc_queue, log):
"""Wrapper to catch raised exceptions in threads
def handle_exc(exc):
"""Handle exceptions in the async work.
"""
def wfunc(*args, **kwargs):
try:
return func(*args, **kwargs)
except ReplayGainError as e:
log.info(e.args[0]) # log non-fatal exceptions
except Exception:
exc_queue.put(sys.exc_info())
return wfunc

# Wrap function and callback to catch exceptions
func = catch_exc(func, self.exc_queue, self._log)
callback = catch_exc(callback, self.exc_queue, self._log)

self.pool.apply_async(func, args, kwds, callback)
if isinstance(exc, ReplayGainError):
self._log.info(exc.args[0]) # Log non-fatal exceptions.
else:
self.exc_queue.put(exc)

self.pool.apply_async(func, args, kwds, callback,
error_callback=handle_exc)
else:
callback(func(*args, **kwds))

Expand Down
3 changes: 3 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ New features:

Bug fixes:

* :doc:`/plugins/replaygain`: Avoid a crash when errors occur in the analysis
backend.
:bug:`4506`
* We now respect the Spotify API's rate limiting, which avoids crashing when the API reports code 429 (too many requests).
:bug:`4370`
* Fix implicit paths OR queries (e.g. ``beet list /path/ , /other-path/``)
Expand Down