Skip to content

Handle flood control, poller timeouts, and answer via webhook response - #274

Open
anko20094 wants to merge 1 commit into
telegram-bot-rb:masterfrom
anko20094:webhook-hardening
Open

anko20094 wants to merge 1 commit into
telegram-bot-rb:masterfrom
anko20094:webhook-hardening

Conversation

@anko20094

Copy link
Copy Markdown
Contributor

Summary

A few independent robustness/feature improvements, happy to split into separate PRs if that's easier to review:

  • 429 / flood control: error_for_response only special-cased 403/404, so a 429 flood-control response just raised a generic Error and silently dropped parameters.retry_after. Added Telegram::Bot::TooManyRequests < Error with a retry_after reader, raised for 429 responses.
  • UpdatesPoller crash on timeout (HTTPClient::ReceiveTimeoutError: execution expired #245): fetch_updates only rescued Timeout::Error, but HTTPClient::ReceiveTimeoutError (what actually gets raised on a stalled long-poll connection) doesn't inherit from it, so the poller loop died instead of just retrying. Widened the rescue to HTTPClient::TimeoutError as well. This is safe to retry unconditionally: get_updates is a long poll keyed by offset, so a dropped connection can't cause duplicate processing, unlike most other bot methods (sendMessage etc.), which I deliberately left alone since Telegram gives no idempotency guarantee on those - blindly retrying those on a timeout could double-send.
  • Log filtering (UpdatesController::LogSubscriber should obfuscate sensitive information #239): LogSubscriber logged the full update verbatim, including message text. Added LogSubscriber.filtered_parameters (via ActiveSupport::ParameterFilter, defaults to %i[text]) so text is redacted by default; assign [] (or your own list) to change that.
  • Answer directly in the webhook response (Reply to Telegram requests directly in webhook mode #59): added via_webhook: true to all the reply helpers (respond_with, reply_with, answer_inline_query, answer_callback_query, answer_pre_checkout_query, answer_shipping_query, edit_message), implementing https://core.telegram.org/bots/faq#how-can-i-make-requests-in-response-to-updates. When set and the controller is running in webhook mode, the call is encoded as {method: ..., ...params} and handed to Middleware to return directly as the webhook HTTP response body, instead of making a separate API request - saves a request and helps with rate limits. Only the first such call in a given update takes effect (subsequent ones, and any call at all in poller mode, fall back to a normal API call), since Telegram only lets you answer once this way. I went with an explicit opt-in flag per call rather than trying to auto-detect "the last call in the action", since with multiple calls in one action (e.g. answer_callback_query + edit_message) there's no way to know upfront which one should get the free ride.

Middleware#call unconditionally returned [200, {}, ['']] before; now it returns whatever dispatch claimed as the webhook response, without changing .dispatch's own return value (some specs, e.g. Session, rely on it echoing the action's return value).

Test plan

  • bundle exec rspec - 364 examples, 0 failures
  • bundle exec rubocop - no offenses
  • Manually exercised via_webhook: true end-to-end through Middleware#call with a real ActionDispatch::Request/Rack env, confirmed no API call is made and the JSON body matches what Telegram expects
  • Manually confirmed LogSubscriber redacts text by default

- Raise Telegram::Bot::TooManyRequests (with retry_after) for 429 responses
  instead of a generic Error, so callers can back off correctly.
- Fix UpdatesPoller crashing on HTTPClient::TimeoutError: it only rescued
  Timeout::Error, which HTTPClient's own timeout errors don't inherit from.
  get_updates is a long poll and safe to retry, since a dropped connection
  can't cause duplicate processing.
- Filter `text` out of UpdatesController::LogSubscriber logs by default, to
  avoid leaking users' message content into logs. Configurable via
  LogSubscriber.filtered_parameters=.
- Add `via_webhook: true` to the reply helpers (respond_with, reply_with,
  answer_inline_query, answer_callback_query, answer_pre_checkout_query,
  answer_shipping_query, edit_message) to answer directly in the webhook
  HTTP response instead of making a separate API call, as described in
  https://core.telegram.org/bots/faq#how-can-i-make-requests-in-response-to-updates.
  Only the first such call per update takes effect; everything else (and any
  call at all outside webhook mode) falls back to a regular API call.
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 this pull request may close these issues.

1 participant