diff --git a/CHANGELOG.md b/CHANGELOG.md index fb88997e6b..95e753ec2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### Fixes - Fixed a bug where the `dart-symbol-map` command did not accept the `--url` argument ([#3108](https://github.com/getsentry/sentry-cli/pull/3108)). +- Retry DNS resolution failures for `sentry.io` requests to reduce intermittent failures for some users ([#3085](https://github.com/getsentry/sentry-cli/pull/3085)) ## 3.1.0 diff --git a/src/api/errors/mod.rs b/src/api/errors/mod.rs index 230a920ff7..1b7be0adae 100644 --- a/src/api/errors/mod.rs +++ b/src/api/errors/mod.rs @@ -14,17 +14,12 @@ pub(super) struct ProjectRenamedError(pub(super) String); pub(super) type ApiResult = Result; #[derive(Debug, thiserror::Error)] -#[error("request failed with retryable status code {}", .body.status)] -pub(super) struct RetryError { - body: ApiResponse, -} - -impl RetryError { - pub fn new(body: ApiResponse) -> Self { - Self { body } - } - - pub fn into_body(self) -> ApiResponse { - self.body - } +pub(super) enum RetryError { + #[error("request failed with retryable status code {}", body.status)] + Status { body: ApiResponse }, + #[error("request failed with retryable error: {source}")] + ApiError { + #[from] + source: ApiError, + }, } diff --git a/src/api/mod.rs b/src/api/mod.rs index 7dce776cae..fa60a59607 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -15,11 +15,12 @@ mod serialization; use std::borrow::Cow; use std::cell::RefCell; use std::collections::HashMap; +use std::error::Error as _; #[cfg(any(target_os = "macos", not(feature = "managed")))] use std::fs::File; use std::io::{self, Read as _, Write}; use std::rc::Rc; -use std::sync::Arc; +use std::sync::{Arc, LazyLock}; use std::{fmt, thread}; use anyhow::{Context as _, Result}; @@ -39,11 +40,12 @@ use serde::{Deserialize, Serialize}; use sha1_smol::Digest; use symbolic::common::DebugId; use symbolic::debuginfo::ObjectKind; +use url::Url; use uuid::Uuid; use crate::api::errors::{ProjectRenamedError, RetryError}; use crate::config::{Auth, Config}; -use crate::constants::{ARCH, EXT, PLATFORM, RELEASE_REGISTRY_LATEST_URL, VERSION}; +use crate::constants::{ARCH, DEFAULT_HOST, EXT, PLATFORM, RELEASE_REGISTRY_LATEST_URL, VERSION}; use crate::utils::http::{self, is_absolute_url}; use crate::utils::non_empty::NonEmptySlice; use crate::utils::progress::{ProgressBar, ProgressBarMode}; @@ -111,6 +113,7 @@ pub struct ApiRequest { is_authenticated: bool, body: Option>, progress_bar_mode: ProgressBarMode, + url: String, } /// Represents an API response. @@ -180,7 +183,7 @@ impl Api { region_url: Option<&str>, ) -> ApiResult { let (url, auth) = self.resolve_base_url_and_auth(url, region_url)?; - self.construct_api_request(method, &url, auth) + self.construct_api_request(method, url, auth) } fn resolve_base_url_and_auth( @@ -210,7 +213,7 @@ impl Api { fn construct_api_request( &self, method: Method, - url: &str, + url: String, auth: Option<&Auth>, ) -> ApiResult { let mut handle = self @@ -1161,7 +1164,7 @@ impl ApiRequest { fn create( mut handle: r2d2::PooledConnection, method: &Method, - url: &str, + url: String, auth: Option<&Auth>, pipeline_env: Option, global_headers: Option>, @@ -1196,7 +1199,7 @@ impl ApiRequest { Method::Delete => handle.custom_request("DELETE")?, } - handle.url(url)?; + handle.url(&url)?; let request = ApiRequest { handle, @@ -1204,6 +1207,7 @@ impl ApiRequest { is_authenticated: false, body: None, progress_bar_mode: ProgressBarMode::Disabled, + url, }; let request = match auth { @@ -1313,11 +1317,22 @@ impl ApiRequest { debug!("retry number {retry_number}, max retries: {max_retries}"); *retry_number += 1; - let mut rv = self.send_into(&mut out)?; + let mut rv = self.send_into(&mut out).map_err(|err| { + // Retry DNS failures for sentry.io, as these likely indicate + // a network issue. DNS failures for other domains should not + // be retried, to avoid masking configuration problems (e.g. + // if the user has mistyped their self-hosted URL). + if is_dns_error(&err) && self.is_sentry_io_host() { + anyhow::anyhow!(RetryError::from(err)) + } else { + anyhow::anyhow!(err) + } + })?; + rv.body = Some(out); if RETRY_STATUS_CODES.contains(&rv.status) { - anyhow::bail!(RetryError::new(rv)); + anyhow::bail!(RetryError::Status { body: rv }); } Ok(rv) @@ -1336,10 +1351,41 @@ impl ApiRequest { }) .call() .or_else(|err| match err.downcast::() { - Ok(err) => Ok(err.into_body()), + Ok(RetryError::Status { body }) => Ok(body), + Ok(RetryError::ApiError { source }) => Err(source), Err(err) => Err(ApiError::with_source(ApiErrorKind::RequestFailed, err)), }) } + + /// Determines whether a URL has a sentry.io host (including subdomains). + fn is_sentry_io_host(&self) -> bool { + /// A regex which matches exactly "sentry.io" and hostnames ending in + /// ".sentry.io". + static SENTRY_IO_HOST_RE: LazyLock = LazyLock::new(|| { + Regex::new(&format!(r"^(\S*\.)?{}$", regex::escape(DEFAULT_HOST))) + .expect("regex is valid") + }); + + Url::parse(&self.url) + .ok() + .map(|url| { + url.host_str() + .is_some_and(|host| SENTRY_IO_HOST_RE.is_match(host)) + }) + .unwrap_or(false) + } +} + +/// Returns true if the error source chain contains a curl DNS resolution error. +fn is_dns_error(err: &ApiError) -> bool { + let mut current = err.source(); + while let Some(error) = current { + if let Some(curl_err) = error.downcast_ref::() { + return curl_err.is_couldnt_resolve_host(); + } + current = error.source(); + } + false } impl ApiResponse { diff --git a/src/constants.rs b/src/constants.rs index 9d8f52ada9..7acee0849b 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -8,6 +8,9 @@ pub const APP_NAME: &str = "sentrycli"; /// The default API URL pub const DEFAULT_URL: &str = "https://sentry.io/"; +/// The default API host +pub const DEFAULT_HOST: &str = "sentry.io"; + /// The version of the library pub const VERSION: &str = env!("CARGO_PKG_VERSION");