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

feat: adds option to increase rover's client timeout #838

Merged
merged 10 commits into from
Sep 23, 2021
35 changes: 16 additions & 19 deletions crates/rover-client/src/blocking/client.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use crate::RoverClientError;

use backoff::ExponentialBackoff;
use graphql_client::{Error as GraphQLError, GraphQLQuery, Response as GraphQLResponse};
use reqwest::{
blocking::{Client as ReqwestClient, Response},
Expand All @@ -11,6 +10,9 @@ use reqwest::{
pub(crate) const JSON_CONTENT_TYPE: &str = "application/json";
pub(crate) const CLIENT_NAME: &str = "rover-client";

const MAX_ELAPSED_TIME: Option<Duration> =
Some(Duration::from_secs(if cfg!(test) { 2 } else { 10 }));

use std::time::Duration;

/// Represents a generic GraphQL client for making http requests.
Expand Down Expand Up @@ -62,6 +64,12 @@ impl GraphQLClient {
request_body: String,
header_map: &HeaderMap,
) -> Result<Response, RoverClientError> {
use backoff::{
retry,
Error::{Permanent, Transient},
ExponentialBackoff,
};

tracing::trace!(request_headers = ?header_map);
tracing::debug!("Request Body: {}", request_body);
let graphql_operation = || {
Expand All @@ -71,41 +79,30 @@ impl GraphQLClient {
.headers(header_map.clone())
.body(request_body.clone())
.send()
.map_err(backoff::Error::Permanent)?;
.map_err(Permanent)?;

if let Err(status_error) = response.error_for_status_ref() {
if let Some(response_status) = status_error.status() {
if response_status.is_server_error() {
Err(backoff::Error::Transient(status_error))
Err(Transient(status_error))
} else {
Err(backoff::Error::Permanent(status_error))
Err(Permanent(status_error))
}
} else {
Err(backoff::Error::Permanent(status_error))
Err(Permanent(status_error))
}
} else {
Ok(response)
}
};

let max_elapsed_time = Some(Duration::from_secs(if cfg!(test) { 2 } else { 10 }));

let backoff_strategy = ExponentialBackoff {
max_elapsed_time,
max_elapsed_time: MAX_ELAPSED_TIME,
..Default::default()
};

backoff::retry(backoff_strategy, graphql_operation).map_err(|e| match e {
backoff::Error::Permanent(reqwest_error) | backoff::Error::Transient(reqwest_error) => {
if reqwest_error.is_connect() {
RoverClientError::CouldNotConnect {
url: reqwest_error.url().cloned(),
source: reqwest_error,
}
} else {
reqwest_error.into()
}
}
retry(backoff_strategy, graphql_operation).map_err(|e| match e {
Permanent(reqwest_error) | Transient(reqwest_error) => reqwest_error.into(),
})
}

Expand Down
13 changes: 0 additions & 13 deletions crates/rover-client/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use reqwest::Url;
use thiserror::Error;

use crate::shared::{BuildErrors, CheckResponse, GraphRef};
Expand Down Expand Up @@ -73,18 +72,6 @@ pub enum RoverClientError {
frontend_url_root: String,
},

#[error("Could not connect to {}.",
if let Some(url) = .url {
url.to_string()
} else {
"unknown URL".to_string()
}
)]
CouldNotConnect {
source: reqwest::Error,
url: Option<Url>,
},

/// Encountered an error sending the request.
#[error(transparent)]
SendRequest(#[from] reqwest::Error),
Expand Down
7 changes: 7 additions & 0 deletions docs/source/configuring.md
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,13 @@ In some configurations (often on internal networks) users may need Rover to comm
- The `--insecure-accept-invalid-hostnames` flag will disable hostname validation. If hostname verification is not used, any valid certificate for any site will be trusted for use from any other. This introduces a significant vulnerability to man-in-the-middle attacks.

- The `--insecure-accept-invalid-certs` flag will disable certificate validation. If invalid certificates are trusted, any certificate for any site will be trusted for use. This includes expired certificates. This introduces significant vulnerabilities, and should only be used as a last resort.

## Increase the client timeout

Sometimes, you may want to make queries for large amounts of data that may take some extra time for the Studio API to process.
Copy link
Contributor

@StephenBarlow StephenBarlow Sep 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation (feel free to adjust anything I've gotten a little wrong or you disagree with):

Increasing request timeouts

By default, Rover times out requests to the Apollo Studio API after 30 seconds. If you're executing a command that might take longer than 30 seconds to process, you can increase this timeout with the --client-timeout option:

rover lengthy command (whatever that would be) --client-timeout=60 # in seconds


Rover supports increasing the client timeout in cases like this with the `--client-timeout` option, which takes a number of seconds as a parameter. By default, Rover will time out requests after 30 seconds.

## Supported environment variables

You can configure Rover's behavior by setting the environment variables listed below.
Expand Down
6 changes: 6 additions & 0 deletions docs/source/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -246,3 +246,9 @@ Some build errors are part of normal workflows. For instance, you may need to pu

This error occurs when an operation check fails. This means that you proposed a schema that would break operations in use by existing clients. You can configure this behavior in the Checks -> Configuration view in [Apollo Studio](https://studio.apollographql.com/), and you can read more about client checks [here](https://www.apollographql.com/docs/studio/schema-checks/).

### E031

This error occurs when Rover made an HTTP(S) request and it timed out.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"made an HTTP request that timed out." <- I'm not too stressed about needing to include the (S)


The client timeout that Rover sets is configurable. You can try increasing the number of seconds Rover will wait to timeout with the `--client-timeout` flag, but it's also possible that you've made a request for too much data from the Studio API, or that the Studio API is experiencing degraded performance. You can check for the latter at our [status page](https://status.apollographql.com).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"You can increase Rover's request timeout, but it's also possible..."

"You can check for known performance issues on our status page"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That link would point to the new section in configuring.md


19 changes: 16 additions & 3 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use structopt::{clap::AppSettings, StructOpt};
use crate::command::output::JsonOutput;
use crate::command::{self, RoverOutput};
use crate::utils::{
client::{get_configured_client, StudioClientConfig},
client::{get_configured_client, ClientTimeout, StudioClientConfig},
env::{RoverEnv, RoverEnvKey},
stringify::option_from_display,
version,
Expand Down Expand Up @@ -94,6 +94,15 @@ pub struct Rover {
)]
accept_invalid_hostnames: bool,

/// Configure the timeout length (in seconds) when performing HTTP(S) requests.
#[structopt(
long = "client-timeout",
case_insensitive = true,
global = true,
default_value
)]
client_timeout: ClientTimeout,

#[structopt(skip)]
#[serde(skip_serializing)]
pub(crate) env_store: RoverEnv,
Expand Down Expand Up @@ -250,8 +259,12 @@ impl Rover {
// if a request hasn't been made yet, this cell won't be populated yet
self.client
.fill(
get_configured_client(self.accept_invalid_certs, self.accept_invalid_hostnames)
.expect("Could not configure the request client"),
get_configured_client(
EverlastingBugstopper marked this conversation as resolved.
Show resolved Hide resolved
self.accept_invalid_certs,
self.accept_invalid_hostnames,
self.client_timeout,
)
.expect("Could not configure the request client"),
)
.expect("Could not overwrite the existing request client");
self.get_reqwest_client()
Expand Down
2 changes: 2 additions & 0 deletions src/error/metadata/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub enum Code {
E028,
E029,
E030,
E031,
}

impl Display for Code {
Expand Down Expand Up @@ -79,6 +80,7 @@ impl Code {
(Code::E028, include_str!("./codes/E028.md").to_string()),
(Code::E029, include_str!("./codes/E029.md").to_string()),
(Code::E030, include_str!("./codes/E030.md").to_string()),
(Code::E031, include_str!("./codes/E031.md").to_string()),
];
contents.into_iter().collect()
}
Expand Down
3 changes: 3 additions & 0 deletions src/error/metadata/codes/E031.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
This error occurs when Rover made an HTTP(S) request and it timed out.

The client timeout that Rover sets is configurable. You can try increasing the number of seconds Rover will wait to timeout with the `--client-timeout` flag, but it's also possible that you've made a request for too much data from the Studio API, or that the Studio API is experiencing degraded performance. You can check for the latter at our [status page](https://status.apollographql.com).
13 changes: 8 additions & 5 deletions src/error/metadata/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,14 @@ impl From<&mut anyhow::Error> for Metadata {
RoverClientError::InvalidHeaderValue(_) => {
(Some(Suggestion::SubmitIssue), Some(Code::E003))
}
RoverClientError::SendRequest(_) => {
(Some(Suggestion::SubmitIssue), Some(Code::E004))
RoverClientError::SendRequest(e) => {
if e.is_connect() {
(Some(Suggestion::CheckServerConnection), Some(Code::E028))
} else if e.is_timeout() {
(Some(Suggestion::IncreaseClientTimeout), Some(Code::E031))
} else {
(Some(Suggestion::SubmitIssue), Some(Code::E004))
}
}
RoverClientError::MalformedResponse { null_field: _ } => {
(Some(Suggestion::SubmitIssue), Some(Code::E005))
Expand Down Expand Up @@ -133,9 +139,6 @@ impl From<&mut anyhow::Error> for Metadata {
(Some(Suggestion::RunComposition), Some(Code::E027))
}
RoverClientError::AdhocError { .. } => (None, None),
RoverClientError::CouldNotConnect { .. } => {
(Some(Suggestion::CheckServerConnection), Some(Code::E028))
}
RoverClientError::InvalidGraphRef { .. } => {
unreachable!("Graph ref parse errors should be caught via structopt")
}
Expand Down
4 changes: 3 additions & 1 deletion src/error/metadata/suggestion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub enum Suggestion {
FixOperationsInSchema {
graph_ref: GraphRef,
},
IncreaseClientTimeout,
}

impl Display for Suggestion {
Expand Down Expand Up @@ -141,7 +142,8 @@ impl Display for Suggestion {
Suggestion::CheckGnuVersion => "This is likely an issue with your current version of `glibc`. Try running `ldd --version`, and if the version >= 2.18, we suggest installing the Rover binary built for `x86_64-unknown-linux-gnu`".to_string(),
Suggestion::FixSubgraphSchema { graph_ref, subgraph } => format!("The changes in the schema you proposed for subgraph {} are incompatible with supergraph {}. See {} for more information on resolving build errors.", Yellow.normal().paint(subgraph.to_string()), Yellow.normal().paint(graph_ref.to_string()), Cyan.normal().paint("https://www.apollographql.com/docs/federation/errors/")),
Suggestion::FixCompositionErrors => format!("The subgraph schemas you provided are incompatible with each other. See {} for more information on resolving build errors.", Cyan.normal().paint("https://www.apollographql.com/docs/federation/errors/")),
Suggestion::FixOperationsInSchema { graph_ref } => format!("The changes in the schema you proposed are incompatible with graph {}. See {} for more information on resolving operation check errors.", Yellow.normal().paint(graph_ref.to_string()), Cyan.normal().paint("https://www.apollographql.com/docs/studio/schema-checks/"))
Suggestion::FixOperationsInSchema { graph_ref } => format!("The changes in the schema you proposed are incompatible with graph {}. See {} for more information on resolving operation check errors.", Yellow.normal().paint(graph_ref.to_string()), Cyan.normal().paint("https://www.apollographql.com/docs/studio/schema-checks/")),
Suggestion::IncreaseClientTimeout => "You can try increasing the timeout value by passing a higher value to the --client-timeout option.".to_string(),
};
write!(formatter, "{}", &suggestion)
}
Expand Down
44 changes: 44 additions & 0 deletions src/utils/client.rs
Original file line number Diff line number Diff line change
@@ -1,26 +1,70 @@
use core::fmt;
use std::{str::FromStr, time::Duration};

use crate::error::RoverError;
use crate::Result;
use crate::PKG_VERSION;

use houston as config;
use reqwest::blocking::Client;
use rover_client::blocking::StudioClient;

use serde::Serialize;

/// the Apollo graph registry's production API endpoint
const STUDIO_PROD_API_ENDPOINT: &str = "https://graphql.api.apollographql.com/api/graphql";

pub(crate) fn get_configured_client(
accept_invalid_certs: bool,
accept_invalid_hostnames: bool,
client_timeout: ClientTimeout,
) -> Result<Client> {
let client = Client::builder()
.gzip(true)
.brotli(true)
.danger_accept_invalid_certs(accept_invalid_certs)
.danger_accept_invalid_hostnames(accept_invalid_hostnames)
.timeout(client_timeout.get_duration())
.build()?;
Ok(client)
}

#[derive(Debug, Copy, Clone, Serialize)]
pub(crate) struct ClientTimeout {
duration: Duration,
}

impl ClientTimeout {
pub(crate) fn new(duration_in_seconds: u64) -> ClientTimeout {
ClientTimeout {
duration: Duration::from_secs(duration_in_seconds),
}
}

pub(crate) fn get_duration(&self) -> Duration {
self.duration
}
}

impl Default for ClientTimeout {
fn default() -> ClientTimeout {
ClientTimeout::new(30)
}
}

impl FromStr for ClientTimeout {
type Err = RoverError;
fn from_str(duration_in_secs: &str) -> Result<ClientTimeout> {
Ok(ClientTimeout::new(duration_in_secs.parse()?))
}
}

impl fmt::Display for ClientTimeout {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.duration.as_secs())
EverlastingBugstopper marked this conversation as resolved.
Show resolved Hide resolved
}
}

pub struct StudioClientConfig {
pub(crate) config: config::Config,
client: Client,
Expand Down