Make sure Duration in config is treated as String

kamilkisiela · kamilkisiela · commit 2fe644da78e1 · 2025-11-25T11:45:25.000+01:00
diff --git a/.gemini/styleguide.md b/.gemini/styleguide.md
@@ -81,6 +81,33 @@ async fn handle(user: &User, req: &Request) -> Result<Response> {
     Ok(...)
 }
 
+---
+
+## `std::time::Duration` in `router-config` Crate
+
+When using `std::time::Duration` in the `router-config` crate **only**, you **must** add both serde and schemars attributes:
+
+```rust
+use std::time::Duration;
+
+#[derive(serde::Serialize, serde::Deserialize)]
+struct Config {
+    #[serde(
+        deserialize_with = "humantime_serde::deserialize",
+        serialize_with = "humantime_serde::serialize",
+    )]
+    #[schemars(with = "String")]
+    timeout: Duration,
+}
+```
+
+- **`#[serde(...)]`** enables human-readable time formats (e.g., `"30s"`, `"1m30s"`) in config files.
+- **`#[schemars(with = "String")]`** ensures the JSON schema correctly represents the field as a string, not as a numeric value.
+
+**Important:** This pattern applies **only** to the `router-config` crate.
+
+---
+
 ## Releasing
 
 We are using `knope` with changesets for declaring changes. If you detect a new file in a PR under `.changeset/` directory, please confirm the following rules:
diff --git a/docs/README.md b/docs/README.md
@@ -1746,7 +1746,7 @@ The path can be either absolute or relative to the router's working directory.
 |Name|Type|Description|Required|
 |----|----|-----------|--------|
 |**path**|`string`|The path to the supergraph file.<br/><br/>Can also be set using the `SUPERGRAPH_FILE_PATH` environment variable.<br/>Format: `"path"`<br/>|yes|
