TelegramBots / Telegram.Bot

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

TestApiAsync shouldn't catch 404 #622

Closed tuscen closed 6 years ago

tuscen commented 6 years ago

Bot API returns 404 on incorrectly formatted tokens. And since TelegramBotClient can't be instantiated with incorrectly formatted token MakeRequestAsync will throw401 Invalid Token. So catching 404 doesn't make sense here.

karb0f0s commented 6 years ago

https://github.com/TelegramBots/Telegram.Bot/blob/ed21285554ea3720cc70fcd8298e4cc2eeededbb/src/Telegram.Bot/TelegramBotClient.cs#L270-L285 GetMeAsync() will throw ApiRequestException("Invalid token", 401) here https://github.com/TelegramBots/Telegram.Bot/blob/ed21285554ea3720cc70fcd8298e4cc2eeededbb/src/Telegram.Bot/TelegramBotClient.cs#L243-L245

karb0f0s commented 6 years ago

@tuscen @pouladpld I did some discovery:

  1. Should_Fail_Test_Api_Token test is unreliable. Replacing bot token "0:1this_is_an-invalid-token_for_tests" with the one provided in Telegram Bot API examples 123456:ABC-DEF1234ghIkl-zyx57W2v1u123ew11 (or changing few characters in real bot token, same result will be for revoked token i guess) breaks the test - it fails because MakeRequestAsync trows new ApiRequestException("Invalid token", 401) https://github.com/TelegramBots/Telegram.Bot/blob/ed21285554ea3720cc70fcd8298e4cc2eeededbb/test/Telegram.Bot.Tests.Integ/Getting%20Updates/GettingUpdatesTests.cs#L33-L43
  2. We should not check provided token in constructor TelegramBotClient(string token, HttpClient httpClient = null) and TelegramBotClient(string token, IWebProxy webProxy). First of all - specified regex Regex.IsMatch(token, @"^\d*:[\w\d-_]{35}$") is incorrect - should be escaped, and second - there is no actual token specification provided in official Telegram Bot API documentation. I think we should leave this check to Telegram.
tuscen commented 6 years ago

@karb0f0s Valid point. Token form is not specified anywhere so it shouldn't be present in the library. In this case catching 404 will be valid in TestApiAsync.

karb0f0s commented 6 years ago

Wrapping up our discussion: