protoncr / tourmaline

Simple Telegram bot library for Crystal
https://tourmaline.dev
MIT License
158 stars 38 forks source link

Command parsing are broken #60

Closed AskarZinurov closed 1 year ago

AskarZinurov commented 1 year ago

Hi! First of all thanks for your work, it is really pleasure to play with such well organized lib.

I took the bot from echo example and trying to get response for echo command, but nothing happens. I had debugged until this line, the handler execution breaks here. Seems the last major changes with messages parsing had broke the feature. The commands are not parsed in both webhook and polling modes.

I had tried the previous release v0.25.1 and it is parsing commands correctly.

watzon commented 1 year ago

Could you give me an example of something that's not working? Also did you install master or the latest released version. Master seems to be functioning correctly for me, so I'm trying to figure out what the difference is between what I'm running and what you're running.

For instance, I:

After all those steps, the /echo command seems to work fine, as well as the /echo@BotName version.

AskarZinurov commented 1 year ago

Hmm, I have done exactly same steps. Maybe problem in crystal version or platform?

crystal -v
Crystal 1.6.2 (2022-11-03)

LLVM: 14.0.6
Default target: aarch64-apple-darwin21.6.0

I tried again sending commands from both web and client apps, the problem still exists. This is only way I was able to handle commands. As I understand my changes is a kind of rollback and maybe they are bringing back some bug.

watzon commented 1 year ago

Hmm it could be Crystal's compatibility with the M series chips or just macOS itself. I'll try and look into the possibility of a platform specific issue. If you manage to find a fix besides just rolling back that works please let me know.

etra0 commented 1 year ago

Hmm, I did the same steps and I hit the same issue. I can confirm that @AskarZinurov patch fixes the issue for me.

My platform is the following one:

Crystal 1.7.2 (2023-01-23)

LLVM: 14.0.6
Default target: aarch64-apple-darwin21.6.0

it might be the regex library that could be broken?

etra0 commented 1 year ago

I can confirm that this works in my linux machine but not in my M1... weird

Crystal 1.7.1 (2023-01-19)

LLVM: 14.0.6
Default target: x86_64-pc-linux-gnu
watzon commented 1 year ago

Interesting, so it's probably a Crystal bug on M1 Macs

etra0 commented 1 year ago

Doing further research, it seems that the raw_text gets filled with gibberish in macOS,

From my linux machine I get

(gdb) p/x (raw_text->c)@10
$4 = {0x2f, 0x65, 0x63, 0x68, 0x6f, 0x20, 0x68, 0x6f, 0x6c, 0x61}
(gdb) p &raw_text->c
$5 = (UInt8 *) 0x7ffff30fcfac "/echo hola "

and from my M1:

(lldb) parray 10 &raw_text->c 
(unsigned char *) $3 = 0x0000000100f419bc "⼀攀挀栀漀 栀漀氀愀 " {
  (unsigned char) [0] = '\xe2'
  (unsigned char) [1] = '\xbc'
  (unsigned char) [2] = '\x80'
  (unsigned char) [3] = '\xe6'
  (unsigned char) [4] = '\x94'
  (unsigned char) [5] = '\x80'
  (unsigned char) [6] = '\xe6'
  (unsigned char) [7] = '\x8c'
  (unsigned char) [8] = '\x80'
  (unsigned char) [9] = '\xe6'
}
etra0 commented 1 year ago

Ok, apparently, changing

https://github.com/protoncr/tourmaline/blob/0b5dfec0dbaee2968e51ea6323770f5f6ff68670/src/tourmaline/helpers.cr#L84

to

- io.set_encoding("UTF-16") 
+ io.set_encoding("UTF-8") 

also fixes it in the M1, but I'm unsure if it would cause any other issues. Thoughts?

watzon commented 1 year ago

Hmm yeah, that's needed if you're using that helper. Telegram uses UTF-16

etra0 commented 1 year ago

I managed to create a minimal repro and it is in fact an issue with Crystal on macOS, as you can check in here https://github.com/crystal-lang/crystal/issues/13029.

Hopefully it'll be fixed soon enough :D

EDIT: Nevermind, lol. The correct fix was to actually specify we're using Little endian UTF-16. I'd expect for crystal using the default in both platforms, but nevertheless, the PR should be fix the issue.