-|[**poll\_interval**](#option1poll_interval)|`object`, `null`|Optional interval at which the file should be polled for changes.<br/>|yes|
+|**poll\_interval**|`string`|Optional interval at which the file should be polled for changes.<br/>If not provided, the file will only be loaded once when the router starts.<br/>|no|
 |**source**|`string`|Constant Value: `"file"`<br/>|yes|
 
 **Additional Properties:** not allowed  
@@ -1768,11 +1768,11 @@ Loads a supergraph from Hive Console CDN.
 |Name|Type|Description|Required|
 |----|----|-----------|--------|
 |**accept\_invalid\_certs**|`boolean`|Whether to accept invalid TLS certificates when connecting to the Hive Console CDN.<br/>Default: `false`<br/>|no|
-|[**connect\_timeout**](#option2connect_timeout)|`object`|Connect timeout for the Hive Console CDN requests.<br/>Default: `"10s"`<br/>|yes|
+|**connect\_timeout**|`string`|Connect timeout for the Hive Console CDN requests.<br/>Default: `"10s"`<br/>|no|
 |**endpoint**|`string`|The CDN endpoint from Hive Console target.<br/><br/>Can also be set using the `HIVE_CDN_ENDPOINT` environment variable.<br/>|yes|
 |**key**|`string`|The CDN Access Token with from the Hive Console target.<br/><br/>Can also be set using the `HIVE_CDN_KEY` environment variable.<br/>|yes|
-|[**poll\_interval**](#option2poll_interval)|`object`|Interval at which the Hive Console should be polled for changes.<br/>Default: `"10s"`<br/>|yes|
-|[**request\_timeout**](#option2request_timeout)|`object`|Request timeout for the Hive Console CDN requests.<br/>Default: `"1m"`<br/>|yes|
+|**poll\_interval**|`string`|Interval at which the Hive Console should be polled for changes.<br/><br/>Can also be set using the `HIVE_CDN_POLL_INTERVAL` environment variable.<br/>Default: `"10s"`<br/>|no|
+|**request\_timeout**|`string`|Request timeout for the Hive Console CDN requests.<br/>Default: `"1m"`<br/>|no|
 |[**retry\_policy**](#option2retry_policy)|`object`|Interval at which the Hive Console should be polled for changes.<br/>Default: `{"max_retries":10}`<br/>|yes|
 |**source**|`string`|Constant Value: `"hive"`<br/>|yes|
 
@@ -1790,89 +1790,6 @@ retry_policy:
 ```
 
 
-<a name="option1poll_interval"></a>
-## Option 1: poll\_interval: object,null
-
-Optional interval at which the file should be polled for changes.
-If not provided, the file will only be loaded once when the router starts.
-
-
-**Properties**
-
-|Name|Type|Description|Required|
-|----|----|-----------|--------|
-|**nanos**|`integer`|Format: `"uint32"`<br/>Minimum: `0`<br/>|yes|
-|**secs**|`integer`|Format: `"uint64"`<br/>Minimum: `0`<br/>|yes|
-
-**Example**
-
-```yaml
-{}
-
-```
-
-<a name="option2connect_timeout"></a>
-## Option 2: connect\_timeout: object
-
-Connect timeout for the Hive Console CDN requests.
-
-
-**Properties**
-
-|Name|Type|Description|Required|
-|----|----|-----------|--------|
-|**nanos**|`integer`|Format: `"uint32"`<br/>Minimum: `0`<br/>|yes|
-|**secs**|`integer`|Format: `"uint64"`<br/>Minimum: `0`<br/>|yes|
-
-**Example**
-
-```yaml
-10s
-
-```
-
-<a name="option2poll_interval"></a>
-## Option 2: poll\_interval: object
-
-Interval at which the Hive Console should be polled for changes.
-
-Can also be set using the `HIVE_CDN_POLL_INTERVAL` environment variable.
-
-
-**Properties**
-
-|Name|Type|Description|Required|
-|----|----|-----------|--------|
-|**nanos**|`integer`|Format: `"uint32"`<br/>Minimum: `0`<br/>|yes|
-|**secs**|`integer`|Format: `"uint64"`<br/>Minimum: `0`<br/>|yes|
-
-**Example**
-
-```yaml
-10s
-
-```
-
-<a name="option2request_timeout"></a>
-## Option 2: request\_timeout: object
-
-Request timeout for the Hive Console CDN requests.
-
-
-**Properties**
-
-|Name|Type|Description|Required|
-|----|----|-----------|--------|
-|**nanos**|`integer`|Format: `"uint32"`<br/>Minimum: `0`<br/>|yes|
-|**secs**|`integer`|Format: `"uint64"`<br/>Minimum: `0`<br/>|yes|
-
-**Example**
-
-```yaml
-1m
-
-```
-
 <a name="option2retry_policy"></a>
 ## Option 2: retry\_policy: object
 
@@ -1964,7 +1881,7 @@ Optional per-subgraph configurations that will override the default configuratio
 |Name|Type|Description|Required|
 |----|----|-----------|--------|
 |**dedupe\_enabled**|`boolean`, `null`|Enables/disables request deduplication to subgraphs.<br/><br/>When requests exactly matches the hashing mechanism (e.g., subgraph name, URL, headers, query, variables), and are executed at the same time, they will<br/>be deduplicated by sharing the response of other in-flight requests.<br/>|no|
-|**pool\_idle\_timeout\_seconds**|`string`|Timeout for idle sockets being kept-alive.<br/>|yes|
+|**pool\_idle\_timeout**|`string`|Timeout for idle sockets being kept-alive.<br/>|yes|
 |**request\_timeout**||Optional timeout configuration for requests to subgraphs.<br/><br/>Example with a fixed duration:<br/>```yaml<br/>  timeout:<br/>    duration: 5s<br/>```<br/><br/>Or with a VRL expression that can return a duration based on the operation kind:<br/>```yaml<br/>  timeout:<br/>    expression: \|<br/>     if (.request.operation.type == "mutation") {<br/>       "10s"<br/>     } else {<br/>       "15s"<br/>     }<br/>```<br/>|no|
 
 **Additional Properties:** not allowed  
diff --git a/lib/executor/src/executors/map.rs b/lib/executor/src/executors/map.rs
@@ -6,7 +6,10 @@ use std::{
 
 use bytes::{BufMut, Bytes, BytesMut};
 use dashmap::DashMap;
-use hive_router_config::{override_subgraph_urls::UrlOrExpression, HiveRouterConfig};
+use hive_router_config::{
+    override_subgraph_urls::UrlOrExpression, traffic_shaping::DurationOrExpression,
+    HiveRouterConfig,
+};
 use http::Uri;
 use hyper_tls::HttpsConnector;
 use hyper_util::{
@@ -41,6 +44,12 @@ type EndpointsBySubgraphMap = DashMap<SubgraphName, SubgraphEndpoint>;
 type ExpressionsBySubgraphMap = HashMap<SubgraphName, VrlProgram>;
 type TimeoutsBySubgraph = DashMap<SubgraphName, DurationOrProgram>;
 
+struct ResolvedSubgraphConfig<'a> {
+    client: Arc<HttpClient>,
+    timeout_config: &'a DurationOrExpression,
+    dedupe_enabled: bool,
+}
+
 pub struct SubgraphExecutorMap {
     executors_by_subgraph: ExecutorsBySubgraphMap,
     /// Mapping from subgraph name to endpoint for quick lookup
@@ -317,53 +326,15 @@ impl SubgraphExecutorMap {
             .or_insert_with(|| Arc::new(Semaphore::new(self.max_connections_per_host)))
             .clone();
 
-        let mut client = self.client.clone();
-        let mut timeout_config = &self.config.traffic_shaping.all.request_timeout;
-        let pool_idle_timeout = self.config.traffic_shaping.all.pool_idle_timeout;
-        let mut dedupe_enabled = self.config.traffic_shaping.all.dedupe_enabled;
-        if let Some(subgraph_traffic_shaping_config) =
-            self.config.traffic_shaping.subgraphs.get(subgraph_name)
-        {
-            if subgraph_traffic_shaping_config.pool_idle_timeout.is_some() {
-                client = Arc::new(
-                    Client::builder(TokioExecutor::new())
-                        .pool_timer(TokioTimer::new())
-                        .pool_idle_timeout(
-                            subgraph_traffic_shaping_config
-                                .pool_idle_timeout
-                                .unwrap_or(pool_idle_timeout),
-                        )
-                        .pool_max_idle_per_host(self.max_connections_per_host)
-                        .build(HttpsConnector::new()),
-                );
-            }
-            dedupe_enabled = subgraph_traffic_shaping_config
-                .dedupe_enabled
-                .unwrap_or(dedupe_enabled);
-            timeout_config = subgraph_traffic_shaping_config
-                .request_timeout
-                .as_ref()
-                .unwrap_or(timeout_config);
-        }
-
-        if !self.timeouts_by_subgraph.contains_key(subgraph_name) {
-            let timeout_prog: DurationOrProgram = compile_duration_expression(timeout_config, None)
-                .map_err(|err| {
-                    SubgraphExecutorError::RequestTimeoutExpressionBuild(
-                        subgraph_name.to_string(),
-                        err,
-                    )
-                })?;
-            self.timeouts_by_subgraph
-                .insert(subgraph_name.to_string(), timeout_prog);
-        }
+        let subgraph_config = self.resolve_subgraph_config(subgraph_name);
+        self.register_timeout_if_absent(subgraph_name, &subgraph_config.timeout_config)?;
 
         let executor = HTTPSubgraphExecutor::new(
             subgraph_name.to_string(),
             endpoint_uri,
-            client,
+            subgraph_config.client,
             semaphore,
-            dedupe_enabled,
+            subgraph_config.dedupe_enabled,
             self.in_flight_requests.clone(),
         );
 
@@ -374,6 +345,62 @@ impl SubgraphExecutorMap {
 
         Ok(())
     }
+
+    /// Resolves traffic shaping configuration for a specific subgraph, applying subgraph-specific
+    /// overrides on top of global settings
+    fn resolve_subgraph_config<'a>(&'a self, subgraph_name: &'a str) -> ResolvedSubgraphConfig<'a> {
+        let mut config = ResolvedSubgraphConfig {
+            client: self.client.clone(),
+            timeout_config: &self.config.traffic_shaping.all.request_timeout,
+            dedupe_enabled: self.config.traffic_shaping.all.dedupe_enabled,
+        };
+
+        let Some(subgraph_config) = self.config.traffic_shaping.subgraphs.get(subgraph_name) else {
+            return config;
+        };
+
+        // Override client only if pool idle timeout is customized
+        if let Some(pool_idle_timeout) = subgraph_config.pool_idle_timeout {
+            config.client = Arc::new(
+                Client::builder(TokioExecutor::new())
+                    .pool_timer(TokioTimer::new())
+                    .pool_idle_timeout(pool_idle_timeout)
+                    .pool_max_idle_per_host(self.max_connections_per_host)
+                    .build(HttpsConnector::new()),
+            );
+        }
+
+        // Apply other subgraph-specific overrides
+        if let Some(dedupe_enabled) = subgraph_config.dedupe_enabled {
+            config.dedupe_enabled = dedupe_enabled;
+        }
+
+        if let Some(custom_timeout) = &subgraph_config.request_timeout {
+            config.timeout_config = custom_timeout;
+        }
+
+        config
+    }
+
+    /// Compiles and caches the timeout configuration for the given subgraph if not already registered
+    fn register_timeout_if_absent(
+        &self,
+        subgraph_name: &str,
+        timeout_config: &hive_router_config::traffic_shaping::DurationOrExpression,
+    ) -> Result<(), SubgraphExecutorError> {
+        if self.timeouts_by_subgraph.contains_key(subgraph_name) {
+            return Ok(());
+        }
+
+        let timeout_prog = compile_duration_expression(timeout_config, None).map_err(|err| {
+            SubgraphExecutorError::RequestTimeoutExpressionBuild(subgraph_name.to_string(), err)
+        })?;
+
+        self.timeouts_by_subgraph
+            .insert(subgraph_name.to_string(), timeout_prog);
+
+        Ok(())
+    }
 }
 
 fn resolve_duration_prog(
diff --git a/lib/router-config/src/supergraph.rs b/lib/router-config/src/supergraph.rs
@@ -23,6 +23,7 @@ pub enum SupergraphSource {
             deserialize_with = "humantime_serde::deserialize",
             serialize_with = "humantime_serde::serialize"
         )]
+        #[schemars(with = "String")]
         poll_interval: Option<Duration>,
     },
     /// Loads a supergraph from Hive Console CDN.
@@ -44,20 +45,23 @@ pub enum SupergraphSource {
             deserialize_with = "humantime_serde::deserialize",
             serialize_with = "humantime_serde::serialize"
         )]
+        #[schemars(with = "String")]
         poll_interval: Duration,
         /// Request timeout for the Hive Console CDN requests.
         #[serde(
             default = "default_hive_request_timeout",
             deserialize_with = "humantime_serde::deserialize",
             serialize_with = "humantime_serde::serialize"
         )]
+        #[schemars(with = "String")]
         request_timeout: Duration,
         /// Connect timeout for the Hive Console CDN requests.
         #[serde(
             default = "default_hive_connect_timeout",
             deserialize_with = "humantime_serde::deserialize",
             serialize_with = "humantime_serde::serialize"
         )]
+        #[schemars(with = "String")]
         connect_timeout: Duration,
         /// Whether to accept invalid TLS certificates when connecting to the Hive Console CDN.
         #[serde(default = "default_accept_invalid_certs")]
diff --git a/lib/router-config/src/traffic_shaping.rs b/lib/router-config/src/traffic_shaping.rs
@@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize};
 pub struct TrafficShapingConfig {
     /// The default configuration that will be applied to all subgraphs, unless overridden by a specific subgraph configuration.
     #[serde(default)]
-    pub all: TrafficShapingExecutorConfig,
+    pub all: TrafficShapingExecutorGlobalConfig,
     /// Optional per-subgraph configurations that will override the default configuration for specific subgraphs.
     #[serde(default, skip_serializing_if = "HashMap::is_empty")]
     pub subgraphs: HashMap<String, TrafficShapingExecutorSubgraphConfig>,
@@ -20,7 +20,7 @@ pub struct TrafficShapingConfig {
 impl Default for TrafficShapingConfig {
     fn default() -> Self {
         Self {
-            all: TrafficShapingExecutorConfig::default(),
+            all: TrafficShapingExecutorGlobalConfig::default(),
             subgraphs: HashMap::new(),
             max_connections_per_host: default_max_connections_per_host(),
         }
@@ -79,7 +79,7 @@ pub struct TrafficShapingExecutorSubgraphConfig {
 
 #[derive(Debug, Deserialize, Serialize, JsonSchema, Clone)]
 #[serde(deny_unknown_fields)]
-pub struct TrafficShapingExecutorConfig {
+pub struct TrafficShapingExecutorGlobalConfig {
     /// Timeout for idle sockets being kept-alive.
     #[serde(
         default = "default_pool_idle_timeout",
@@ -130,12 +130,13 @@ pub enum DurationOrExpression {
         deserialize_with = "humantime_serde::deserialize",
         serialize_with = "humantime_serde::serialize"
     )]
+    #[schemars(with = "String")]
     Duration(Duration),
     /// A VRL expression that evaluates to a duration. The result can be an integer (milliseconds), a float (milliseconds), or a duration string (e.g. "5s").
     Expression { expression: String },
 }
 
-impl Default for TrafficShapingExecutorConfig {
+impl Default for TrafficShapingExecutorGlobalConfig {
     fn default() -> Self {
         Self {
             pool_idle_timeout: default_pool_idle_timeout(),
