Add extensions.service for all subgraph errors

apollographql · Nov 5, 2024 · e87384f · e87384f
1 parent 36bdb5e
commit e87384f
Show file tree

Hide file tree

Showing 11 changed files with 191 additions and 36 deletions.
diff --git a/...src/axum_factory/snapshots/apollo_router__axum_factory__tests__defer_is_not_buffered.snap b/...src/axum_factory/snapshots/apollo_router__axum_factory__tests__defer_is_not_buffered.snap
@@ -26,7 +26,8 @@ expression: parts
           "@"
         ],
         "extensions": {
-          "code": "FETCH_ERROR"
+          "code": "FETCH_ERROR",
+          "service": "reviews"
         }
       }
     ],

diff --git a/...src/plugins/cache/snapshots/apollo_router__plugins__cache__tests__missing_entities-2.snap b/...src/plugins/cache/snapshots/apollo_router__plugins__cache__tests__missing_entities-2.snap
@@ -28,7 +28,10 @@ expression: response
         "currentUser",
         "allOrganizations",
         2
-      ]
+      ],
+      "extensions": {
+        "service": "orga"
+      }
     }
   ]
 }
diff --git a/apollo-router/src/plugins/include_subgraph_errors.rs b/apollo-router/src/plugins/include_subgraph_errors.rs
@@ -42,36 +42,47 @@ impl Plugin for IncludeSubgraphErrors {
     }
 
     fn subgraph_service(&self, name: &str, service: subgraph::BoxService) -> subgraph::BoxService {
-        // Search for subgraph in our configured subgraph map.
-        // If we can't find it, use the "all" value
-        if !*self.config.subgraphs.get(name).unwrap_or(&self.config.all) {
-            let sub_name_response = name.to_string();
-            let sub_name_error = name.to_string();
-            return service
-                .map_response(move |mut response: SubgraphResponse| {
-                    if !response.response.body().errors.is_empty() {
+        // Search for subgraph in our configured subgraph map. If we can't find it, use the "all" value
+        let include_subgraph_errors = *self.config.subgraphs.get(name).unwrap_or(&self.config.all);
+
+        let sub_name_response = name.to_string();
+        let sub_name_error = name.to_string();
+        return service
+            .map_response(move |mut response: SubgraphResponse| {
+                let errors = &mut response.response.body_mut().errors;
+                if !errors.is_empty() {
+                    if include_subgraph_errors {
+                        for error in errors.iter_mut() {
+                            error
+                                .extensions
+                                .entry("service")
+                                .or_insert(sub_name_response.clone().into());
+                        }
+                    } else {
                         tracing::info!("redacted subgraph({sub_name_response}) errors");
-                        for error in response.response.body_mut().errors.iter_mut() {
+                        for error in errors.iter_mut() {
                             error.message = REDACTED_ERROR_MESSAGE.to_string();
                             error.extensions = Object::default();
                         }
                     }
-                    response
-                })
-                // _error to stop clippy complaining about unused assignments...
-                .map_err(move |mut _error: BoxError| {
+                }
+
+                response
+            })
+            .map_err(move |error: BoxError| {
+                if include_subgraph_errors {
+                    error
+                } else {
                     // Create a redacted error to replace whatever error we have
                     tracing::info!("redacted subgraph({sub_name_error}) error");
-                    _error = Box::new(crate::error::FetchError::SubrequestHttpError {
+                    Box::new(crate::error::FetchError::SubrequestHttpError {
                         status_code: None,
                         service: "redacted".to_string(),
                         reason: "redacted".to_string(),
-                    });
-                    _error
-                })
-                .boxed();
-        }
-        service
+                    })
+                }
+            })
+            .boxed();
     }
 }
 
@@ -104,7 +115,7 @@ mod test {
     use crate::Configuration;
 
     static UNREDACTED_PRODUCT_RESPONSE: Lazy<Bytes> = Lazy::new(|| {
-        Bytes::from_static(r#"{"data":{"topProducts":null},"errors":[{"message":"couldn't find mock for query {\"query\":\"query($first: Int) { topProducts(first: $first) { __typename upc } }\",\"variables\":{\"first\":2}}","path":[],"extensions":{"test":"value","code":"FETCH_ERROR"}}]}"#.as_bytes())
+        Bytes::from_static(r#"{"data":{"topProducts":null},"errors":[{"message":"couldn't find mock for query {\"query\":\"query($first: Int) { topProducts(first: $first) { __typename upc } }\",\"variables\":{\"first\":2}}","path":[],"extensions":{"test":"value","code":"FETCH_ERROR","service":"products"}}]}"#.as_bytes())
     });
 
     static REDACTED_PRODUCT_RESPONSE: Lazy<Bytes> = Lazy::new(|| {

diff --git a/...llo_router__services__supergraph__tests__errors_from_primary_on_deferred_responses-2.snap b/...llo_router__services__supergraph__tests__errors_from_primary_on_deferred_responses-2.snap
@@ -24,7 +24,10 @@ expression: stream.next_response().await.unwrap()
           "path": [
             "computer",
             "errorField"
-          ]
+          ],
+          "extensions": {
+            "service": "computers"
+          }
         }
       ]
     }

diff --git a/...snapshots/apollo_router__services__supergraph__tests__errors_on_deferred_responses-2.snap b/...snapshots/apollo_router__services__supergraph__tests__errors_on_deferred_responses-2.snap
@@ -17,7 +17,10 @@ expression: stream.next_response().await.unwrap()
           "message": "error user 0",
           "path": [
             "currentUser"
-          ]
+          ],
+          "extensions": {
+            "service": "user"
+          }
         }
       ]
     }

