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

Reply to Telegram requests directly in webhook mode #59

Open
zhulik opened this issue Jan 5, 2018 · 7 comments
Open

Reply to Telegram requests directly in webhook mode #59

zhulik opened this issue Jan 5, 2018 · 7 comments

Comments

@zhulik
Copy link

zhulik commented Jan 5, 2018

Due to official FAQ https://core.telegram.org/bots/faq#how-can-i-make-requests-in-response-to-updates, it's possible to reply direcly to Telegram requests in webhook mode. It's a preferrable way for highly-loadded bots because of reducing API calls.

As I can see, there is no way to work in such mode with your library.

I suggest to implement render method in UpdatesController and allow users to respond to Telegram requests direcly.
It will solve few problems:

  • Reduce overall count of requests to Telegram API(do you remember about 429?)
  • Allow users to move bot logic from controller to their own handlers/classes/etc without pain and test it the their own manner

In poller mode render method can work as respond_with and use API call

@zhulik zhulik changed the title Answer to Telegram request in webhook mode Reply to Telegram requests directly in webhook mode Jan 5, 2018
@printercu
Copy link
Member

Does it support multiple responses? It's usual to make 2 api requests for callback queries: edit message with new content and telegram requires answer_callback_query to stop showing spinner.

It's also not clear how this should work when poller fetches multiple updates at a time. Do you have a link to detailed explanation of "Reply directly and give method as JSON payload in the reply"?

@zhulik
Copy link
Author

zhulik commented Jan 5, 2018

It does not support multiple responses, so sometimes it's needed to use API. But, it allows to overcome telegram API restrictions such as 429 responses.

About callback_queries: spinner is hiding when you update the message, so the answer_callback_query isn't required.
UPD: seems like it's platform-dependent, I still see the spinners on my Android phone, but they are hiding on my Linux laptop

For poller mode there is no difference if it fetches multiple updates, render should just be mapped to respond_with.

Example can be found in FAQ: https://core.telegram.org/bots/samples/hellobot

@printercu
Copy link
Member

printercu commented Jan 5, 2018

For poller mode there is no difference

Yep, I've messed all together - it's not related to poller mode where multiple updates may appear within single request.

For now I don't want to keep separate apis for responding within and outside webhook, I'll think more about it. I think about some wrapper like webhook_response { bot.some_action(...) } which will also work with all existing helpers: webhook_response { respond_with :message, text: '...' }. This also may be implemented smart, so it would respond to webhook with one of multiple actions and make api calls for the rest.

I hope you can use this small patch to give this feature a try in production and share feedback:

# in controller
def dispatch
  super
  @_response
end

protected def render(result)
  @_response = result.to_json
end

# sample usage
def smth(*)
  render method: :sendMessage, text: '....'
  # bot.any_additional_api_calls
end

# and modified middleware
Telegram::Bot::Middleware.class_eval do
  def call(env)
    request = ActionDispatch::Request.new(env)
    update = request.request_parameters
    response = controller.dispatch(bot, update)
    if response
      [200, {'Content-Type' => 'application/json'}, [response]]      
    else
      [200, {}, ['']]
    end
  end
end

UPD. updated middleware to set content-type for json responses.

@zhulik
Copy link
Author

zhulik commented Jan 5, 2018

Thank you, will try ASAP

@printercu
Copy link
Member

I've updated middleware in my snippet, please use updated version.

@printercu
Copy link
Member

printercu commented Mar 4, 2018

Have you tried suggested solution? I appreciate any feedback on responses to http hooks (issues in production, limitations, etc.).

@zhulik
Copy link
Author

zhulik commented Mar 4, 2018

Oh, sorry, I have no time to check it=(

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

No branches or pull requests

2 participants