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

Fix crash fetching cover art for unknown album #459

Merged
merged 1 commit into from
Jan 29, 2020
Merged

Fix crash fetching cover art for unknown album #459

merged 1 commit into from
Jan 29, 2020

Conversation

kevinoid
Copy link
Contributor

Ripping an unknown album when cover art fetching is enabled (e.g. whipper cd rip --unknown --cover-art complete) causes whipper to crash with an error similar to the following:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File ".../whipper/whipper/command/main.py", line 43, in main
    ret = cmd.do()
  File ".../whipper/whipper/command/basecommand.py", line 139, in do
    return self.cmd.do()
  File ".../whipper/whipper/command/basecommand.py", line 139, in do
    return self.cmd.do()
  File ".../whipper/whipper/command/cd.py", line 191, in do
    self.doCommand()
  File ".../whipper/whipper/command/cd.py", line 363, in doCommand
    self.program.metadata.mbid)
AttributeError: 'NoneType' object has no attribute 'mbid'

due to accessing self.program.metadata.mbid when self.program.metadata is None. This PR attempts to avoid teh issue by only fetching cover art when self.program.metadata is available.

Thanks for considering,
Kevin

@ABCbum
Copy link
Contributor

ABCbum commented Jan 29, 2020

Hi @kevinoid , i'm afraid that without metadata (especially release id) fetching coverart from Cover Art Archive is not possible. Despite that i think we should still improve the message given to users. Thanks (. ❛ ᴗ ❛.)

@kevinoid
Copy link
Contributor Author

i'm afraid that without metadata (especially release id) fetching coverart from Cover Art Archive is not possible.

I agree. That's why this PR disables fetching when the metadata is not present.

Despite that i think we should still improve the message given to users.

Agreed. PR updated.

@JoeLametta
Copy link
Collaborator

JoeLametta commented Jan 29, 2020

Pushed a new commit: don't know if the stricter check makes sense...

@kevinoid
Copy link
Contributor Author

Pushed a new commit: don't know if the stricter check makes sense...

Looks good to me. Thanks!

Ripping an unknown album when cover art fetching is enabled (e.g.
`whipper cd rip --unknown --cover-art complete`) causes whipper to crash
with an error similar to the following:

```python
Traceback (most recent call last):
    File "<string>", line 1, in <module>
    File ".../whipper/whipper/command/main.py", line 43, in main
    ret = cmd.do()
    File ".../whipper/whipper/command/basecommand.py", line 139, in do
    return self.cmd.do()
    File ".../whipper/whipper/command/basecommand.py", line 139, in do
    return self.cmd.do()
    File ".../whipper/whipper/command/cd.py", line 191, in do
    self.doCommand()
    File ".../whipper/whipper/command/cd.py", line 363, in doCommand
    self.program.metadata.mbid)
AttributeError: 'NoneType' object has no attribute 'mbid'
```

due to accessing `self.program.metadata.mbid` when
`self.program.metadata` is `None`. To avoid this, only attempt to get
cover art when `self.program.metadata` is available.

Also print a warning when the cover art can't be fetched to inform the
user that it isn't being downloaded.

Signed-off-by: Kevin Locke <[email protected]>
@JoeLametta JoeLametta merged commit dea7db1 into whipper-team:develop Jan 29, 2020
@JoeLametta
Copy link
Collaborator

Merged, thanks!

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.

3 participants