From cbbfc5b3d1dfcec06fbafabb6d57d01bfe772379 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Tue, 31 Jan 2023 13:25:58 +0400 Subject: [PATCH] Rewrite hacks for reliable `Update` deserialization --- crates/teloxide-core/CHANGELOG.md | 3 + crates/teloxide-core/src/bot.rs | 6 +- crates/teloxide-core/src/net/request.rs | 46 ++++++- crates/teloxide-core/src/types/update.rs | 126 +++++++----------- .../src/update_listeners/webhooks/axum.rs | 12 +- 5 files changed, 108 insertions(+), 85 deletions(-) diff --git a/crates/teloxide-core/CHANGELOG.md b/crates/teloxide-core/CHANGELOG.md index eab2daed..dadb1ec3 100644 --- a/crates/teloxide-core/CHANGELOG.md +++ b/crates/teloxide-core/CHANGELOG.md @@ -10,8 +10,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - `Update::user` now handles channel posts, chat member changes and chat join request updates correctly ([#835][pr835]) +- In cases when `teloxide` can't deserialize an update, error now includes the full json value ([#826][pr826]) + [pr835]: https://github.com/teloxide/teloxide/pull/835 +[pr826]: https://github.com/teloxide/teloxide/pull/826 ## 0.9.0 - 2023-01-17 diff --git a/crates/teloxide-core/src/bot.rs b/crates/teloxide-core/src/bot.rs index deea3566..65f2770c 100644 --- a/crates/teloxide-core/src/bot.rs +++ b/crates/teloxide-core/src/bot.rs @@ -206,7 +206,7 @@ impl Bot { ) -> impl Future> + 'static where P: Payload + Serialize, - P::Output: DeserializeOwned, + P::Output: DeserializeOwned + 'static, { let client = self.client.clone(); let token = Arc::clone(&self.token); @@ -237,7 +237,7 @@ impl Bot { ) -> impl Future> where P: MultipartPayload + Serialize, - P::Output: DeserializeOwned, + P::Output: DeserializeOwned + 'static, { let client = self.client.clone(); let token = Arc::clone(&self.token); @@ -267,7 +267,7 @@ impl Bot { ) -> impl Future> where P: MultipartPayload + Serialize, - P::Output: DeserializeOwned, + P::Output: DeserializeOwned + 'static, { let client = self.client.clone(); let token = Arc::clone(&self.token); diff --git a/crates/teloxide-core/src/net/request.rs b/crates/teloxide-core/src/net/request.rs index 7fa345ec..7e4ca1bb 100644 --- a/crates/teloxide-core/src/net/request.rs +++ b/crates/teloxide-core/src/net/request.rs @@ -1,4 +1,4 @@ -use std::time::Duration; +use std::{any::TypeId, time::Duration}; use reqwest::{ header::{HeaderValue, CONTENT_TYPE}, @@ -19,7 +19,7 @@ pub async fn request_multipart( _timeout_hint: Option, ) -> ResponseResult where - T: DeserializeOwned, + T: DeserializeOwned + 'static, { // Workaround for [#460] // @@ -58,7 +58,7 @@ pub async fn request_json( _timeout_hint: Option, ) -> ResponseResult where - T: DeserializeOwned, + T: DeserializeOwned + 'static, { // Workaround for [#460] // @@ -91,7 +91,7 @@ where async fn process_response(response: Response) -> ResponseResult where - T: DeserializeOwned, + T: DeserializeOwned + 'static, { if response.status().is_server_error() { tokio::time::sleep(DELAY_ON_SERVER_ERROR).await; @@ -100,6 +100,44 @@ where let text = response.text().await?; serde_json::from_str::>(&text) + .map(|mut response| { + use crate::types::{Update, UpdateKind}; + use std::any::Any; + + // HACK: Fill-in error information into `UpdateKind::Error`. + // + // Why? Well, we need `Update` deserialization to be reliable, + // even if Telegram breaks something in their Bot API, we want + // 1. Deserialization to """succeed""" + // 2. Get the `update.id` + // + // Both of these points are required for `get_updates(...) -> Vec` + // to behave well after Telegram introduces updates that we can't parse. + // (1.) makes it so only some of the updates in a butch need to be skipped + // (otherwise serde'll stop on the first error). (2.) allows us to issue + // the next `get_updates` call with the right offset, even if the last + // update in the batch didn't deserialize well. + // + // serde's interface doesn't allows us to implement `Deserialize` in such + // a way, that we could keep the data we couldn't parse, so our + // `Deserialize` impl for `UpdateKind` just returns + // `UpdateKind::Error(/* some empty-ish value */)`. Here, through some + // terrible hacks and downcasting, we fill-in the data we couldn't parse + // so that our users can make actionable bug reports. + if TypeId::of::() == TypeId::of::() { + if let TelegramResponse::Ok { response, .. } = &mut response { + if let Some(update) = + (response as &mut T as &mut dyn Any).downcast_mut::() + { + if let UpdateKind::Error(value) = &mut update.kind { + *value = serde_json::from_str(&text).unwrap_or_default(); + } + } + } + } + + response + }) .map_err(|source| RequestError::InvalidJson { source, raw: text.into() })? .into() } diff --git a/crates/teloxide-core/src/types/update.rs b/crates/teloxide-core/src/types/update.rs index 7cd59ff6..24d92778 100644 --- a/crates/teloxide-core/src/types/update.rs +++ b/crates/teloxide-core/src/types/update.rs @@ -156,7 +156,10 @@ pub enum UpdateKind { /// An error that happened during deserialization. /// /// This allows `teloxide` to continue working even if telegram adds a new - /// kind of updates. + /// kinds of updates. + /// + /// **Note that deserialize implementation always returns an empty value**, + /// teloxide fills in the data when doing deserialization. Error(Value), } @@ -182,94 +185,63 @@ impl<'de> Deserialize<'de> for UpdateKind { // Try to deserialize a borrowed-str key, or else try deserializing an owned // string key - let k = map.next_key::<&str>().or_else(|_| { + let key = map.next_key::<&str>().or_else(|_| { map.next_key::().map(|k| { tmp = k; tmp.as_deref() }) }); - if let Ok(Some(k)) = k { - let res = match k { - "message" => { - map.next_value::().map(UpdateKind::Message).map_err(|_| false) + let this = key + .ok() + .flatten() + .and_then(|key| match key { + "message" => map.next_value::().ok().map(UpdateKind::Message), + "edited_message" => { + map.next_value::().ok().map(UpdateKind::EditedMessage) + } + "channel_post" => { + map.next_value::().ok().map(UpdateKind::ChannelPost) + } + "edited_channel_post" => { + map.next_value::().ok().map(UpdateKind::EditedChannelPost) + } + "inline_query" => { + map.next_value::().ok().map(UpdateKind::InlineQuery) } - "edited_message" => map - .next_value::() - .map(UpdateKind::EditedMessage) - .map_err(|_| false), - "channel_post" => map - .next_value::() - .map(UpdateKind::ChannelPost) - .map_err(|_| false), - "edited_channel_post" => map - .next_value::() - .map(UpdateKind::EditedChannelPost) - .map_err(|_| false), - "inline_query" => map - .next_value::() - .map(UpdateKind::InlineQuery) - .map_err(|_| false), "chosen_inline_result" => map .next_value::() - .map(UpdateKind::ChosenInlineResult) - .map_err(|_| false), - "callback_query" => map - .next_value::() - .map(UpdateKind::CallbackQuery) - .map_err(|_| false), - "shipping_query" => map - .next_value::() - .map(UpdateKind::ShippingQuery) - .map_err(|_| false), + .ok() + .map(UpdateKind::ChosenInlineResult), + "callback_query" => { + map.next_value::().ok().map(UpdateKind::CallbackQuery) + } + "shipping_query" => { + map.next_value::().ok().map(UpdateKind::ShippingQuery) + } "pre_checkout_query" => map .next_value::() - .map(UpdateKind::PreCheckoutQuery) - .map_err(|_| false), - "poll" => map.next_value::().map(UpdateKind::Poll).map_err(|_| false), - "poll_answer" => map - .next_value::() - .map(UpdateKind::PollAnswer) - .map_err(|_| false), - "my_chat_member" => map - .next_value::() - .map(UpdateKind::MyChatMember) - .map_err(|_| false), - "chat_member" => map - .next_value::() - .map(UpdateKind::ChatMember) - .map_err(|_| false), + .ok() + .map(UpdateKind::PreCheckoutQuery), + "poll" => map.next_value::().ok().map(UpdateKind::Poll), + "poll_answer" => { + map.next_value::().ok().map(UpdateKind::PollAnswer) + } + "my_chat_member" => { + map.next_value::().ok().map(UpdateKind::MyChatMember) + } + "chat_member" => { + map.next_value::().ok().map(UpdateKind::ChatMember) + } "chat_join_request" => map .next_value::() - .map(UpdateKind::ChatJoinRequest) - .map_err(|_| false), + .ok() + .map(UpdateKind::ChatJoinRequest), + _ => Some(empty_error()), + }) + .unwrap_or_else(empty_error); - _ => Err(true), - }; - - let value_available = match res { - Ok(ok) => return Ok(ok), - Err(e) => e, - }; - - let mut value = serde_json::Map::new(); - value.insert( - k.to_owned(), - if value_available { - map.next_value::().unwrap_or(Value::Null) - } else { - Value::Null - }, - ); - - while let Ok(Some((k, v))) = map.next_entry::<_, Value>() { - value.insert(k, v); - } - - return Ok(UpdateKind::Error(Value::Object(value))); - } - - Ok(UpdateKind::Error(Value::Object(<_>::default()))) + Ok(this) } } @@ -319,6 +291,10 @@ impl Serialize for UpdateKind { } } +fn empty_error() -> UpdateKind { + UpdateKind::Error(Value::Object(<_>::default())) +} + #[cfg(test)] mod test { use crate::types::{ diff --git a/crates/teloxide/src/update_listeners/webhooks/axum.rs b/crates/teloxide/src/update_listeners/webhooks/axum.rs index ba2c930f..2a915a55 100644 --- a/crates/teloxide/src/update_listeners/webhooks/axum.rs +++ b/crates/teloxide/src/update_listeners/webhooks/axum.rs @@ -9,7 +9,7 @@ use tokio::sync::mpsc; use crate::{ requests::Requester, stop::StopFlag, - types::Update, + types::{Update, UpdateKind}, update_listeners::{webhooks::Options, UpdateListener}, }; @@ -186,8 +186,14 @@ pub fn axum_no_setup( Some(tx) => tx, }; - match serde_json::from_str(&input) { - Ok(update) => { + match serde_json::from_str::(&input) { + Ok(mut update) => { + // See HACK comment in + // `teloxide_core::net::request::process_response::{closure#0}` + if let UpdateKind::Error(value) = &mut update.kind { + *value = serde_json::from_str(&input).unwrap_or_default(); + } + tx.send(Ok(update)).expect("Cannot send an incoming update from the webhook") } Err(error) => {