diff --git a/...pshots/apollo_router__services__supergraph__tests__errors_on_incremental_responses-2.snap b/...pshots/apollo_router__services__supergraph__tests__errors_on_incremental_responses-2.snap
@@ -23,7 +23,10 @@ expression: stream.next_response().await.unwrap()
             "activeOrganization",
             "suborga",
             0
-          ]
+          ],
+          "extensions": {
+            "service": "orga"
+          }
         }
       ]
     },
@@ -56,7 +59,10 @@ expression: stream.next_response().await.unwrap()
             "activeOrganization",
             "suborga",
             2
-          ]
+          ],
+          "extensions": {
+            "service": "orga"
+          }
         }
       ]
     }

diff --git a/...raph/snapshots/apollo_router__services__supergraph__tests__errors_on_nullified_paths.snap b/...raph/snapshots/apollo_router__services__supergraph__tests__errors_on_nullified_paths.snap
@@ -20,7 +20,8 @@ expression: stream.next_response().await.unwrap()
         "bar"
       ],
       "extensions": {
-        "code": "NOT_FOUND"
+        "code": "NOT_FOUND",
+        "service": "S2"
       }
     }
   ]

diff --git a/...es/supergraph/snapshots/apollo_router__services__supergraph__tests__missing_entities.snap b/...es/supergraph/snapshots/apollo_router__services__supergraph__tests__missing_entities.snap
@@ -18,7 +18,10 @@ expression: stream.next_response().await.unwrap()
       "path": [
         "currentUser",
         "activeOrganization"
-      ]
+      ],
+      "extensions": {
+        "service": "orga"
+      }
     }
   ]
 }
diff --git a/...ion/snapshots/integration_tests__integration__traffic_shaping__subgraph_rate_limit-2.snap b/...ion/snapshots/integration_tests__integration__traffic_shaping__subgraph_rate_limit-2.snap
@@ -2,4 +2,4 @@
 source: apollo-router/tests/integration/traffic_shaping.rs
 expression: response
 ---
-"{\"data\":null,\"errors\":[{\"message\":\"Your request has been rate limited\",\"path\":[],\"extensions\":{\"code\":\"REQUEST_RATE_LIMITED\"}}]}"
+"{\"data\":null,\"errors\":[{\"message\":\"Your request has been rate limited\",\"path\":[],\"extensions\":{\"code\":\"REQUEST_RATE_LIMITED\",\"service\":\"products\"}}]}"
diff --git a/...egration/snapshots/integration_tests__integration__traffic_shaping__subgraph_timeout.snap b/...egration/snapshots/integration_tests__integration__traffic_shaping__subgraph_timeout.snap
@@ -2,4 +2,4 @@
 source: apollo-router/tests/integration/traffic_shaping.rs
 expression: response
 ---
-"{\"data\":null,\"errors\":[{\"message\":\"Request timed out\",\"path\":[],\"extensions\":{\"code\":\"REQUEST_TIMEOUT\"}}]}"
+"{\"data\":null,\"errors\":[{\"message\":\"Request timed out\",\"path\":[],\"extensions\":{\"code\":\"REQUEST_TIMEOUT\",\"service\":\"products\"}}]}"
diff --git a/apollo-router/tests/integration/subgraph_response.rs b/apollo-router/tests/integration/subgraph_response.rs
@@ -81,6 +81,125 @@ async fn test_subgraph_returning_different_typename_on_query_root() -> Result<()
     Ok(())
 }
 
