feat(orphan-hoist-config): adds config for hoisting orphaned errors (backport #8998)

.changesets/exp_hoist_orphan_errors.md

@@ -0,0 +1,36 @@
+### Add `experimental_hoist_orphan_errors` configuration for controlling orphan error path assignment
+
+Adds a new `experimental_hoist_orphan_errors` configuration that controls how entity-less ("orphan") errors from subgraphs are assigned paths in the response. When enabled for a subgraph, orphan errors are assigned to the nearest non-array ancestor in the response path, preventing them from being duplicated across every element in an array. This can be enabled globally via `all` or per-subgraph via the `subgraphs` map. Per-subgraph settings override `all`.
+
+Here's an example when targeting a specific subgraph, `my_subgraph`:
+
+```yaml
+experimental_hoist_orphan_errors:
+  subgraphs:
+    my_subgraph:
+      enabled: true
+```
+
+An example when targeting all subgraphs:
+
+```yaml
+experimental_hoist_orphan_errors:
+  all:
+    enabled: true
+```
+
+And an example enabling for all subgraphs except one:
+
+```yaml
+experimental_hoist_orphan_errors:
+  all:
+    enabled: true
+  subgraphs:
+    noisy_one:
+      enabled: false
+```
+
+Using this feature should only happen if you know you have subgraphs that don't respond with the correct paths when making entity calls. If you're unsure, you probably don't need this!
+
+
+By [@aaronArinder](https://github.com/aaronArinder) in https://github.com/apollographql/router/pull/8998

apollo-router/src/configuration/mod.rs

@@ -222,6 +222,13 @@ pub struct Configuration {
     /// Type conditioned fetching configuration.
     #[serde(default)]
     pub(crate) experimental_type_conditioned_fetching: bool,
+
+    /// When enabled for specific subgraphs, orphan errors (those without a valid
+    /// `_entities` path) are assigned to the nearest non-array ancestor in the
+    /// response path, preventing them from being duplicated across every array
+    /// element.
+    #[serde(default)]
+    pub(crate) experimental_hoist_orphan_errors: SubgraphConfiguration<HoistOrphanErrors>,
 }
 
 impl PartialEq for Configuration {
@@ -256,6 +263,7 @@ impl<'de> serde::Deserialize<'de> for Configuration {
             experimental_chaos: chaos::Config,
             batching: Batching,
             experimental_type_conditioned_fetching: bool,
+            experimental_hoist_orphan_errors: SubgraphConfiguration<HoistOrphanErrors>,
         }
         let mut ad_hoc: AdHocConfiguration = serde::Deserialize::deserialize(deserializer)?;
 
@@ -287,6 +295,7 @@ impl<'de> serde::Deserialize<'de> for Configuration {
             limits: ad_hoc.limits,
             experimental_chaos: ad_hoc.experimental_chaos,
             experimental_type_conditioned_fetching: ad_hoc.experimental_type_conditioned_fetching,
+            experimental_hoist_orphan_errors: ad_hoc.experimental_hoist_orphan_errors,
             plugins: ad_hoc.plugins,
             apollo_plugins: ad_hoc.apollo_plugins,
             batching: ad_hoc.batching,
@@ -327,6 +336,7 @@ impl Configuration {
         chaos: Option<chaos::Config>,
         uplink: Option<UplinkConfig>,
         experimental_type_conditioned_fetching: Option<bool>,
+        experimental_hoist_orphan_errors: Option<SubgraphConfiguration<HoistOrphanErrors>>,
         batching: Option<Batching>,
         server: Option<Server>,
     ) -> Result<Self, ConfigurationError> {
@@ -356,6 +366,7 @@ impl Configuration {
             batching: batching.unwrap_or_default(),
             experimental_type_conditioned_fetching: experimental_type_conditioned_fetching
                 .unwrap_or_default(),
+            experimental_hoist_orphan_errors: experimental_hoist_orphan_errors.unwrap_or_default(),
             notify,
         };
 
@@ -500,6 +511,7 @@ impl Configuration {
             uplink,
             experimental_type_conditioned_fetching: experimental_type_conditioned_fetching
                 .unwrap_or_default(),
+            experimental_hoist_orphan_errors: Default::default(),
             batching: batching.unwrap_or_default(),
             raw_yaml: None,
         };
@@ -1574,3 +1586,17 @@ impl Batching {
         }
     }
 }
+
+/// Per-subgraph configuration for hoisting orphan errors.
+///
+/// "Orphan errors" are errors from entity fetches that lack a valid `_entities` path.
+/// When hoisting is enabled, these errors are assigned to the nearest non-array
+/// ancestor in the response path, preventing them from being duplicated across
+/// every element in an array.
+#[derive(Debug, Clone, Default, Deserialize, Serialize, JsonSchema)]
+#[serde(deny_unknown_fields)]
+pub(crate) struct HoistOrphanErrors {
+    /// Enable hoisting of orphan errors for this subgraph.
+    #[serde(default)]
+    pub(crate) enabled: bool,
+}

apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap

@@ -5481,6 +5481,18 @@ expression: "&schema"
         }
       ]
     },
