diff --git a/Cargo.lock b/Cargo.lock index eb7a1de5a..5828c91c1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6089,6 +6089,7 @@ dependencies = [ "bytestring", "derive_builder", "futures", + "h2 0.3.26", "http-serde", "humantime", "hyper 0.14.28", diff --git a/cli/src/commands/deployments/register.rs b/cli/src/commands/deployments/register.rs index 63f5b41d5..adfdeeab6 100644 --- a/cli/src/commands/deployments/register.rs +++ b/cli/src/commands/deployments/register.rs @@ -52,6 +52,11 @@ pub struct Register { #[clap(long="extra-header", value_parser = parse_header, action = clap::ArgAction::Append)] extra_headers: Option>, + /// Attempt discovery using a client that defaults to HTTP1.1 instead of a prior-knowledge HTTP2 client. + /// This may be necessary if you see `META0014` discovering local dev servers like `wrangler dev`. + #[clap(long = "use-http1.1")] + use_http_11: bool, + /// The URL or ARN that Restate server needs to fetch service information from. /// /// The URL must be network-accessible from Restate server. In case of using @@ -175,6 +180,7 @@ pub async fn run_register(State(env): State, discover_opts: &Register) - DeploymentEndpoint::Uri(uri) => RegisterDeploymentRequest::Http { uri: uri.clone(), additional_headers: headers.clone().map(Into::into), + use_http_11: discover_opts.use_http_11, force, dry_run, }, diff --git a/crates/admin-rest-model/src/deployments.rs b/crates/admin-rest-model/src/deployments.rs index 9e6030bda..66ae4cc53 100644 --- a/crates/admin-rest-model/src/deployments.rs +++ b/crates/admin-rest-model/src/deployments.rs @@ -104,6 +104,17 @@ pub enum RegisterDeploymentRequest { /// Additional headers added to the discover/invoke requests to the deployment. /// additional_headers: Option, + + /// # Use http1.1 + /// + /// If `true`, discovery will be attempted using a client that defaults to HTTP1.1 + /// instead of a prior-knowledge HTTP2 client. HTTP2 may still be used for TLS servers + /// that advertise HTTP2 support via ALPN. HTTP1.1 deployments will only work in + /// request-response mode. + /// + #[serde(default = "restate_serde_util::default::bool::")] + use_http_11: bool, + /// # Force /// /// If `true`, it will override, if existing, any deployment using the same `uri`. diff --git a/crates/admin/src/rest_api/deployments.rs b/crates/admin/src/rest_api/deployments.rs index 2e45a0903..2abc47319 100644 --- a/crates/admin/src/rest_api/deployments.rs +++ b/crates/admin/src/rest_api/deployments.rs @@ -48,11 +48,19 @@ pub async fn create_deployment( RegisterDeploymentRequest::Http { uri, additional_headers, + use_http_11, force, dry_run, } => ( DiscoverEndpoint::new( - Endpoint::Http(uri, Default::default()), + Endpoint::Http( + uri, + if use_http_11 { + http::Version::HTTP_11 + } else { + http::Version::HTTP_2 + }, + ), additional_headers.unwrap_or_default().into(), ), force, diff --git a/crates/errors/src/error_codes/META0014.md b/crates/errors/src/error_codes/META0014.md new file mode 100644 index 000000000..0e80e344e --- /dev/null +++ b/crates/errors/src/error_codes/META0014.md @@ -0,0 +1,9 @@ +## META0014 + +Service discovery response failed, and the server may have responded in HTTP1.1. +This can happen when discovering locally running dev servers from Faas platforms +eg `wrangler dev`. FaaS platforms in generally will support HTTP2, however, so +this is only a local development concern. + +You can try to discover the endpoint with `--use-http1.1` when working +with these local dev servers. This should not be needed in production. diff --git a/crates/errors/src/lib.rs b/crates/errors/src/lib.rs index 513270ced..4a5d6bfc5 100644 --- a/crates/errors/src/lib.rs +++ b/crates/errors/src/lib.rs @@ -37,7 +37,7 @@ mod helper; declare_restate_error_codes!( RT0001, RT0002, RT0003, RT0004, RT0005, RT0006, RT0007, RT0009, RT0010, RT0011, RT0012, RT0013, RT0014, META0003, META0004, META0005, META0006, META0009, META0010, META0011, META0012, - META0013 + META0013, META0014 ); // -- Some commonly used errors diff --git a/crates/service-client/Cargo.toml b/crates/service-client/Cargo.toml index 342d2ed90..e263e83be 100644 --- a/crates/service-client/Cargo.toml +++ b/crates/service-client/Cargo.toml @@ -25,6 +25,7 @@ http-serde = { version = "1.1.2" } humantime = { workspace = true } hyper = { workspace = true } hyper-rustls = { workspace = true } +h2 = { version = "0.3.20" } once_cell = { workspace = true } rustls = { workspace = true } serde = { workspace = true } diff --git a/crates/service-client/src/http.rs b/crates/service-client/src/http.rs index 988506112..884703dd6 100644 --- a/crates/service-client/src/http.rs +++ b/crates/service-client/src/http.rs @@ -20,26 +20,34 @@ use hyper::http::HeaderValue; use hyper::{Body, HeaderMap, Method, Request, Response, Uri, Version}; use hyper_rustls::HttpsConnector; use restate_types::config::HttpOptions; +use std::error::Error; use std::fmt::Debug; use std::future; use std::future::Future; - type Connector = ProxyConnector>; #[derive(Clone, Debug)] pub struct HttpClient { - client: hyper::Client, + // alpn client defaults to http1.1, but can upgrade to http2 using ALPN for TLS servers + alpn_client: hyper::Client, + // h2 client defaults to http2 and so supports unencrypted http2 servers + h2_client: hyper::Client, } impl HttpClient { - pub fn new(client: hyper::Client) -> Self { - Self { client } + pub fn new( + alpn_client: hyper::Client, + h2_client: hyper::Client, + ) -> Self { + Self { + alpn_client, + h2_client, + } } pub fn from_options(options: &HttpOptions) -> HttpClient { let mut builder = hyper::Client::builder(); builder - .http2_only(true) .http2_keep_alive_timeout(options.http_keep_alive_options.timeout.into()) .http2_keep_alive_interval(Some(options.http_keep_alive_options.interval.into())); @@ -48,15 +56,20 @@ impl HttpClient { http_connector.set_nodelay(true); http_connector.set_connect_timeout(Some(options.connect_timeout.into())); + let https_connector = hyper_rustls::HttpsConnectorBuilder::new() + .with_native_roots() + .https_or_http() + .enable_http2() + .wrap_connector(http_connector); + + let proxy_connector = ProxyConnector::new(options.http_proxy.clone(), https_connector); + HttpClient::new( - builder.build::<_, hyper::Body>(ProxyConnector::new( - options.http_proxy.clone(), - hyper_rustls::HttpsConnectorBuilder::new() - .with_native_roots() - .https_or_http() - .enable_http2() - .wrap_connector(http_connector), - )), + builder.clone().build::<_, Body>(proxy_connector.clone()), // h1 client with alpn upgrade support + { + builder.http2_only(true); + builder.build::<_, hyper::Body>(proxy_connector) // h2-prior knowledge client + }, ) } @@ -117,18 +130,44 @@ impl HttpClient { Err(err) => return future::ready(Err(err.into())).right_future(), }; - let fut = self.client.request(request); + let client = match request.version() { + Version::HTTP_2 => &self.h2_client, + _ => &self.alpn_client, + }; - Either::Left(async move { Ok(fut.await?) }) + let fut = client.request(request); + + Either::Left(async move { + match fut.await { + Ok(res) => Ok(res), + Err(err) if is_possible_h11_only_error(&err) => { + Err(HttpError::PossibleHTTP11Only(err)) + } + Err(err) => Err(HttpError::Hyper(err)), + } + }) } } +fn is_possible_h11_only_error(err: &hyper::Error) -> bool { + // this is the error we see from the h2 lib when the server sends back an http1.1 response + // to an http2 request. http2 is designed to start requests with what looks like an invalid + // HTTP1.1 method, so typically 1.1 servers respond with a 40x, and the h2 client sees + // this as an invalid frame. + err.source() + .and_then(|err| err.downcast_ref::()) + .and_then(|err| err.reason()) + == Some(h2::Reason::FRAME_SIZE_ERROR) +} + #[derive(Debug, thiserror::Error)] pub enum HttpError { #[error(transparent)] Hyper(#[from] hyper::Error), #[error(transparent)] Http(#[from] hyper::http::Error), + #[error("server possibly only supports HTTP1.1, consider discovery with --use-http1.1: {0}")] + PossibleHTTP11Only(#[source] hyper::Error), } impl HttpError { @@ -138,6 +177,7 @@ impl HttpError { match self { HttpError::Hyper(err) => err.is_retryable(), HttpError::Http(err) => err.is_retryable(), + HttpError::PossibleHTTP11Only(_) => false, } } } diff --git a/crates/service-client/src/lib.rs b/crates/service-client/src/lib.rs index 1d5007fc5..fa569fa02 100644 --- a/crates/service-client/src/lib.rs +++ b/crates/service-client/src/lib.rs @@ -27,6 +27,7 @@ use std::future; use std::future::Future; use std::sync::Arc; +pub use crate::http::HttpError; pub use crate::lambda::AssumeRoleCacheMode; use crate::request_identity::SignRequest; diff --git a/crates/service-protocol/src/discovery.rs b/crates/service-protocol/src/discovery.rs index b8e2cf887..25d84c828 100644 --- a/crates/service-protocol/src/discovery.rs +++ b/crates/service-protocol/src/discovery.rs @@ -17,7 +17,7 @@ use hyper::http::{HeaderName, HeaderValue}; use hyper::{Body, HeaderMap, StatusCode}; use itertools::Itertools; use once_cell::sync::Lazy; -use restate_errors::{META0003, META0012, META0013}; +use restate_errors::{META0003, META0012, META0013, META0014}; use restate_schema_api::deployment::ProtocolType; use restate_service_client::{Endpoint, Method, Parts, Request, ServiceClient, ServiceClientError}; use restate_types::endpoint_manifest; @@ -112,31 +112,42 @@ pub struct DiscoveredMetadata { pub supported_protocol_versions: RangeInclusive, } -#[derive(Debug, thiserror::Error, CodedError)] +#[derive(Debug, thiserror::Error)] pub enum DiscoveryError { // Errors most likely related to SDK bugs #[error("received a bad response from the SDK: {0}")] - #[code(META0013)] BadResponse(Cow<'static, str>), #[error( "received a bad response from the SDK that cannot be decoded: {0}. Discovery response: {}", String::from_utf8_lossy(.1) )] - #[code(unknown)] Decode(#[source] serde_json::Error, Bytes), // Network related retryable errors #[error("bad status code: {0}")] - #[code(META0003)] BadStatusCode(u16), #[error("client error: {0}")] - #[code(META0003)] Client(#[from] ServiceClientError), #[error("unsupported service protocol versions: [{min_version}, {max_version}]. Supported versions by this runtime are [{}, {}]", i32::from(MIN_SERVICE_PROTOCOL_VERSION), i32::from(MAX_SERVICE_PROTOCOL_VERSION))] - #[code(META0012)] UnsupportedServiceProtocol { min_version: i32, max_version: i32 }, } +impl CodedError for DiscoveryError { + fn code(&self) -> Option<&'static codederror::Code> { + match self { + DiscoveryError::BadResponse(_) => Some(&META0013), + DiscoveryError::Decode(_, _) => None, + DiscoveryError::BadStatusCode(_) => Some(&META0003), + // special code for possible http1.1 errors + DiscoveryError::Client(ServiceClientError::Http( + restate_service_client::HttpError::PossibleHTTP11Only(_), + )) => Some(&META0014), + DiscoveryError::Client(_) => Some(&META0003), + DiscoveryError::UnsupportedServiceProtocol { .. } => Some(&META0012), + } + } +} + impl DiscoveryError { /// Retryable errors are those which can be caused by transient faults and where /// retrying can succeed.