Album bundle: fall back to per-track on an I/O error, don't hard-fail the batch

Defense-in-depth follow-up to #760. Even with the entrypoint chown fix, if the
album-bundle staging dir ever can't be created/written (permissions, read-only
mount, disk full), the dispatch caught the plugin exception and marked the whole
batch failed — even though the album had already downloaded (the #715 symptom:
'release finishes downloading but the batch fails').

Now an OSError from the plugin is flagged fallback-eligible, so the dispatch
returns to the per-track flow instead of hard-failing. OSError covers the
staging/filesystem failure that motivated this (#760's PermissionError) and, by
Python's IOError==OSError aliasing, any propagated transient I/O error —
falling back is never worse than hard-failing, and per-track is the universal
graceful path. Programming errors (TypeError, KeyError, RuntimeError, …) are
NOT OSError and stay terminal, so genuine bugs still fail loudly — the existing
'plugin exception => failure' contract and its test are preserved.

Test: new test_dispatch_staging_oserror_falls_back_to_per_track (PermissionError
on the staging dir -> result False, phase 'analysis', not failed). Existing
RuntimeError-is-terminal test still passes. 131 album-bundle/plugin tests green.
This commit is contained in:
BoulderBadgeDad 2026-06-01 13:23:08 -07:00
parent aabf1c0e6a
commit c8c3789cb9
2 changed files with 36 additions and 1 deletions

View file

@ -173,7 +173,22 @@ def try_dispatch(
)
except Exception as exc:
logger.exception("[Album Bundle] %s plugin raised: %s", mode, exc)
outcome = {'success': False, 'error': f'Plugin error: {exc}'}
# An OSError means an I/O step failed after the source already had the
# album — most importantly the staging dir not being writable (#760),
# but also any transient filesystem error. Treat it as fallback-eligible
# so we return to the per-track flow instead of hard-failing the whole
# batch (the #715 symptom: files download, then the batch fails).
# Programming errors (TypeError, KeyError, …) are NOT OSError and stay
# terminal, so genuine bugs still fail loudly. (requests' network
# exceptions also subclass OSError, but plugins normally catch those
# internally and return an outcome rather than raising; if one does
# surface here, falling back to per-track is still the safe choice.)
is_io_failure = isinstance(exc, OSError)
outcome = {
'success': False,
'error': f'Plugin error: {exc}',
'fallback': is_io_failure,
}
if not outcome.get('success'):
err = outcome.get('error', 'Album bundle download failed')

View file

@ -275,6 +275,26 @@ def test_dispatch_plugin_exception_treated_as_failure() -> None:
assert 'network down' in state.failed_with
def test_dispatch_staging_oserror_falls_back_to_per_track():
"""A filesystem/staging failure (e.g. #760's PermissionError creating the
staging dir) means the album downloaded but couldn't be staged locally —
fall back to the per-track flow rather than hard-failing the whole batch."""
state = _FakeState()
plugin = MagicMock()
plugin.download_album_to_staging.side_effect = PermissionError(
"[Errno 13] Permission denied: 'storage/album_bundle_staging'")
result = try_dispatch(
batch_id='b1', is_album=True,
album_context={'name': 'Carnival'}, artist_context={'name': 'Some Artist'},
config_get=_config({'download_source.mode': 'soulseek'}),
plugin_resolver=lambda _name: plugin, state=state,
)
assert result is False # fell back; master continues per-track
assert state.failed_with == '' # NOT hard-failed
assert state.fields['phase'] == 'analysis'
assert state.fields['album_bundle_state'] == 'fallback'
def test_dispatch_strips_whitespace_from_names() -> None:
"""Trailing whitespace in batch context shouldn't fail the
eligibility predicate AND should be cleaned before passing to