Skip to content

Commit

Permalink
Display reqwest error source (#10004)
Browse files Browse the repository at this point in the history
## Problem

Reqwest errors don't include details about the inner source error. This
means that we get opaque errors like:

```
receive body: error sending request for url (http://localhost:9898/v1/location_config)
```

Instead of the more helpful:

```
receive body: error sending request for url (http://localhost:9898/v1/location_config): operation timed out
```

Touches #9801.

## Summary of changes

Include the source error for `reqwest::Error` wherever it's displayed.
  • Loading branch information
erikgrinaker authored and awarus committed Dec 5, 2024
1 parent bbb0504 commit 8c6b41d
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 14 deletions.
3 changes: 2 additions & 1 deletion control_plane/src/safekeeper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
//! ```text
//! .neon/safekeepers/<safekeeper id>
//! ```
use std::error::Error as _;
use std::future::Future;
use std::io::Write;
use std::path::PathBuf;
Expand All @@ -26,7 +27,7 @@ use crate::{

#[derive(Error, Debug)]
pub enum SafekeeperHttpError {
#[error("Reqwest error: {0}")]
#[error("request error: {0}{}", .0.source().map(|e| format!(": {e}")).unwrap_or_default())]
Transport(#[from] reqwest::Error),

#[error("Error: {0}")]
Expand Down
6 changes: 3 additions & 3 deletions pageserver/client/src/mgmt_api.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::{collections::HashMap, error::Error as _};

use bytes::Bytes;
use detach_ancestor::AncestorDetached;
Expand All @@ -25,10 +25,10 @@ pub struct Client {

#[derive(thiserror::Error, Debug)]
pub enum Error {
#[error("send request: {0}")]
#[error("send request: {0}{}", .0.source().map(|e| format!(": {e}")).unwrap_or_default())]
SendRequest(reqwest::Error),

#[error("receive body: {0}")]
#[error("receive body: {0}{}", .0.source().map(|e| format!(": {e}")).unwrap_or_default())]
ReceiveBody(reqwest::Error),

#[error("receive error body: {0}")]
Expand Down
7 changes: 6 additions & 1 deletion pageserver/src/consumption_metrics/upload.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::error::Error as _;
use std::time::SystemTime;

use chrono::{DateTime, Utc};
Expand Down Expand Up @@ -350,7 +351,11 @@ impl std::fmt::Display for UploadError {

match self {
Rejected(code) => write!(f, "server rejected the metrics with {code}"),
Reqwest(e) => write!(f, "request failed: {e}"),
Reqwest(e) => write!(
f,
"request failed: {e}{}",
e.source().map(|e| format!(": {e}")).unwrap_or_default()
),
Cancelled => write!(f, "cancelled"),
}
}
Expand Down
3 changes: 2 additions & 1 deletion safekeeper/src/http/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
//! etc.
use reqwest::{IntoUrl, Method, StatusCode};
use std::error::Error as _;
use utils::{
http::error::HttpErrorBody,
id::{NodeId, TenantId, TimelineId},
Expand All @@ -26,7 +27,7 @@ pub struct Client {
#[derive(thiserror::Error, Debug)]
pub enum Error {
/// Failed to receive body (reqwest error).
#[error("receive body: {0}")]
#[error("receive body: {0}{}", .0.source().map(|e| format!(": {e}")).unwrap_or_default())]
ReceiveBody(reqwest::Error),

/// Status is not ok, but failed to parse body as `HttpErrorBody`.
Expand Down
3 changes: 2 additions & 1 deletion storage_controller/src/compute_hook.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::error::Error as _;
use std::sync::Arc;
use std::{collections::HashMap, time::Duration};

Expand Down Expand Up @@ -172,7 +173,7 @@ struct ComputeHookNotifyRequest {
#[derive(thiserror::Error, Debug)]
pub(crate) enum NotifyError {
// Request was not send successfully, e.g. transport error
#[error("Sending request: {0}")]
#[error("Sending request: {0}{}", .0.source().map(|e| format!(": {e}")).unwrap_or_default())]
Request(#[from] reqwest::Error),
// Request could not be serviced right now due to ongoing Operation in control plane, but should be possible soon.
#[error("Control plane tenant busy")]
Expand Down
11 changes: 8 additions & 3 deletions storage_controller/src/peer_client.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use crate::tenant_shard::ObservedState;
use pageserver_api::shard::TenantShardId;
use serde::{Deserialize, Serialize};
use std::{collections::HashMap, time::Duration};
use std::collections::HashMap;
use std::error::Error as _;
use std::time::Duration;
use tokio_util::sync::CancellationToken;

use hyper::Uri;
Expand All @@ -17,11 +19,14 @@ pub(crate) struct PeerClient {

#[derive(thiserror::Error, Debug)]
pub(crate) enum StorageControllerPeerError {
#[error("failed to deserialize error response with status code {0} at {1}: {2}")]
#[error(
"failed to deserialize error response with status code {0} at {1}: {2}{}",
.2.source().map(|e| format!(": {e}")).unwrap_or_default()
)]
DeserializationError(StatusCode, Url, reqwest::Error),
#[error("storage controller peer API error ({0}): {1}")]
ApiError(StatusCode, String),
#[error("failed to send HTTP request: {0}")]
#[error("failed to send HTTP request: {0}{}", .0.source().map(|e| format!(": {e}")).unwrap_or_default())]
SendError(reqwest::Error),
#[error("Cancelled")]
Cancelled,
Expand Down
14 changes: 10 additions & 4 deletions storage_scrubber/src/cloud_admin_api.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::error::Error as _;

use chrono::{DateTime, Utc};
use futures::Future;
use hex::FromHex;
Expand Down Expand Up @@ -30,14 +32,18 @@ impl std::fmt::Display for Error {
match &self.kind {
ErrorKind::RequestSend(e) => write!(
f,
"Failed to send a request. Context: {}, error: {}",
self.context, e
"Failed to send a request. Context: {}, error: {}{}",
self.context,
e,
e.source().map(|e| format!(": {e}")).unwrap_or_default()
),
ErrorKind::BodyRead(e) => {
write!(
f,
"Failed to read a request body. Context: {}, error: {}",
self.context, e
"Failed to read a request body. Context: {}, error: {}{}",
self.context,
e,
e.source().map(|e| format!(": {e}")).unwrap_or_default()
)
}
ErrorKind::ResponseStatus(status) => {
Expand Down

0 comments on commit 8c6b41d

Please sign in to comment.