Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sendPhoto silently does nothing? #137

Closed
lawik opened this issue Sep 23, 2022 · 12 comments · Fixed by #138
Closed

sendPhoto silently does nothing? #137

lawik opened this issue Sep 23, 2022 · 12 comments · Fixed by #138

Comments

@lawik
Copy link

lawik commented Sep 23, 2022

Hey

Playing around with a bot and trying to send a Photo, the multipart request just never completes. I get the log line before but never the one after.

@visciang
Copy link
Owner

visciang commented Sep 24, 2022

Hi @lawik,

I've pushed branch ISSUE-137, it extend the example/example_bot.exs with a command to send you back a photo.
Just tested right now and "it works on my machine"!

Could you play there and check again?

Start the bot from that branch:

~/Code/telegram ISSUE-137
❯ BOT_TOKEN=___YOUR_BOT_TOKEN___ ./example/example_bot.exs

then from a telegram client send to your bot the command /send_photo.
You should get back the telegram logo without notification sound.

If you can pin the variables that give you the issue will be easier to backtrack the root cause.

@visciang
Copy link
Owner

visciang commented Sep 25, 2022

Some constraints on photos are described here (max 10 MB, ...) https://core.telegram.org/bots/api#sendphoto
Anyway, a HTTP request (multipart or not) should end soon or later, even more considering:

    @gun_config Application.compile_env(:telegram, :gun_config,
                  timeout: 60_000,
                  connect_timeout: 5_000,
                  certificates_verification: true)

@lawik
Copy link
Author

lawik commented Sep 26, 2022

Did some fun debugging. My images were slightly bigger, 211Kb and up. Not big by any means but bigger than 6.3Kb that you have for testing. So yours is just under 64Kb :)

I tried padding your image with 1Kb of nulls until a send failed.

I was okay sending 64745 bytes.
I failed sending 65769 bytes.

@lawik
Copy link
Author

lawik commented Sep 26, 2022

So yes, your example works. No other image I have will :D

@lawik
Copy link
Author

lawik commented Sep 26, 2022

Oh, and it never returns. It idles for 30 seconds and then times out and retries.

@visciang
Copy link
Owner

Ahh!
I would like to play with the tesla Telegram.Client adapter.
Currently it uses gun and the Retry middleware (with default opts)

@lawik
Copy link
Author

lawik commented Sep 26, 2022

It also happens with both {:file_content, binary, name} and {:file, path} variants which was surprising.

I just tried switching the entire client to Hackney. Went through, no issue. So this is probably an issue with Gun then.

Seems similar to this: elixir-tesla/tesla#230

@visciang
Copy link
Owner

Exactly what I expected!
Maybe more close elixir-tesla/tesla#394.

Honestly, the HTTP side of things is really horrible Erlang/Elixir side.
(even more now, when 99% of the beam app find application in the web and the 5G inter-NF is HTTP based).

@lawik
Copy link
Author

lawik commented Sep 26, 2022

Well found! That is the exact issue but not the same HTTP client.
I haven't had many problems with the HTTP-clients but I'll take your word for it :)

@lawik
Copy link
Author

lawik commented Sep 26, 2022

Is there a particular reason this library configures Gun and not another HTTP library? Can it be overridden with config?

I don't have time to figure out the Tesla problem but switching to Hackney seems to work fine.

@visciang
Copy link
Owner

No particular reason.

The tesa client adapter and its config could be made configurable, ie the library user can choose what to use or just leave the default (hackney).

And, by the way, the need to have an HTTP client stays in the pooler, that should be used in dev mode or if serving small amount of traffic. Another way to say it doesn't matter that much what client you use unless you have strong dependency requirements.

@visciang
Copy link
Owner

@lawik could you give a quick review to #138 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants