mirror of
https://github.com/sigma67/ytmusicapi.git
synced 2026-04-26 07:46:00 +03:00
[GH-ISSUE #221] Session hangs indefinitely #174
Labels
No labels
a/b
bug
documentation
enhancement
good first issue
help wanted
invalid
pull-request
question
wontfix
yt-error
yt-update
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/ytmusicapi#174
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @colbeyhair on GitHub (Aug 20, 2021).
Original GitHub issue: https://github.com/sigma67/ytmusicapi/issues/221
I have found that while using this API in programs that run for a long time, the program will sometimes hang up indefinitely (for several days at least).
When the program hangs up, sending a ctrl+c sigkill results in a traceback showing that the program was in the middle of a session request, specifically in the
get_playlists()method. (Unfortunately I lost the traceback and cannot copy it here).The requests documentation page recommends passing a timeout parameter to all session instances to avoid such an indefinite hangup: https://docs.python-requests.org/en/master/user/quickstart/#timeouts
Such a parameter is not passed when the
requests.Session()is initialized here: https://github.com/sigma67/ytmusicapi/blob/master/ytmusicapi/ytmusic.py#L62I think that adding a timeout of a few seconds should allow the program to timeout when appropriate and avoid hangups. Perhaps the timeout can be a parameter passed when instantiating ytmusic if adjustability is desired.
@sigma67 commented on GitHub (Aug 20, 2021):
Good point, I didn't have this issue yet so I never noticed. The primary concern that could be affected negatively by a timeout would be
upload_song, but that currently doesn't use the session anyway.Since the primary concern is to avoid rare indefinite hangups, we could set a conservative value. Do you think 30 seconds is appropriate? I don't think we need to make it a parameter if it only happens rarely. Besides, it is already possible to pass a requests Session object as the
requests_sessionparameter, which is then used by ytmusicapi.Additionally, requests doesn't really support timeouts on a session level, you need to modify the session object like this: https://github.com/psf/requests/issues/2011#issuecomment-490050252
@colbeyhair commented on GitHub (Aug 20, 2021):
I agree! A conservative value makes sense to avoid the hangups, and since it is only to keep a program moving in rare cases, a long timeout seems reasonable as a default value.
However, I think it would be very useful to have a timeout argument in the ytmusic constructor. If it happens that I have a bad internet connection and I get timeouts frequently, it could be very useful to pass a parameter from my user code so I can set the timeout as short as possible without interfering with normal request operations. That way, my program wouldn't have to wait a full 30s before raising exception for me to try again.
Having any timeout, even hard-coded, would fix the issue I'm observing. Having a variable timeout would just be "nice to have"
@sigma67 commented on GitHub (Aug 20, 2021):
Setting a timeout is possible right now, it's a two-liner (which is what I was trying to say above). Here's an example for a 3 second timeout, feel free to try it out:
See also the docs
I think that should cover the variable timeout part.
@colbeyhair commented on GitHub (Aug 22, 2021):
Thanks for the suggestion, sigma! I have tried the above recommendation (with a change to the YTMusic argument to avoid a kwarg typo):
It took me a while to get back to you because I wanted to try your solution pretty thoroughly. I was able to capture the following exception and traceback when the session inevitably timed out:
I am able to successfully handle this exception:
@sigma67 commented on GitHub (Aug 26, 2021):
That's great, thanks for the detailed feedback. Will add some documentation and the default timeout.