Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TimeOut Cant Set int.MaxValue #8

Closed
wuyu8512 opened this issue Jul 18, 2021 · 5 comments
Closed

TimeOut Cant Set int.MaxValue #8

wuyu8512 opened this issue Jul 18, 2021 · 5 comments

Comments

@wuyu8512
Copy link

wuyu8512 commented Jul 18, 2021

Consider changing int to TimeSpan?

@AmanAgnihotri
Copy link
Member

AmanAgnihotri commented Jul 20, 2021

When you talk about the timeout value, I imagine you mean the Timeout property made available by the BotConfig class.

I made this property an int type so as to make it configurable from the appsettings.json or any other configuration file that IConfiguration can load as a section.

Even the HttpClient's internal implementation is such that 2147483 is the maximum integer timeout (in seconds) that you can setup with BotClient which amounts to some 24.85 days. Another alternative for HttpClient is the infinite timeout but I personally do not think it would be wise to use that as it will block the HttpClient forever should the upstream service never responds. Telegram anyway times out the bot requests, if I recall correctly. At least that was my experience with GetUpdates based requests during polling.

Personally, I think the 90 seconds default timeout value is reasonable. You can even set it up as 86400 seconds or anything equivalent to 24.85 days. That should be more than enough for any need you may have.

Why would you want to setup an int.MaxValue based timeout anyway? HttpClient will fail there as it imposes a limit on its maximum TimeSpan unless the TimeSpan denotes infinite time.

I would like to know some good use case for this that you may have for which even 24 days are not enough for a timeout.

@wuyu8512
Copy link
Author

Hi, thank you very much for your answers, in fact, I will upload some large files, but the speed of uploading to server using bot api(--local) is very slow (this may be my problem), so I would like to have a longer time to wait, but Actually it will not exceed 24 days

Even the HttpClient's internal implementation is such

I'm glad to hear this detail,I think this question is actually irrelevant, maybe we can set it to System.Threading.Timeout.Infinite to represent infinite waiting

@AmanAgnihotri
Copy link
Member

Unless you are using the local Telegram Bot Server, the upload limit for a bot using the HTTP Bot API is 50 MB and the download limit for the same is 20 MB. If you are using the local Telegram Bot Server, the upload limit becomes 2000 MB and the download limit does not exist (and I guess that'd be practically 2000 MB only).

I will caution against the use of Infinite timeout for various reasons.

Even 2000 MB limit should sit well with timeout set as 2147483 seconds which is the maximum integer you can go in HttpClient. With a 10 kbps connection, 2000 MB will take like 18.5 days to send which is under 24 days.

You will have to re-implement the AddBotClient extension method for your very specific use-case to setup Infinite timeout. Doable but need not be done given your situation.

I would like to keep the Timeout property as an int to allow for its setup through appsettings.json itself.

@wuyu8512
Copy link
Author

wuyu8512 commented Jul 20, 2021

If you are using the local Telegram Bot Server

Of course

I would like to keep the Timeout property as an int to allow for its setup through appsettings.json itself.

System.Threading.Timeout.Infinite is an integer constant, value is -1,I want to say that if the user sets it -1, then it is set System.Threading.Timeout.InfiniteTimeSpan

@AmanAgnihotri
Copy link
Member

AmanAgnihotri commented Jul 20, 2021

Ah, I did not know that System.Threading.Timeout.Infinite is equivalent to the -1 value. Unfortunately, I tried setting -1 as the value of the Timeout property and the program still blew up. After digging a bit, I figured that it is just as possible and perhaps even more readable to setup TimeSpan in likes of appsettings.json.

Here is an article which shows how to do that:

TimeSpan configuration values in .NET Core by Mark Seeman

Since this meets my requirement of allowing timeouts to be configurable from appsettings.json file, I set the Timeout property to TimeSpan type in the following commit:

Make Timeout a TimeSpan Type

Since this way one can also setup the timeout to an InfiniteTimeSpan which was not possible before as -1 and InfiniteTimeSpan are not equivalent for the HttpClient which complained for -1 timeout value.

I also made this available as a NuGet package.

Thank you for your suggestion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants