Add timeout to HTTP requests#422
Conversation
Added timeout parameter to GET, POST, and DELETE requests to improve resiliency and prevent UI hangs. The most important change is for the POST request which is used to update playback state. If this POST request has no response with no timeout value, the Kodi video player UI will become unresponsive.
|
|
||
| r = requests.get(url, headers=self.headers, verify=self.verify_cert) | ||
| try: | ||
| r = requests.get(url, headers=self.headers, verify=self.verify_cert, timeout=5) |
There was a problem hiding this comment.
I have doubts that a 5 second timeout is enough for get requests. I've seen some calls (like music in the 10.11 release cycle) take upwards of 30. That's admittedly an outlier and is because the server has problems, but knowing how large some libraries get and how much data has to get processed for the api response, 5 seconds seems very short.
There was a problem hiding this comment.
what about changing the timeout to (5, 60) - 5s connect timeout and 60s read timeout. is that okay or do you suggest other values? should it be changed for the post and delete requests also?
There was a problem hiding this comment.
I would imagine 60s read for GET is sufficient. I don't think there's any real concern for posts and deletes needing anything extra
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
change timeout for get request to 5s connection timeout and 60s read timeout. changed except: to except Exception: for delete request, to avoid catching system exceptions.
Changed except: to except Exception: so that it doesn't catch system exceptions from BaseException, which could cause unintended stalling.
| requests.delete(url, headers=self.headers, verify=self.verify_cert) | ||
| try: | ||
| requests.delete(url, headers=self.headers, verify=self.verify_cert, timeout=5) | ||
| except Exception: |
3 participants
Added timeout parameter to GET, POST, and DELETE requests to improve resiliency and prevent UI hangs.
The most important change is to the POST request which is used to update playback state. If this POST request has no response with no timeout value, the Kodi video player UI will become unresponsive.
In Kodi, Jellycon hooks into the seek callback which is synchronous. So after a seek, if the Jellycon POST request to update playback state hangs, then the Kodi player UI will also hang.