Catch when items have no path

[Serene-Arc]

Description

When importing things using beets, by pure chance, it is apparently possible to have an item inserted into the database that does not have a path registered. Because the database returns None for the items' paths, cascading errors cause most library-wide operations to fail. Update, write, etc, all fail.

This effectively poisons the database and there's no way to remove it other than manually editing the database file.

This PR just changes two lines and a typing so that the update command can still be used. The other commands will still fail but this seems to be a pretty rare edge case so I've only changed the update command. Now the logic, when there is no path associated with an entry, will consider it to be the same as a deleted file and purge it from the database.

To Do

• Changelog. (Add an entry to docs/changelog.rst near the top of the document.)
• Tests. (Encouraged but not strictly required.)

[sampsyo]

That's interesting! I would love to know what the root cause is… that is, why it's ever possible to end up with a path with value None. It seems like that's somewhat worrisome and could be indicative of a larger problem.

However, I certainly don't object to suppressing a crash in this case. It looks like this addresses a couple of places where the None would case problems, but of course there are probably others. Regardless, it is probably fine to just address these two!

[Serene-Arc]

There are cases where things can crash, or even just a user pressing ctrl+c a bunch of times at the exact wrong moment. I encountered this when using beets with the audible plugin but I don't see anything that would make it specific to that plugin.

I think it's just chance. When beets is trying to calculate the new path, maybe there's a point where it keeps a null path in the database? I'm not sure, I haven't looked into the cause too much because, once it happens, you need a way to rectify it. It doesn't seem to be very common anyway.

[JOJ0]

Awesome @Serene-Arc, I'm sure this is a good idea.

Just a sidenote / related: I also can report things that happen / get missing / are incomplete from using beets "not entirely the right way" - I often ctrl-c beets in my dev environment but also happens in my prod-setup, when things are about to be "cleaned up" by beets. I for example noticed crashes with empty album names (Not sure anymore, was it album names....... or something else...anyway....investigation showed that fields where empty in the sqlite db which caused it....).

[Serene-Arc]

Yeah I send signals to it on occasion too and it can cause these and similar problems. Also DNS errors can cause abrupt stops which I need to chase down too.

[Serene-Arc]

@JOJ0 The empty albums can be fixed by the update command now I think, it can handle practically any screwed up metadata except the path field, which always causes a hard crash without this fix.

[sampsyo]

OK, sounds good! Please go ahead and merge whenever you're happy. I still don't think this lets us off the hook for the original underlying problem (see below), but I support adding this to avoid crashes while we don't know how this could happen.

@Serene-Arc:

There are cases where things can crash, or even just a user pressing ctrl+c a bunch of times at the exact wrong moment.

@JOJ0:

I also can report things that happen / get missing / are incomplete from using beets "not entirely the right way" - I often ctrl-c beets in my dev environment but also happens in my prod-setup, when things are about to be "cleaned up" by beets.

FWIW, I would still consider this a bug, even if it's a ^C interruption. When beets inserts things into the SQLite database, it does so (or at least attempts to do so) atomically: that is, even if the program crashes, the new row in the database is supposed to either entirely exist or entirely not exist; there should never be any in-between states visible after a crash. (This is of course hard to achieve, but SQLite fortunately takes care of it for us.) So I suspect that there's some situation where some code is using two different transactions: one to add items to the database, and another one to update path. It would be great if we avoided that in all cases and only inserted fully-formed items into the database.

[Serene-Arc]

@sampsyo I'll add a separate bug so I remember to chase it down (or it'll remind someone, eventually) but I don't think this is a common occurrence so it should be a fairly low priority bug.