+#[tokio::test(flavor = "multi_thread")]
+async fn test_valid_extensions_service_for_subgraph_error() -> Result<(), BoxError> {
+    let mut router = IntegrationTest::builder()
+        .config(CONFIG)
+        .responder(ResponseTemplate::new(200).set_body_json(json!({
+            "data": { "topProducts": null },
+            "errors": [{
+                "message": "Some error on subgraph",
+                "path": ["topProducts"]
+            }]
+        })))
+        .build()
+        .await;
+
+    router.start().await;
+    router.assert_started().await;
+
+    let (_trace_id, response) = router
+        .execute_query(&json!({ "query": "{ topProducts { name } }" }))
+        .await;
+    assert_eq!(response.status(), 200);
+    assert_eq!(
+        response.json::<serde_json::Value>().await?,
+        json!({
+            "data": { "topProducts": null },
+            "errors": [{
+                "message": "Some error on subgraph",
+                "path":["topProducts"],
+                "extensions": {
+                    "service": "products"
+                }
+            }]
+        })
+    );
+
+    router.graceful_shutdown().await;
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn test_valid_extensions_service_is_preserved_for_subgraph_error() -> Result<(), BoxError> {
+    let mut router = IntegrationTest::builder()
+        .config(CONFIG)
+        .responder(ResponseTemplate::new(200).set_body_json(json!({
+            "data": { "topProducts": null },
+            "errors": [{
+                "message": "Some error on subgraph",
+                "path": ["topProducts"],
+                "extensions": {
+                    "service": 3.14,
+                }
+            }]
+        })))
+        .build()
+        .await;
+
+    router.start().await;
+    router.assert_started().await;
+
+    let (_trace_id, response) = router
+        .execute_query(&json!({ "query": "{ topProducts { name } }" }))
+        .await;
+    assert_eq!(response.status(), 200);
+    assert_eq!(
+        response.json::<serde_json::Value>().await?,
+        json!({
+            "data": { "topProducts": null },
+            "errors": [{
+                "message": "Some error on subgraph",
+                "path":["topProducts"],
+                "extensions": {
+                    "service": 3.14
+                }
+            }]
+        })
+    );
+
+    router.graceful_shutdown().await;
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn test_valid_extensions_service_for_invalid_subgraph_response() -> Result<(), BoxError> {
+    let mut router = IntegrationTest::builder()
+        .config(CONFIG)
+        .responder(ResponseTemplate::new(200))
+        .build()
+        .await;
+
+    router.start().await;
+    router.assert_started().await;
+
+    let (_trace_id, response) = router
+        .execute_query(&json!({ "query": "{ topProducts { name } }" }))
+        .await;
+    assert_eq!(response.status(), 200);
+    assert_eq!(
+        response.json::<serde_json::Value>().await?,
+        json!({
+          "data": null,
+          "errors": [
+            {
+              "message": "HTTP fetch failed from 'products': subgraph response does not contain 'content-type' header; expected content-type: application/json or content-type: application/graphql-response+json",
+              "path": [],
+              "extensions": {
+                "code": "SUBREQUEST_HTTP_ERROR",
+                "service": "products",
+                "reason": "subgraph response does not contain 'content-type' header; expected content-type: application/json or content-type: application/graphql-response+json",
+                "http": { "status": 200 }
+              }
+            }
+          ]
+        })
+    );
+
+    router.graceful_shutdown().await;
+    Ok(())
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn test_valid_error_locations() -> Result<(), BoxError> {
     let mut router = IntegrationTest::builder()
@@ -116,7 +235,8 @@ async fn test_valid_error_locations() -> Result<(), BoxError> {
                     { "line": 1, "column": 2 },
                     { "line": 3, "column": 4 },
                 ],
-                "path":["topProducts"]
+                "path":["topProducts"],
+                "extensions": { "service": "products" }
             }]
         })
     );
@@ -153,7 +273,8 @@ async fn test_empty_error_locations() -> Result<(), BoxError> {
             "data": { "topProducts": null },
             "errors": [{
                 "message":"Some error on subgraph",
-                "path":["topProducts"]
+                "path":["topProducts"],
+                "extensions": { "service": "products" }
             }]
         })
     );
@@ -195,6 +316,7 @@ async fn test_invalid_error_locations() -> Result<(), BoxError> {
                     "service": "products",
                     "reason": "invalid `locations` within error: invalid type: boolean `true`, expected u32",
                     "code": "SUBREQUEST_MALFORMED_RESPONSE",
+                    "service": "products"
                 }
             }]
         })
@@ -213,7 +335,7 @@ async fn test_invalid_error_locations_with_single_negative_one_location() -> Res
             "errors": [{
                 "message": "Some error on subgraph",
                 "locations": [{ "line": -1, "column": -1 }],
-                "path": ["topProducts"]
+                "path": ["topProducts"],
             }]
         })))
         .build()
@@ -232,7 +354,8 @@ async fn test_invalid_error_locations_with_single_negative_one_location() -> Res
             "data": { "topProducts": null },
             "errors": [{
                 "message":"Some error on subgraph",
-                "path":["topProducts"]
+                "path":["topProducts"],
+                "extensions": { "service": "products" }
             }]
         })
     );
@@ -277,7 +400,8 @@ async fn test_invalid_error_locations_contains_negative_one_location() -> Result
                     { "line": 1, "column": 2 },
                     { "line": 3, "column": 4 },
                 ],
-                "path":["topProducts"]
+                "path":["topProducts"],
+                "extensions": { "service": "products" }
             }]
         })
     );