Skip to content

Commit

Permalink
fix: Restore transient error detection for canceled hyper requests
Browse files Browse the repository at this point in the history
Signed-off-by: Joshua Potts <[email protected]>

Signed-off-by: Joshua Potts <[email protected]>
  • Loading branch information
iamjpotts committed Sep 8, 2024
1 parent 55abf78 commit 773a076
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 16 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ hedera-proto = { path = "./protobufs", version = "0.13.0", features = [
] }
hex = "0.4.3"
hmac = "0.12.1"
# Dependency of tonic 0.11. Can be removed when tonic is upgraded to 0.12.
hyper_0 = { package = "hyper", version = "0.14", default-features = false }
# Dependency of tonic 0.12
hyper = { version = "1.3.1", default-features = false }
log = "0.4.17"
num-bigint = "0.4.3"
once_cell = "1.10.0"
Expand All @@ -52,7 +56,6 @@ parking_lot = "0.12.0"
serde_json = { version = "1.0.96", optional = true }
serde = { version = "1.0.163", optional = true }
serde_derive = { version = "1.0.163", optional = true }
hyper = { version = "1.3.1", default-features = false }
pem = "3.0.1"
cbc = "0.1.2"
aes = "0.8.3"
Expand Down Expand Up @@ -80,7 +83,7 @@ features = ["ecdsa", "precomputed-tables", "std"]

[dependencies.pkcs8]
version = "0.10.0"
default_features = false
default-features = false
features = ["encryption"]

[dependencies.triomphe]
Expand Down
24 changes: 10 additions & 14 deletions src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@
* limitations under the License.
* ‍
*/
mod error;

use std::any::type_name;
use std::borrow::Cow;
use std::error::Error as StdError;
use std::ops::ControlFlow;
use std::time::{
Duration,
Expand All @@ -41,6 +41,7 @@ use tonic::transport::Channel;
use triomphe::Arc;

use crate::client::NetworkData;
use crate::execute::error::is_hyper_canceled;
use crate::ping_query::PingQuery;
use crate::{
client,
Expand Down Expand Up @@ -313,17 +314,6 @@ fn map_tonic_error(
node_index: usize,
request_free: bool,
) -> retry::Error {
/// punches through all the layers of `tonic::Status` sources to check if this is a `hyper::Error` that is canceled.

fn is_hyper_canceled(status: &tonic::Status) -> bool {
status
.source()
.and_then(|it| it.downcast_ref::<tonic::transport::Error>())
.and_then(StdError::source)
.and_then(|it| it.downcast_ref::<hyper::Error>())
.is_some_and(hyper::Error::is_canceled)
}

const MIME_HTML: &[u8] = b"text/html";

match status.code() {
Expand Down Expand Up @@ -376,13 +366,19 @@ async fn execute_single<E: Execute + Sync>(
let (node_account_id, channel) = ctx.network.channel(node_index);

log::debug!(
"Executing {} on node at index {node_index} / node id {node_account_id}",
"Preparing {} on node at index {node_index} / node id {node_account_id}",
type_name::<E>()
);

let (request, context) = executable
.make_request(transaction_id.as_ref(), node_account_id)
.map_err(crate::retry::Error::Permanent)?;
// Does not represent a network error or error returned by a node
.map_err(retry::Error::Permanent)?;

log::debug!(
"Executing {} on node at index {node_index} / node id {node_account_id}",
type_name::<E>()
);

let fut = executable.execute(channel, request);

Expand Down
69 changes: 69 additions & 0 deletions src/execute/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* ‌
* Hedera Rust SDK
* ​
* Copyright (C) 2022 - 2023 Hedera Hashgraph, LLC
* ​
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* ‍
*/
use std::error::Error;

use serde::de::StdError;

/// Punches through all the layers of `tonic::Status` sources to check if this is a `hyper::Error` that is canceled.
pub(super) fn is_hyper_canceled(status: &tonic::Status) -> bool {
let source = status
.source()
.and_then(|it| it.downcast_ref::<tonic::transport::Error>())
.and_then(StdError::source);

let Some(source) = source else {
return false;
};

if let Some(hyper_0) = source.downcast_ref::<hyper_0::Error>() {
// tonic 0.11 (current dependency)
hyper_0.is_canceled()
} else if let Some(hyper_1) = source.downcast_ref::<hyper::Error>() {
// tonic 0.12 and request
hyper_1.is_canceled()
} else {
false
}
}

/// Tests some non-detection scenarios.
///
/// Because hyper does not expose constructors for its error variants, there is no
/// reasonable way to construct a test for positive detection of a hyper cancellation.
#[cfg(test)]
mod test_is_hyper_canceled {
use tonic::Code;

use super::is_hyper_canceled;

#[test]
fn ignores_tonic_abort() {
let input = tonic::Status::new(Code::Aborted, "foo");

assert!(!is_hyper_canceled(&input));
}

#[test]
fn ignores_tonic_cancel() {
let input = tonic::Status::new(Code::Cancelled, "foo");

assert!(!is_hyper_canceled(&input));
}
}

0 comments on commit 773a076

Please sign in to comment.