ayrat555 / frankenstein

Telegram bot API client for Rust
Do What The F*ck You Want To Public License
268 stars 28 forks source link

Stack Overflow on sending message #67

Closed amadare42 closed 2 years ago

amadare42 commented 2 years ago

Hey. I saw other stack-overflow issue, but it was related to different part of the code. I still have this issue reproducible on windows with Visual Studio toolchain (2022 Preview, rust version 1.61.0, Windows 10 21H2). API response serialization fails after sending message.

This is happening at telegram_api_impl.rs:51:

    pub fn decode_response<T: serde::de::DeserializeOwned>(response: Response) -> Result<T, Error> {
        match response.into_string() {
            Ok(message) => {
                let json_result: Result<T, serde_json::Error> = serde_json::from_str(&message); // < here

Here is (slightly modified) message that was causing this. It isn't even that large.

"{"ok":true,"result":{"message_id":101,"from":{"id":661885372,"is_bot":true,"first_name":"________________","username":"________________"},"chat":{"id":281814033,"first_name":"____","last_name":"__________","username":"_________","type":"private"},"date":1654353153,"reply_to_message":{"message_id":98,"from":{"id":281814033,"is_bot":false,"first_name":"____","last_name":"__________","username":"_________","language_code":"uk"},"chat":{"id":281814033,"first_name":"____","last_name":"__________","username":"_________","type":"private"},"date":1654352592,"text":"doom"},"text":" Doom \ud83d\uddd7  \nMain Story: 11\u00bd Hours \nMain + Extra: 16 Hours \nCompletionist: 26 Hours \n\nDoom \ud83d\uddd7  \nMain Story: 5 Hours \nMain + Extra: 7 Hours \nCompletionist: 9 Hours \n\nDoom Eternal \ud83d\uddd7  \nMain Story: 14 Hours \nMain + Extra: 19 Hours \nCompletionist: 26 Hours \n\nDoom II Hell on Earth \ud83d\uddd7  \nMain Story: 7 Hours \nMain + Extra: 9\u00bd Hours \nCompletionist: 14 Hours \n\nDoom 3 \ud83d\uddd7  \nMain Story: 11 Hours \nMain + Extra: 12\u00bd Hours \nCompletionist: 16 Hours","entities":[{"offset":0,"length":1,"type":"text_link","url":"https://howlongtobeat.com/games/doom_2016.jpg"},{"offset":1,"length":4,"type":"bold"},{"offset":6,"length":2,"type":"text_link","url":"https://howlongtobeat.com/game?id=2708"},{"offset":84,"length":4,"type":"bold"},{"offset":89,"length":2,"type":"text_link","url":"https://howlongtobeat.com/game?id=2701"},{"offset":163,"length":12,"type":"bold"},{"offset":176,"length":2,"type":"text_link","url":"https://howlongtobeat.com/game?id=57506"},{"offset":253,"length":21,"type":"bold"},{"offset":275,"length":2,"type":"text_link","url":"https://howlongtobeat.com/game?id=2711"},{"offset":351,"length":6,"type":"bold"},{"offset":358,"length":2,"type":"text_link","url":"https://howlongtobeat.com/game?id=2704"}]}}"

It looks like that json size may become pretty large if you have a lot of links, I would assume that relaying on having enough stack-size isn't ideal.

When running under my Debian WSL it is working fine, but I presume that larger messages may fail there as well.

ayrat555 commented 2 years ago

@amadare42 thanks for reporting. I'll work on this tomorrow.

I hoped https://github.com/ayrat555/frankenstein/commit/abab2128a4296a0aeec13f6820f1283a317a8c91 fixed the problem.

but probably we need to resolve https://github.com/ayrat555/frankenstein/issues/63

btw are you sure you're using the latest version of the library?

amadare42 commented 2 years ago

@ayrat555 sorry, I somehow forgot to mention version of library itself.

I was using version 0.15.0. Does 0.15.1 have fix for this issue?

ayrat555 commented 2 years ago

no, unfortunately, it doesn't

can you please also share the method and params that you used so I can try reproducing it

amadare42 commented 2 years ago

Here is branch with application where this issue reproducible: https://github.com/amadare42/rust-hltb-bot/tree/so-issue Steps are simple:

  1. Set API_KEY env parameter
  2. Send message "doom" to your bot
ayrat555 commented 2 years ago

@amadare42 I was able to reproduce the issue.

I created a pr which fixes the problem

can you please confirm that it works for you?

amadare42 commented 2 years ago

@ayrat555 yep, that fixed it - issue isn't reproducible for me anymore. Thanks!

ayrat555 commented 2 years ago

Great. Let's wait for @EdJoPaTo's review and then I'll merge it

amadare42 commented 2 years ago

@ayrat555 update: I managed to get SO on get_updates. :) I guess, adding serde-stacker there as well would be good idea as well. I updated my so-issue branch - steps are the same.

ayrat555 commented 2 years ago

I implemented another fix.

can you please try it?

amadare42 commented 2 years ago

@ayrat555 yep, now it is working. Thank you again! :)

ayrat555 commented 2 years ago

released in 0.16.0