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

Prevent a crash if MusicBrainz release date is missing #139

Merged
merged 1 commit into from
Mar 6, 2017
Merged

Prevent a crash if MusicBrainz release date is missing #139

merged 1 commit into from
Mar 6, 2017

Conversation

ribbons
Copy link
Contributor

@ribbons ribbons commented Mar 5, 2017

If MusicBrainz returns data for a CD being ripped but no release date is returned (e.g. https://musicbrainz.org/release/340b94f5-139e-4f7b-90dd-82d04a70c57b), we attempt to set the DATE tag to None (which is caught by mutagen), causing the following crash:

Traceback (most recent call last):                
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.4.2-py2.7.egg/morituri/extern/task/task.py", line 511, in c
    callable(*args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.4.2-py2.7.egg/morituri/common/encode.py", line 227, in _tag
    w.save()
  File "/usr/local/lib/python2.7/dist-packages/mutagen/_util.py", line 158, in wrapper
    return func(*args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/mutagen/_util.py", line 129, in wrapper
    return func(self, h, *args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/mutagen/flac.py", line 860, in save
    self.metadata_blocks, available, content_size, padding)
  File "/usr/local/lib/python2.7/dist-packages/mutagen/flac.py", line 154, in _writeblocks
    data += cls._writeblock(block)
  File "/usr/local/lib/python2.7/dist-packages/mutagen/flac.py", line 126, in _writeblock
    datum = block.write()
  File "/usr/local/lib/python2.7/dist-packages/mutagen/flac.py", line 354, in write
    return super(VCFLACDict, self).write(framing=framing)
  File "/usr/local/lib/python2.7/dist-packages/mutagen/_vorbis.py", line 191, in write
    self.validate()
  File "/usr/local/lib/python2.7/dist-packages/mutagen/_vorbis.py", line 171, in validate
    raise ValueError("%r is not a valid value" % value)
ValueError: None is not a valid value
CRITICAL:morituri.command.cd:Giving up on track 1 after 5 times
Traceback (most recent call last):
  File "/usr/local/bin/whipper", line 11, in <module>
    load_entry_point('whipper==0.4.2', 'console_scripts', 'whipper')()
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.4.2-py2.7.egg/morituri/command/main.py", line 31, in main
    ret = cmd.do()
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.4.2-py2.7.egg/morituri/command/basecommand.py", line 123, in do
    return self.cmd.do()
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.4.2-py2.7.egg/morituri/command/basecommand.py", line 123, in do
    return self.cmd.do()
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.4.2-py2.7.egg/morituri/command/cd.py", line 206, in do
    self.doCommand()
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.4.2-py2.7.egg/morituri/command/cd.py", line 507, in doCommand
    ripIfNotRipped(i + 1)
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.4.2-py2.7.egg/morituri/command/cd.py", line 447, in ripIfNotRipped
    "track can't be ripped. "

To resolve the issue, this PR adds a check to see if there is a value for release date before setting it. I've not added a warning message, as this is already output before the crash occurs:

WARNING:morituri.common.mbngs:Release with ID '340b94f5-139e-4f7b-90dd-82d04a70c57b' (The Goons - 'Enter Bluebottle...') does not have a date

Not sure if this should have a unit test & if so where it should go (don't seem to be any MusicBrainz unit tests for track ripping?), so I've not added one - let me know if I should.

Fixes #133.

If MusicBrainz returns data for the CD being ripped but no release date
is returned, we attempt to set the DATE tag to None (which is caught by
mutagen).  To resolve this, check if there is a value for release date
before setting.
@JoeLametta
Copy link
Collaborator

Not sure if this should have a unit test & if so where it should go (don't seem to be any MusicBrainz unit tests for track ripping?), so I've not added one - let me know if I should.

I think that's fine (similar to what I've mentioned here in the bug report).

Thanks for the PR.

@JoeLametta JoeLametta merged commit 8c74d73 into whipper-team:master Mar 6, 2017
@ribbons
Copy link
Contributor Author

ribbons commented Mar 6, 2017

I think that's fine (similar to what I've mentioned here in the bug report).

Thanks for the PR.

No worries - sorry, totally forgot to search for relevant bug reports :-/.

@ribbons ribbons deleted the no-date-crash branch March 6, 2017 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fails to rip if MB Release doesn't have a release date/year
2 participants