+    "HoistOrphanErrors": {
+      "additionalProperties": false,
+      "description": "Per-subgraph configuration for hoisting orphan errors.\n\n\"Orphan errors\" are errors from entity fetches that lack a valid `_entities` path.\nWhen hoisting is enabled, these errors are assigned to the nearest non-array\nancestor in the response path, preventing them from being duplicated across\nevery element in an array.",
+      "properties": {
+        "enabled": {
+          "default": false,
+          "description": "Enable hoisting of orphan errors for this subgraph.",
+          "type": "boolean"
+        }
+      },
+      "type": "object"
+    },
     "Homepage": {
       "additionalProperties": false,
       "description": "Configuration options pertaining to the home page.",
@@ -9476,6 +9488,31 @@ expression: "&schema"
       },
       "type": "object"
     },
+    "SubgraphHoistOrphanErrorsConfiguration": {
+      "description": "Configuration options pertaining to the subgraph server component.",
+      "properties": {
+        "all": {
+          "allOf": [
+            {
+              "$ref": "#/definitions/HoistOrphanErrors"
+            }
+          ],
+          "default": {
+            "enabled": false
+          },
+          "description": "options applying to all subgraphs"
+        },
+        "subgraphs": {
+          "additionalProperties": {
+            "$ref": "#/definitions/HoistOrphanErrors"
+          },
+          "default": {},
+          "description": "per subgraph options",
+          "type": "object"
+        }
+      },
+      "type": "object"
+    },
     "SubgraphInvalidationConfig": {
       "additionalProperties": false,
       "properties": {
@@ -12065,6 +12102,20 @@ expression: "&schema"
     "experimental_diagnostics": {
       "$ref": "#/definitions/Config5"
     },
+    "experimental_hoist_orphan_errors": {
+      "allOf": [
+        {
+          "$ref": "#/definitions/SubgraphHoistOrphanErrorsConfiguration"
+        }
+      ],
+      "default": {
+        "all": {
+          "enabled": false
+        },
+        "subgraphs": {}
+      },
+      "description": "When enabled for specific subgraphs, orphan errors (those without a valid\n`_entities` path) are assigned to the nearest non-array ancestor in the\nresponse path, preventing them from being duplicated across every array\nelement."
+    },
     "experimental_type_conditioned_fetching": {
       "default": false,
       "description": "Type conditioned fetching configuration.",

apollo-router/src/configuration/tests.rs

@@ -1322,3 +1322,106 @@ fn it_prevents_enablement_of_both_subgraph_caching_plugins() {
         serde_json::from_value(make_config(Some(true), Some(true)));
     config_result.expect_err("both plugins configured");
 }
+
+#[rstest::rstest]
+#[case::all_enabled("some_subgraph_name", true, &[], true)]
+#[case::all_enabled_unknown("unknown", true, &[], true)]
+#[case::subgraph_enabled("some_subgraph_name", false, &[("some_subgraph_name", true)], true)]
+#[case::subgraph_disabled("disabled", false, &[("disabled", false)], false)]
+#[case::subgraph_unknown_falls_back_to_all("unknown", false, &[("some_subgraph_name", true)], false)]
+#[case::default_hoists_nothing("anything", false, &[], false)]
+#[case::all_with_subgraph_override("overridden", true, &[("overridden", false)], false)]
+fn hoist_orphan_errors_get(
+    #[case] query_subgraph: &str,
+    #[case] all_enabled: bool,
+    #[case] subgraph_entries: &[(&str, bool)],
+    #[case] expected: bool,
+) {
+    let config = super::subgraph::SubgraphConfiguration {
+        all: super::HoistOrphanErrors {
+            enabled: all_enabled,
+        },
+        subgraphs: subgraph_entries
+            .iter()
+            .map(|(k, v)| (k.to_string(), super::HoistOrphanErrors { enabled: *v }))
+            .collect(),
+    };
+    assert_eq!(config.get(query_subgraph).enabled, expected);
+}
+
+#[test]
+fn hoist_orphan_errors_deserializes_with_subgraphs() {
+    let config: Configuration = serde_json::from_value(json!({
+        "experimental_hoist_orphan_errors": {
+            "subgraphs": {
+                "some_subgraph_name": {
+                    "enabled": true
+                },
+                "other": {
+                    "enabled": false
+                }
+            }
+        }
+    }))
+    .expect("valid config");
+    assert!(
+        config
+            .experimental_hoist_orphan_errors
+            .get("some_subgraph_name")
+            .enabled
+    );
+    assert!(!config.experimental_hoist_orphan_errors.get("other").enabled);
+    assert!(
+        !config
+            .experimental_hoist_orphan_errors
+            .get("unknown")
+            .enabled
+    );
+}
+
+#[test]
+fn hoist_orphan_errors_deserializes_with_all() {
+    let config: Configuration = serde_json::from_value(json!({
+        "experimental_hoist_orphan_errors": {
+            "all": {
+                "enabled": true
+            }
+        }
+    }))
+    .expect("valid config");
+    assert!(
+        config
+            .experimental_hoist_orphan_errors
+            .get("anything")
+            .enabled
+    );
+}
+
+#[test]
+fn hoist_orphan_errors_all_with_subgraph_override() {
+    let config: Configuration = serde_json::from_value(json!({
+        "experimental_hoist_orphan_errors": {
+            "all": {
+                "enabled": true
+            },
+            "subgraphs": {
+                "noisy_one": {
+                    "enabled": false
+                }
+            }
+        }
+    }))
+    .expect("valid config");
+    assert!(
+        config
+            .experimental_hoist_orphan_errors
+            .get("anything")
+            .enabled
+    );
+    assert!(
+        !config
+            .experimental_hoist_orphan_errors
+            .get("noisy_one")
+            .enabled
+    );
+}