TelegramBots / Telegram.Bot

.NET Client for Telegram Bot API
https://telegrambots.github.io/book
MIT License
3.22k stars 690 forks source link

Add exception type for too many requests #628

Closed tuscen closed 5 years ago

tuscen commented 6 years ago

HTTP Code: 429 Error message: Too Many Requests: retry after [0-9]+

poulad commented 6 years ago

What's HTTP status code?

karb0f0s commented 6 years ago

429?

tuscen commented 6 years ago

@karb0f0s I added it after @pouladpld had asked :) Forgot about status code when I opened the issue.

poulad commented 6 years ago

image

karb0f0s commented 6 years ago

@pouladpld @Tuscen any suggestions on this commit Add TooManyRequestsException

karb0f0s commented 6 years ago

wow here is a story - 2 years to add new http error codes: https://github.com/dotnet/corefx/issues/4382

karb0f0s commented 6 years ago

BTW I think constants should be replaced with HttpStatusCode - either exception class accept HttpStatusCode, or error code will be cast to int? https://github.com/TelegramBots/Telegram.Bot/blob/ce94da88aecb870b7e7a263d2f0e50da82beaba0/src/Telegram.Bot/TelegramBotClient.cs#L222-L225 https://github.com/TelegramBots/Telegram.Bot/blob/ce94da88aecb870b7e7a263d2f0e50da82beaba0/src/Telegram.Bot/TelegramBotClient.cs#L243-L245 https://github.com/TelegramBots/Telegram.Bot/blob/ce94da88aecb870b7e7a263d2f0e50da82beaba0/src/Telegram.Bot/TelegramBotClient.cs#L198 https://github.com/TelegramBots/Telegram.Bot/blob/ce94da88aecb870b7e7a263d2f0e50da82beaba0/src/Telegram.Bot/TelegramBotClient.cs#L297

Maybe the same should be made here https://github.com/TelegramBots/Telegram.Bot/blob/ce94da88aecb870b7e7a263d2f0e50da82beaba0/src/Telegram.Bot/Exceptions/BadRequestException.cs#L10 and here https://github.com/TelegramBots/Telegram.Bot/blob/ce94da88aecb870b7e7a263d2f0e50da82beaba0/src/Telegram.Bot/Exceptions/ForbiddenException.cs#L10 bcs at first there is a check for HttpStatusCode and after comes some constants

matteocontrini commented 6 years ago

@karb0f0s is that integration test working? Last time I asked to BotSupport they said that when sending messages to groups the 429 reply could actually never come because they wait for small flood delays and then handle the request. Here is a part of my conversation with them. This was in September.

Me:

1) it seems that when reaching the limit of 20 messages per minute per group, the Bot API starts rejecting the requests instead of sending back 429. I'm getting ESOCKETTIMEDOUT in a Node.js app when I go over that limit. I expect 429 to be returned so that I can filter the type of response and decide what to do 2) when I reach the limit above and I start getting ESOCKETTIMEDOUT, it seems that I should not retry the requests anymore because they're queued on your end? Is that correct? I've seen messages being sent to a group I was testing with in batches of 5 for some time after I killed the script that was sending the messages

Support:

I can confirm it, requests are waiting for small flood delays on our side. Socket timeout should be set to at least 65 seconds to always receive an answer and don't receive ESOCKETTIMEDOUT ;)

tuscen commented 6 years ago

@matteocontrini I tried to get 429 in tests but I couldn't. Now I know why :)

karb0f0s commented 6 years ago

@matteocontrini it worked yesterday. i send request in parallel, it breaks flood protection i guess

karb0f0s commented 6 years ago

should this exception have parameter reflecting retry delay?

tuscen commented 6 years ago

@karb0f0s retry delay should be inside ApiException.ResponseParameters.RetryAfter property.

karb0f0s commented 6 years ago

throw TooManyRequestsException with with apiResponse.Parameters