-
Notifications
You must be signed in to change notification settings - Fork 120
Description
Hi!
Ive realized that the error handling of commands is a bit limited. I explain.
So for example if we try to do client.sticker_get("song", uri, tag) and the particular sticker doesnt exist in that uri we get a:
CommandError: [50@0] {sticker} no such sticker
But if the sticker database isnt active we get
CommandError: [5@0] {sticker} sticker database is disabled
Same error (a very clear different accompany text tho, nice) for two arguebly very different things
In a example code:
try:
played = int(client.sticker_get("song", song["file"], "played"))
except CommandError:
played = 0
try:
client.sticker_set("song", song["file"], "played", played + 1)
except CommandError:
passAgainst:
try:
played = int(client.sticker_get("song", song["file"], "played"))
except NoStickerError:
played = 0
except StickerDBError:
pass
else:
client.sticker_set("song", song["file"], "played", played + 1)Dont give too much importance to the example itself, its just where i realized this, given the solution it doesnt make "much" difference; but i do belive it would be better practice to have clearer errors. Havent tried but I imagine similar things might occur with other commands.
Also it could be argued that if a sticker doesnt exist it could return None instead of error. That would make the code:
# At the start of the code
try:
# A test to see that stickers db is activated
except StickerDBError:
# whatever
# ...
played = client.sticker_get("song", song["file"], "played")
if played is None:
played = 0
client.sticker_set("song", song["file"], "played", int(played) + 1)But i gues thats more about what mpd itself wants to do, and it wouldnt be quite right to have a different behaviour here
Thanks for the amazing library! :D