Closed oddstr13 closed 1 month ago
Attention: Patch coverage is 23.07692%
with 10 lines
in your changes are missing coverage. Please review.
Project coverage is 21.51%. Comparing base (
842729b
) to head (1899ecb
). Report is 7 commits behind head on master.
Files | Patch % | Lines |
---|---|---|
jellyfin_kodi/jellyfin/api.py | 20.00% | 8 Missing :warning: |
jellyfin_kodi/helper/exceptions.py | 50.00% | 1 Missing :warning: |
jellyfin_kodi/library.py | 0.00% | 1 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
If the plugin is not installed, this is no good. Doesn't trigger a warning in the UI and throws an error in the logs.
2024-05-17 10:08:32.514 T:3058698 info <general>: JELLYFIN.jellyfin_kodi.database -> INFO::jellyfin_kodi/database/__init__.py:154 Database locked in: /home/matt/.kodi/userdata/Database/MyMusic83.db
2024-05-17 10:08:32.522 T:3058698 info <general>: JELLYFIN.jellyfin_kodi.objects.kodi.movies -> INFO::jellyfin_kodi/objects/kodi/movies.py:162 Starting migration for Omega database changes
2024-05-17 10:08:32.523 T:3058698 info <general>: JELLYFIN.jellyfin_kodi.objects.kodi.movies -> INFO::jellyfin_kodi/objects/kodi/movies.py:172 Omega database migration is complete
2024-05-17 10:08:32.685 T:3058698 info <general>: JELLYFIN.jellyfin_kodi.jellyfin.http -> ERROR::jellyfin_kodi/jellyfin/http.py:124 404 Client Error: Not Found for url: http://{jellyfin-server}/Jellyfin.Plugin.KodiSyncQueue/GetPluginSettings
2024-05-17 10:08:32.687 T:3058698 info <general>: JELLYFIN.jellyfin_kodi.library -> ERROR::jellyfin_kodi/library.py:386 (404, HTTPError('404 Client Error: Not Found for url: http://{jellyfin-server}/Jellyfin.Plugin.KodiSyncQueue/GetPluginSettings'))
Traceback (most recent call last):
File "jellyfin_kodi/jellyfin/http.py", line 96, in request
r.raise_for_status()
File "/home/matt/.kodi/addons/script.module.requests/lib/requests/models.py", line 1021, in raise_for_status
raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 404 Client Error: Not Found for url: http://{jellyfin-server}/Jellyfin.Plugin.KodiSyncQueue/GetPluginSettings
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "jellyfin_kodi/library.py", line 356, in startup
if self.server.jellyfin.check_companion_enabled() is not False:
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "jellyfin_kodi/jellyfin/api.py", line 264, in check_companion_enabled
plugin_settings = self._get("Jellyfin.Plugin.KodiSyncQueue/GetPluginSettings") or {}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "jellyfin_kodi/jellyfin/api.py", line 61, in _get
return self._http("GET", handler, {'params': params})
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "jellyfin_kodi/jellyfin/api.py", line 58, in _http
return self.client.request(request)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "jellyfin_kodi/jellyfin/http.py", line 159, in request
raise HTTPException(r.status_code, error)
jellyfin_kodi.helper.exceptions.HTTPException: (404, HTTPError('404 Client Error: Not Found for url: http://{jellyfin-server}/Jellyfin.Plugin.KodiSyncQueue/GetPluginSettings'))
Not even hitting the except requests.RequestException as e:
block so it can't proceed and just blows up. Even fails to establish a websocket connection after this.
If the plugin is installed, then things appear to work properly even without an admin user.
Maybe a suggestion: Can't we append an apikey/token in the url, in the same way it's done for the websocket connection?
I've tested it via postman on my local installation, and that seems to work as well (although I've generated an apikey and used that, as I don't know where the token
comes from for the websocket client and if it works with that as well)
We can add auth to this request, yes. However that's not the problem. The /Plugins
endpoint is now only available to admin level users. General users can't access it, so even if they're authenticated they the existing check would still fail.
Not even hitting the
except requests.RequestException as e:
block so it can't proceed and just blows up. Even fails to establish a websocket connection after this.
As mentioned in the issue (https://github.com/jellyfin/jellyfin-kodi/issues/861#issuecomment-2115876181), the requests.exceptions.*
are caught in the http.py
file and a HTTPException
is raised instead. That's why you need to catch the HTTPException
instead. Might be worth a try to see if it then works. (At least that did the trick in my workaround)
We can add auth to this request, yes. However that's not the problem. The
/Plugins
endpoint is now only available to admin level users. General users can't access it, so even if they're authenticated they the existing check would still fail.
Then I think the suggestion above could fix it, as long as the proper exception is caught.
If the plugin is not installed, this is no good. Doesn't trigger a warning in the UI and throws an error in the logs.
2024-05-17 10:08:32.685 T:3058698 info <general>: JELLYFIN.jellyfin_kodi.jellyfin.http -> ERROR::jellyfin_kodi/jellyfin/http.py:124 404 Client Error: Not Found for url: http://{jellyfin-server}/Jellyfin.Plugin.KodiSyncQueue/GetPluginSettings 2024-05-17 10:08:32.687 T:3058698 info <general>: JELLYFIN.jellyfin_kodi.library -> ERROR::jellyfin_kodi/library.py:386 (404, HTTPError('404 Client Error: Not Found for url: http://{jellyfin-server}/Jellyfin.Plugin.KodiSyncQueue/GetPluginSettings')) Traceback (most recent call last): requests.exceptions.HTTPError: 404 Client Error: Not Found for url: http://{jellyfin-server}/Jellyfin.Plugin.KodiSyncQueue/GetPluginSettings jellyfin_kodi.helper.exceptions.HTTPException: (404, HTTPError('404 Client Error: Not Found for url: http://{jellyfin-server}/Jellyfin.Plugin.KodiSyncQueue/GetPluginSettings'))
Not even hitting the
except requests.RequestException as e:
block so it can't proceed and just blows up. Even fails to establish a websocket connection after this.
Should've known that all the layers of requests abstraction would bite me here. Thanks for actually testing.
Properly catching the proper exception (which I naively assumed it already was) is the solution here.
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
No data about Coverage
1.9% Duplication on New Code
To determine whether the plugin is enabled or not.
Does not check if any of the items to track settings are enabled, but assumes at least one of them are.
Fixes #861