Improve retry logic

[Malinskiy]

Current logic for http requests just retries immediately which leads to doing all the retries one after the other https://github.com/jellyfin/mopidy-jellyfin/blob/master/mopidy_jellyfin/http.py#L53

Context: I have an embedded box with mopidy + jellyfin starting during systemd boot. The problem is even though network.target is up the network is not really available at this point for mopidy-jellyfin, hence the HTTP connection might fail.

Jun 04 16:21:20 schiit mopidy[361]: ERROR    2023-06-04 16:21:20,039 [361:MainThread] mopidy_jellyfin.http
Jun 04 16:21:20 schiit mopidy[361]:   Failed to reach Jellyfin public API on try 0 with problem: HTTPSConnectionPool(host='jellyfin.home.XXX', port=443): Max retries exceeded with url: /system/info/public (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7f88235da590>: Failed to establish a new connection: [Errno -3] Temporary failure in name resolution'))
....
Jun 04 16:21:20 schiit mopidy[361]: ERROR    2023-06-04 16:21:20,046 [361:MainThread] mopidy_jellyfin.http
Jun 04 16:21:20 schiit mopidy[361]:   Failed to reach Jellyfin public API on try 5 with problem: HTTPSConnectionPool(host='jellyfin.home.XXX', port=443): Max retries exceeded with url: /system/info/public (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7f88235da590>: Failed to establish a new connection: [Errno -3] Temporary failure in name resolution'))

Suggestion: exponential backoff for temporary network failure

[veneficium42]

I've been trying to find the issue in my container setup for the last 2 hours and I just realized that even if mopidy starts after jellyfin it still sends the request before jellyfin is actually ready to receive it.
I had to restart mopidy from the running container to fix this issue but I would really like a way to retry the connection after mopidy has already started.

[sanzoghenzo]

Having this problem also with my setup, an always on jellyfin server and mopidy installed on my htpc. changing the systemd target to "network-online" doesn't solve the issue, so a better retry logic is needed.

I suggest to rely on the good work done by stamina to avoid reinventing the wheel.

If you're open to PRs, I can give it a stab.

[sanzoghenzo]

Sorry to bother you @mcarlton00, but I started to work on the PR and I noticed that most of the unit test are failing. It seems that a lot has changed but the tests were not updated accordingly.

I also see that this plugin still supports python 2.7, but it has been almost 6 year since mopidy 3 release. I believe it's time for a big cleanup! I would also use more modern tooling (uv, ruff linter and formatter to begin with) to simplify/speedup the development process.

I would like to help, but because of the aforementioned outdated tests (especially in test_remote.py) I wonder if the json files with fake payloads in the tests/data folder are outdated, too... Should I just ignore the tests?

[mcarlton00]

Sorry, serious lack of time to be too active in issues anymore, and can't even use this client right now because of upstream dependency issues breaking it on Arch systems. This is probably more suited for a general dev practices thread instead of here, or in chat, but to answer your main questions:

• Tests - literally have not been touched since I took over the project, for various reasons. Just consider them dead code
• Python 2.7 - "supports" is a very strong word here. There's multiple f-strings in the codebase now, so <python3 simply won't work at all. There's just some flags around imports pertaining to older versions that will never be hit and never got cleaned up.
• Modern tooling - This may just be me misunderstanding, but I genuinely don't see the point in chasing the shiny new tool for the sake of chasing the shiny new tool, and last time I tried one it literally wouldn't work since we need to use --system-site-packages when creating the venv so mopidy can have access to the low level audio libraries on the host. If you're really that passionate about it then sure I guess? But for this particular project it seems like jumping through a lot of hoops to reduce your setup time by 1 shell command, especially since a dev environment takes about 10 seconds from nothing to functional (speaking for uv in particular here).

As to the issue as a whole, I could've sworn I had changed the retry code here into something we do on other clients that just increases the backoff with each failure with an upper limit. From a quick search, looks like I may have only done that to the websocket connection and forgot to do it on the normal connection process. I guess we could replace that with a whole library, just kinda feels like driving a nail with a sledgehammer.

example:

mopidy-jellyfin/mopidy_jellyfin/ws_client.py

Lines 78 to 87 in 85c271f

	retry_count = 0
	while not self.stop:
	time.sleep(retry_count * 5)
	self.wsc.run_forever(ping_interval=10)
	# If connection fails, attempt to reconnect every 60 seconds at max
	max_tries = 12
	if retry_count < max_tries:
	retry_count += 1

[sanzoghenzo]

I will open a separate issue for the tooling part to not pollute this.

In the meantime, the reason for using a that particular library is right in its documentation.

But maybe it's just my tendency to not reinvent the wheel when somebody more clever than me already solved an issue I have 😅

[sanzoghenzo]

I realize only now that mopidy startup waits for the plugins initialization to complete before moving further.

It may be wise to also move all the http connections out of the intialization of the backend. skimming through the code, I believe that there are 2 points to handle

• JellyfinHandler.__init__ calls JellyfinHttpClient.check_redirect (the reason for this issue) and does the login. If the failure is not related to auth, it should be deferred and re-tried after the initialization completes
• JellyfinPlaylistProvider.__init__ calls refresh right away, which in turn calls methods of JellyfinHandler that need the http connection. I'm not a mopidy expert so I don't know if this can be moved and called after the initialization.

There is an old issue on the mopidy repo that talks about avoiding long extensions startup time, but I didn't get anything actionable from it.

Also worth mentioning, version 4.0 is in the making, and they're talking about/experimenting on async loops... maybe #156 can wait until we have the new mopidy version!

[oddstr13]

Absolutely no reason to bring in an external library for connection back-off.
It is not a very complex task, and shouldn't take more than a handful of lines to implement in a utility function.

I refer you to this abomination: https://www.npmjs.com/package/is-even

[sanzoghenzo]

Absolutely no reason to bring in an external library for connection back-off. It is not a very complex task, and shouldn't take more than a handful of lines to implement in a utility function.

Fair enough... from a quick look at it, indeed stamina seems overengineered.
It's my impostor syndrome + lazyness that makes me fall into the trap of calling it a day with a cli command and some copy-pasting from the docs of someone (in this case, the creator of attrs and structlog, among other projects) I believe is much more knowledgeable than me!

I refer you to this abomination: https://www.npmjs.com/package/is-even

Even better, today my colleague pointed me to this 🤣 https://github.com/jezen/is-thirteen

---

In the meantime, I solved my problem at the systemd level: the After=network-online.target wasn'tworking because I didn't have network nor networkmanager enabled, and even after enabling networkd I had to add a loop to check the ping to my media server (ironically, with a fixed 5 second sleep in a loop) in the ExecStartPre. I'll share the exact config when I'll get back to my pc