Closed t-n-u-z closed 1 year ago
Just to let you know: I use this patch for a month now and it works fine. Because my bot does not use files, I have not yet tested how the timeout affects a file upload. I will test it hopefully soon.
To calculate the timeout, I use the walrus operator. This would increase the minimum version to Python 3.8. If the compatibility with 3.7 is still desired, I would have to rewrite it accordingly.
Me thinks it's better to do this without walrus operator. Not just because of backwards compatibility, it is also pretty hard to make out what is going on. Maybe like this:
timeout_connect = 5
timeout_read = None
dt = data.get("timeout", 0)
if dt > 0:
timeout_read = dt + timeout_connect
timeout = (timeout_connect, timeout_read)
You are right. The spelling with the walrus operator is not so reader-friendly. I will adjust it.
In the meantime I have tested the timeout when uploading files. It does not behave as expected. The connect timeout triggers before the file is completely uploaded. It seems to be difficult to handle write timeouts with the request module. Therefore I will remove the timeout for the file upload again.
See also https://stackoverflow.com/questions/70612906/python-requests-post-timeout-specification-with-large-files and https://stackoverflow.com/questions/63992667/python-error-with-request-post-connection-aborted-timeoutthe-write-operati
I am running OrigamiBot on my homeserver and the ISP forces a reconnect every 24 hours. After that the bot does not receive updates/new messages anymore. To reproduce this you can also manually disconnect and reconnect the internet connection e.g. by unplugging the LAN cable.
I think that the request (long polling) is still waiting for an answer but because of the short internet lack the polling waits endless.
Probably it is enough to set a timeout for the requests. I will try it and if it works I will make a pull request.
Nevertheless it is always useful to set a timeout for requests. See https://dev.to/bearer/the-importance-of-request-timeouts-l3n