TelegramBots / Telegram.Bot

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

Add catch block for Deserialization issues #503

Closed Vodolazz closed 6 years ago

Vodolazz commented 6 years ago

https://github.com/TelegramBots/telegram.bot/blob/578fb246a9576e77c18a48049f7308b2e80c41d8/src/Telegram.Bot/TelegramBotClient.cs#L2242

When server returns unexpected response in body, Json deserializer fails to deserialize it and throws exception, which is not very even from the outside of the library. Suggest to catch deserializer exception and throw something meaningful instead, like 'unexpected response' or similar.

poulad commented 6 years ago

@Vodolazz tg client catches the expected exceptions only. A few lines above that, it checks for status code of the response (as per tg's defined contract in api docs) and only after that starts deserialization.

Any other kind of exception such as in network failures or improper JSON returned from tg server(or even a proxy on the way) should directly be thrown up to the caller.

poulad commented 6 years ago

@Vodolazz Anyways, what invalid JSON have you received from the server? Please share it. I never saw this issue.

Vodolazz commented 6 years ago

ok, it's up to you. for me it wasn't just obvious from the first glance that serialization exception necesserally means unexpected answer. Had to look through the source code to investigate issue. regarding your second question, there were some unexpected symbols like '<', I believe that was xml/html body. Unfortunately I do not have complete message at the moment.

Vodolazz commented 6 years ago

By the way, regarding status code message handling. actually meaningful exception on bad status code is thrown after the serialization attempt. I believe that can be a source of the problem. Lines 2235 - 2256.

poulad commented 6 years ago

@Vodolazz can you confirm what version you were exactly using when you got such error? I have changed that deserialization in recent relrases.