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

Add connect timeout and overall timeout #1238

Merged
merged 4 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion atuin-client/src/api_client.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::HashMap;
use std::env;
use std::time::Duration;

use eyre::{bail, Result};
use reqwest::{
Expand Down Expand Up @@ -106,7 +107,12 @@ pub async fn latest_version() -> Result<Version> {
}

impl<'a> Client<'a> {
pub fn new(sync_addr: &'a str, session_token: &'a str) -> Result<Self> {
pub fn new(
sync_addr: &'a str,
session_token: &'a str,
connect_timeout: u64,
timeout: u64,
) -> Result<Self> {
let mut headers = HeaderMap::new();
headers.insert(AUTHORIZATION, format!("Token {session_token}").parse()?);

Expand All @@ -115,6 +121,8 @@ impl<'a> Client<'a> {
client: reqwest::Client::builder()
.user_agent(APP_USER_AGENT)
.default_headers(headers)
.connect_timeout(Duration::new(connect_timeout, 0))
.timeout(Duration::new(timeout, 0))
.build()?,
})
}
Expand Down
14 changes: 12 additions & 2 deletions atuin-client/src/record/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ pub enum Operation {
}

pub async fn diff(settings: &Settings, store: &mut impl Store) -> Result<(Vec<Diff>, RecordIndex)> {
let client = Client::new(&settings.sync_address, &settings.session_token)?;
let client = Client::new(
&settings.sync_address,
&settings.session_token,
settings.network_connect_timeout,
settings.network_timeout,
)?;

let local_index = store.tail_records().await?;
let remote_index = client.record_index().await?;
Expand Down Expand Up @@ -218,7 +223,12 @@ pub async fn sync_remote(
local_store: &mut impl Store,
settings: &Settings,
) -> Result<(i64, i64)> {
let client = Client::new(&settings.sync_address, &settings.session_token)?;
let client = Client::new(
&settings.sync_address,
&settings.session_token,
settings.network_connect_timeout,
settings.network_timeout,
)?;

let mut uploaded = 0;
let mut downloaded = 0;
Expand Down
5 changes: 5 additions & 0 deletions atuin-client/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,9 @@ pub struct Settings {
pub workspaces: bool,
pub ctrl_n_shortcuts: bool,

pub network_connect_timeout: u64,
pub network_timeout: u64,

// This is automatically loaded when settings is created. Do not set in
// config! Keep secrets and settings apart.
pub session_token: String,
Expand Down Expand Up @@ -371,6 +374,8 @@ impl Settings {
.set_default("workspaces", false)?
.set_default("ctrl_n_shortcuts", false)?
.set_default("secrets_filter", true)?
.set_default("network_connect_timeout", 5)?
.set_default("network_timeout", 30)?
.add_source(
Environment::with_prefix("atuin")
.prefix_separator("_")
Expand Down
7 changes: 6 additions & 1 deletion atuin-client/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,12 @@ async fn sync_upload(
}

pub async fn sync(settings: &Settings, force: bool, db: &mut (impl Database + Send)) -> Result<()> {
let client = api_client::Client::new(&settings.sync_address, &settings.session_token)?;
let client = api_client::Client::new(
&settings.sync_address,
&settings.session_token,
settings.network_connect_timeout,
settings.network_timeout,
)?;

let key = load_key(settings)?; // encryption key

Expand Down
7 changes: 6 additions & 1 deletion atuin/src/command/client/account/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ pub async fn run(settings: &Settings) -> Result<()> {
bail!("You are not logged in");
}

let client = api_client::Client::new(&settings.sync_address, &settings.session_token)?;
let client = api_client::Client::new(
&settings.sync_address,
&settings.session_token,
settings.network_connect_timeout,
settings.network_timeout,
)?;

client.delete().await?;

Expand Down
7 changes: 6 additions & 1 deletion atuin/src/command/client/sync/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ use colored::Colorize;
use eyre::Result;

pub async fn run(settings: &Settings, db: &impl Database) -> Result<()> {
let client = api_client::Client::new(&settings.sync_address, &settings.session_token)?;
let client = api_client::Client::new(
&settings.sync_address,
&settings.session_token,
settings.network_connect_timeout,
settings.network_timeout,
)?;

let status = client.status().await?;
let last_sync = Settings::last_sync()?;
Expand Down
2 changes: 1 addition & 1 deletion atuin/tests/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ async fn registration() {
.await
.unwrap();

let client = api_client::Client::new(&address, &registration_response.session).unwrap();
let client = api_client::Client::new(&address, &registration_response.session, 5, 30).unwrap();

// the session token works
let status = client.status().await.unwrap();
Expand Down
13 changes: 13 additions & 0 deletions docs/docs/config/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -280,3 +280,16 @@ macOS does not have an <kbd>Alt</kbd> key, although terminal emulators can often
# Use Ctrl-0 .. Ctrl-9 instead of Alt-0 .. Alt-9 UI shortcuts
ctrl_n_shortcuts = true
```

## network_timeout
Default: 30

The max amount of time (in seconds) to wait for a network request. If any
operations with a sync server take longer than this, the code will fail -
rather than wait indefinitely.

## network_connect_timeout
Default: 5

The max time (in seconds) we wait for a connection to become established with a
remote sync server. Any longer than this and the request will fail.