Nicotine+ doesn't open: Value: 'int' object has no attribute 'split' error

[lachlanshoesmith]

Nicotine+ version: 3.2.1
Operating System/Distribution: Arch (using kernel 5.6.10-arch1-1)

Describe the bug

Nicotine+ crashes when I open it with the following error:

Nicotine+ Version: 3.2.1
GTK Version: 3.24.31
Python Version: 3.10.2 (main, Jan 15 2022, 19:56:27) [GCC 11.1.0]

Type: <class 'AttributeError'>
Value: 'int' object has no attribute 'split'
Traceback:   File "/usr/lib/python3.10/site-packages/pynicotine/gtkgui/frame.py", line 1982, in do_activate
    NicotineFrame(
  File "/usr/lib/python3.10/site-packages/pynicotine/gtkgui/frame.py", line 200, in __init__
    connect_ready = network_processor.start(self, self.network_callback)
  File "/usr/lib/python3.10/site-packages/pynicotine/pynicotine.py", line 247, in start
    self.transfers.init_transfers()
  File "/usr/lib/python3.10/site-packages/pynicotine/transfers.py", line 133, in init_transfers
    self.downloadsview.init_transfers(self.downloads)
  File "/usr/lib/python3.10/site-packages/pynicotine/gtkgui/transferlist.py", line 210, in init_transfers
    self.update(forceupdate=True)
  File "/usr/lib/python3.10/site-packages/pynicotine/gtkgui/transferlist.py", line 311, in update
    self.update_specific(transfer_i)
  File "/usr/lib/python3.10/site-packages/pynicotine/gtkgui/transferlist.py", line 498, in update_specific
    shortfn = filename.split("\\")[-1]

Expected behavior

Nicotine+ will open successfully.

Steps to reproduce the bug

Open Nicotine+ on my system.

Additional context

I don't really have any good explanation for this error, but it also occurs when using the nicotine-plus-git package. If anyone has any pointers I'd be happy to provide some more information than I have.

[mathiascode]

Did this issue start occurring recently? If so, do you remember what you did? There seems to be an invalid file path value at the second index of one of the downloads, in ~/.local/share/nicotine/downloads.json. Could you post the problematic download here?

[lachlanshoesmith]

Did this issue start occurring recently?

It's been a while since I first started having this error. I never really made the connection that a download could have caused the error, so I stopped using Nicotine for a while in case an update came around that fixed this error.

Could you post the problematic download here?

Here's my downloads.json.

[slook]

In the very first record... 16 that value is an integer, and not a string as it should be. Somehow the size is mixup with filename...

["imwinnin", 16, "", "Old", null, null, "16", "36:53"],

For you @lachlantula you can fix the error by deleting this record from the file (or deleting the entire file).

For us we should put the three missing sanity checks into add_stored_transfers for user=i[0], filename=i[1], path=i[2] here:

nicotine-plus/pynicotine/transfers.py

Line 250 in 701cc30

nicotine-plus/pynicotine/transfers.py

Line 297 in 701cc30

user=i[0], filename=i[1], path=i[2], status=status,

[mathiascode]

In the very first record... 16 that value is an integer, and not a string as it should be. Somehow the size is mixup with filename...

That's the bitrate, the size is missing entirely.

It's possible that an old build extracted the incorrect values when queuing a download from a certain part of the program, at least I have vague memories of this. I've checked all parts of the code that provide a file path to Transfer objects, and everything seemed fine. Since nobody else has reported this issue, it probably happened a long time ago, and started causing issues after the critical error dialog was added.

For us we should put the three missing sanity checks into add_stored_transfers for user=i[0], filename=i[1], path=i[2] here:

nicotine-plus/pynicotine/transfers.py

Line 250 in 701cc30

nicotine-plus/pynicotine/transfers.py

Line 297 in 701cc30

user=i[0], filename=i[1], path=i[2], status=status,

str(i[0]), str(i[1]) and str(i[2]) should do. We should also skip transfers that don't include these properties.

[slook]

str(i[0]), str(i[1]) and str(i[2]) should do.

No, this is insufficient, because str() will gladly convert an int value into a valid string, which then appears in the list.

We should also skip transfers that don't include these properties.

Indeed, isinstance() is required if we are to ensure that those values are not integers or some other data type that can be interpreted as a string when it shouldn't be. The values of [0], [1], and [2] all need to be strings, if they're not, then something went seriously wrong and the entire record must be purged (which happens naturally upon save if the record isn't added in the list).

[mathiascode]

No, this is insufficient, because str() will gladly convert an int value into a valid string, which then appears in the list.

It depends on the situation, but yes, we should probably discard the download entirely here, since these strings are sent to other peers.

I find isinstance() ugly in general, but I guess we don't have another option here.

[slook]

We could use isdigit() or is_integer() to check any() of map() values contain only numerics in one go, maybe this will be faster or maybe not.

Or we could do some string operation, like split() or somesuch, to invoke the except handler that is in-place.