Skip to content

Commit

Permalink
chore(tonic): Move TimeoutExpired out of transport (#1826)
Browse files Browse the repository at this point in the history
By moving the TimeoutExpired type from the transport module
to the the tonic engine proper, the type is made available to
other transport implementations and tonic can be built without
the "server" feature while still returning a Status indicating
the timeout.

Alternative transport implementations can use the type, now that it is
moved, when they want the tonic Status error handling to recognize the
error as having been triggered by a timeout in the transport logic. The
tonic Status error will have a code of Cancelled with a message of
"Timeout expired".

There is already a test for this:

    cargo test picks_server_timeout_if_thats_sorter

which worked the original way and contines to work; but now a new
transport implementation can get the same behavior.

Addresses #1825.
  • Loading branch information
FrankReh authored Jul 28, 2024
1 parent 2d97911 commit 71fca36
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 27 deletions.
2 changes: 1 addition & 1 deletion tonic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ pub use extensions::GrpcMethod;
pub use http::Extensions;
pub use request::{IntoRequest, IntoStreamingRequest, Request};
pub use response::Response;
pub use status::{Code, Status};
pub use status::{Code, Status, TimeoutExpired};

pub(crate) type Error = Box<dyn std::error::Error + Send + Sync>;

Expand Down
23 changes: 21 additions & 2 deletions tonic/src/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,8 +606,7 @@ fn find_status_in_source_chain(err: &(dyn Error + 'static)) -> Option<Status> {
});
}

#[cfg(feature = "server")]
if let Some(timeout) = err.downcast_ref::<crate::transport::TimeoutExpired>() {
if let Some(timeout) = err.downcast_ref::<TimeoutExpired>() {
return Some(Status::cancelled(timeout.to_string()));
}

Expand Down Expand Up @@ -1013,3 +1012,23 @@ mod tests {
assert_eq!(status.details(), DETAILS);
}
}

/// Error returned if a request didn't complete within the configured timeout.
///
/// Timeouts can be configured either with [`Endpoint::timeout`], [`Server::timeout`], or by
/// setting the [`grpc-timeout` metadata value][spec].
///
/// [`Endpoint::timeout`]: crate::transport::server::Server::timeout
/// [`Server::timeout`]: crate::transport::channel::Endpoint::timeout
/// [spec]: https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md
#[derive(Debug)]
pub struct TimeoutExpired(pub ());

impl fmt::Display for TimeoutExpired {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "Timeout expired")
}
}

// std::error::Error only requires a type to impl Debug and Display
impl std::error::Error for TimeoutExpired {}
2 changes: 0 additions & 2 deletions tonic/src/transport/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,6 @@ pub use self::error::Error;
#[doc(inline)]
#[cfg(feature = "server")]
pub use self::server::Server;
#[doc(inline)]
pub use self::service::grpc_timeout::TimeoutExpired;

#[cfg(feature = "tls")]
pub use self::tls::Certificate;
Expand Down
23 changes: 1 addition & 22 deletions tonic/src/transport/service/grpc_timeout.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use crate::metadata::GRPC_TIMEOUT_HEADER;
use crate::{metadata::GRPC_TIMEOUT_HEADER, TimeoutExpired};
use http::{HeaderMap, HeaderValue, Request};
use pin_project::pin_project;
use std::{
fmt,
future::Future,
pin::Pin,
task::{ready, Context, Poll},
Expand Down Expand Up @@ -147,26 +146,6 @@ fn try_parse_grpc_timeout(
}
}

/// Error returned if a request didn't complete within the configured timeout.
///
/// Timeouts can be configured either with [`Endpoint::timeout`], [`Server::timeout`], or by
/// setting the [`grpc-timeout` metadata value][spec].
///
/// [`Endpoint::timeout`]: crate::transport::server::Server::timeout
/// [`Server::timeout`]: crate::transport::channel::Endpoint::timeout
/// [spec]: https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md
#[derive(Debug)]
pub struct TimeoutExpired(());

impl fmt::Display for TimeoutExpired {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "Timeout expired")
}
}

// std::error::Error only requires a type to impl Debug and Display
impl std::error::Error for TimeoutExpired {}

#[cfg(test)]
mod tests {
use super::*;
Expand Down

0 comments on commit 71fca36

Please sign in to comment.