jellyfin / jellyfin-kodi

Jellyfin Plugin for Kodi
https://jellyfin.org
GNU General Public License v3.0
856 stars 113 forks source link

Crash when trying to remove libraries: FullSync.remove_library() missing 1 required positional argument: 'library_id' #923

Open jfly opened 1 month ago

jfly commented 1 month ago

Describe the bug

The plugin crashes when I try to remove libraries under Addons > Jellyfin > Manage libraries > Remove libraries.

I see the following in my logs:

2024-10-01 11:11:11.911 T:1158343    info <general>: JELLYFIN.jellyfin_kodi.library -> ERROR::jellyfin_kodi/library.py:640 FullSync.remove_library() missing 1 required positional argument: 'library_id'
                                                   Traceback (most recent call last):
                                                     File "jellyfin_kodi/library.py", line 636, in remove_library
                                                       sync.remove_library(library_id)
                                                     File "jellyfin_kodi/helper/wrapper.py", line 43, in wrapper
                                                       result = func(self, dialog=dialog, *args, **kwargs)
                                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                                   TypeError: FullSync.remove_library() missing 1 required positional argument: 'library_id'

Additional context

  1. Here's the problematic function call: https://github.com/jellyfin/jellyfin-kodi/blob/163765872896e2e5d71190dcdfd0c5ee48e828a6/jellyfin_kodi/library.py#L636
  2. You can see that remove_library takes 3 parameters (self, library_id, dialog). However, it's actually decorated with the @progress decorator, which adds an optional item argument as the second parameter after self.

I think to fix this we'd need to start passing library_id as a keyword argument (right now it's getting consumed as the item parameter to the function that @progress returns). We'd also need to pass in dialog, I'm not sure what that is.

oddstr13 commented 1 month ago

Looks like this is likely due to the function getting called with an empty string for the id, I need more logs, specifically the json blob(s) printed at the debug level (enable in the addon settings, not Kodi settings) just before the logged error. Probably easiest to provide more rather than less logs, so I don't have to request more multiple times :slightly_smiling_face:

https://github.com/jellyfin/jellyfin-kodi/blob/163765872896e2e5d71190dcdfd0c5ee48e828a6/jellyfin_kodi/entrypoint/service.py#L366-L371

https://github.com/jellyfin/jellyfin-kodi/blob/163765872896e2e5d71190dcdfd0c5ee48e828a6/jellyfin_kodi/entrypoint/service.py#L352-L360

https://github.com/jellyfin/jellyfin-kodi/blob/163765872896e2e5d71190dcdfd0c5ee48e828a6/jellyfin_kodi/entrypoint/service.py#L208 https://github.com/jellyfin/jellyfin-kodi/blob/163765872896e2e5d71190dcdfd0c5ee48e828a6/jellyfin_kodi/helper/utils.py#L129

Could be a trailing comma in a list of IDs or some such

jfly commented 1 month ago

Oh dear. I completely misunderstood this "Select the libraries to remove" UI:

image

I now see that it's a xbmcgui.Dialog.multiselect, which allows you to just select nothing and press OK

kodi-2024-10-01_18.45.15.webm

This results in selection being an empty array here: https://github.com/jellyfin/jellyfin-kodi/blob/163765872896e2e5d71190dcdfd0c5ee48e828a6/jellyfin_kodi/library.py#L595, which zips right past this if selection is None check, and we end up setting Id to an empty array here.

So I'm unblocked! I just need to actually select the libraries I want. This simplest way to avoid the crash might be to tweak this if statement to also detect empty arrays. But it might also be nice to put the user in a loop, with a nice dialog telling them to actually select something (or bail out by selecting "Cancel").

oddstr13 commented 1 month ago

Yea, those dialogs have tripped me up a few times too, it's the same functionality for adding libraries IIRC