From ecaa5d5bec25beddd10dd4a94595c940e71e9122 Mon Sep 17 00:00:00 2001 From: Waffle Date: Wed, 12 May 2021 11:51:03 +0300 Subject: [PATCH] Add `net::default_reqwest_settings` function This function can help when users want to set up their own client setting for one reason or another, since settings set by the function, are required for stable work. This function was previously private and named `sound_bot`. The old name was confusing since safety and soundness are used in the Rust context almost entirely for `unsafe` code, UB & co. So I've changed the name to a more descriptive one. --- CHANGELOG.md | 2 ++ src/bot.rs | 41 +++++++++-------------------------------- src/net.rs | 42 +++++++++++++++++++++++++++++++++++++++--- 3 files changed, 50 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a2543d9..ae63f5fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,11 +13,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Getters for fields nested in `Chat` ([#80][pr80]) - API errors: `ApiError::NotEnoughRightsToManagePins`, `ApiError::BotKickedFromSupergroup` ([#84][pr84]) - Telegram bot API 5.2 support ([#86][pr86]) +- `net::default_reqwest_settings` function ([#90][pr90]) [pr75]: https://github.com/teloxide/teloxide-core/pull/75 [pr80]: https://github.com/teloxide/teloxide-core/pull/80 [pr84]: https://github.com/teloxide/teloxide-core/pull/84 [pr86]: https://github.com/teloxide/teloxide-core/pull/86 +[pr90]: https://github.com/teloxide/teloxide-core/pull/90 ### Changed diff --git a/src/bot.rs b/src/bot.rs index 132bbaaa..e5677716 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -1,9 +1,6 @@ -use std::{future::Future, sync::Arc, time::Duration}; +use std::{future::Future, sync::Arc}; -use reqwest::{ - header::{HeaderMap, CONNECTION}, - Client, ClientBuilder, -}; +use reqwest::Client; use serde::{de::DeserializeOwned, Serialize}; use crate::{ @@ -17,8 +14,7 @@ mod api; mod api_url; mod download; -pub(crate) const TELOXIDE_TOKEN: &str = "TELOXIDE_TOKEN"; -pub(crate) const TELOXIDE_PROXY: &str = "TELOXIDE_PROXY"; +const TELOXIDE_TOKEN: &str = "TELOXIDE_TOKEN"; /// A requests sender. /// @@ -76,13 +72,18 @@ impl Bot { where S: Into, { - Self::with_client(token, build_sound_bot()) + let client = net::default_reqwest_settings() + .build() + .expect("Client creation failed"); + + Self::with_client(token, client) } /// Creates a new `Bot` with the specified token and your /// [`reqwest::Client`]. /// /// # Caution + /// /// Your custom client might not be configured correctly to be able to work /// in long time durations, see [issue 223]. /// @@ -239,30 +240,6 @@ impl Bot { } } -/// Returns a builder with safe settings. -/// -/// By "safe settings" I mean that a client will be able to work in long time -/// durations, see the [issue 223]. -/// -/// [issue 223]: https://github.com/teloxide/teloxide/issues/223 -pub(crate) fn sound_bot() -> ClientBuilder { - let mut headers = HeaderMap::new(); - headers.insert(CONNECTION, "keep-alive".parse().unwrap()); - - let connect_timeout = Duration::from_secs(5); - let timeout = 10; - - ClientBuilder::new() - .connect_timeout(connect_timeout) - .timeout(Duration::from_secs(connect_timeout.as_secs() + timeout + 2)) - .tcp_nodelay(true) - .default_headers(headers) -} - -pub(crate) fn build_sound_bot() -> Client { - sound_bot().build().expect("creating reqwest::Client") -} - fn get_env(env: &'static str) -> String { std::env::var(env).unwrap_or_else(|_| panic!("Cannot get the {} env variable", env)) } diff --git a/src/net.rs b/src/net.rs index 6c9665dc..6ce6525d 100644 --- a/src/net.rs +++ b/src/net.rs @@ -1,5 +1,7 @@ //! Network-specific API. +use std::time::Duration; + pub use self::download::{download_file, download_file_stream, Download}; pub(crate) use self::{ @@ -33,19 +35,53 @@ pub const TELEGRAM_API_URL: &str = "https://api.telegram.org"; /// /// If `TELOXIDE_PROXY` exists, but isn't correct url. pub fn client_from_env() -> reqwest::Client { - use crate::bot::{sound_bot, TELOXIDE_PROXY}; use reqwest::Proxy; - let builder = sound_bot(); + const TELOXIDE_PROXY: &str = "TELOXIDE_PROXY"; + + let builder = default_reqwest_settings(); match std::env::var(TELOXIDE_PROXY).ok() { - Some(proxy) => builder.proxy(Proxy::all(&proxy).expect("creating reqwest::Proxy")), + Some(proxy) => builder.proxy(Proxy::all(&proxy).expect("reqwest::Proxy creation failed")), None => builder, } .build() .expect("creating reqwest::Client") } +/// Returns a reqwest client builder with default settings. +/// +/// Client built from default settings is supposed to work in long time +/// durations, see the [issue 223]. +/// +/// Current setting are: +/// - `connection/keep-alive` default header +/// - connection timeout of 5 seconds +/// - timeout of 17 seconds +/// - tcp_nodelay is on +/// +/// Notes: +/// 1. the settings may change in the future +/// 2. if you are using polling mechanizm to get updates, the timeout configured +/// in the client should be bigger than polling timeout +/// +/// [issue 223]: https://github.com/teloxide/teloxide/issues/223 +pub fn default_reqwest_settings() -> reqwest::ClientBuilder { + use reqwest::header::{HeaderMap, CONNECTION}; + + let mut headers = HeaderMap::new(); + headers.insert(CONNECTION, "keep-alive".parse().unwrap()); + + let connect_timeout = Duration::from_secs(5); + let timeout = connect_timeout + Duration::from_secs(12); + + reqwest::Client::builder() + .connect_timeout(connect_timeout) + .timeout(timeout) + .tcp_nodelay(true) + .default_headers(headers) +} + /// Creates URL for making HTTPS requests. See the [Telegram documentation]. /// /// [Telegram documentation]: https://core.telegram.org/bots/api#making-requests